1082551 - wrong call in log4moz.js ( "this._fos().XXX(...)" should be "this._fos.XXX(...)" )

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[rene]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Firefox/24.0
Build ID: 20140927221706

Steps to reproduce:

tried to use FileAppender


Actual results:

Error writing file:
TypeError: this._fos is not a function


Expected results:

file should have opened...

see https://bugzilla.mozilla.org/show_bug.cgi?id=470208

this._fos().write(message, message.length);
should be
this._fos.write(message, message.length);

Comment 1

[WADA:World Anti-bad-Duping Agency]

(In reply to rene from comment #0
> Actual results:
> Error writing file:
> TypeError: this._fos is not a function
>
> this._fos().write(message, message.length);
> should be
> this._fos.write(message, message.length);

Code is here.
> http://mxr.mozilla.org/comm-central/source/mailnews/db/gloda/modules/log4moz.js#745

Property, function, is defined by;
> 713 FileAppender.prototype = {
> 714   __proto__: Appender.prototype,
> 715 
> 716   __fos: null,
> 717   get _fos() {
> 718     if (!this.__fos)
> 719       this.openStream();
> 720     return this.__fos;
> 721   },

I believe following(you suggested) will produce "this._fos is undefined".
   this._fos.write(message, message.length);

I think;
   this._fos().write(message, message.length);
should be
   this.__fos.write(message, message.length);
or
   this.get_fos().write(message, message.length);

I guess "__fos" like one(two "_") is for internal use at deeper level of an Object. "this.get_fos()" like one is better used by higher level of an Object.

FYI.
"._fos." (not ".__fos.") is seen in log4moz.js.
> http://mxr.mozilla.org/comm-central/source/mailnews/db/gloda/modules/log4moz.js#783
> 778   doAppend: function RFApp_doAppend(message) {
> 779     if (message === null || message.length <= 0)
> 780       return;
> 781     try {
> 782       this.rotateLogs();
> 783       this._fos.write(message, message.length);   <=  Should be "this.get_fos().write(...)" or "this.__fos.write(...)"
> 784     } catch(e) {
> 785       dump("Error writing file:\n" + e);
> 786     }

Comment 2

[rene]

wada wrote:
>   this._fos().write(message, message.length);
>should be
>   this.__fos.write(message, message.length);
>or
>   this.get_fos().write(message, message.length);

no, this._fos (one _, without brackets) uses the getter and delivers this.__fos (two slashes) while ensuring that the stream is already open.

I'm currently using the suggested change https://www.absorb.it/virtual-id/browser/modules/vI_log.js#L292 and it works

Comment 3

[WADA:World Anti-bad-Duping Agency]

Sorry, it was "get _fos()" which is getter, instead of "get_fos()"(no space). I missed a space.
Code in "doAppend: function FApp_doAppend" should be "this._fos.write", as written in "doAppend: function RFApp_doAppend".

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v1

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 8648487 [details] [diff] [review]
patch, v1

Review of attachment 8648487 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, r=mkmelin

Comment 6

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/6df59da4dfced548f5950296542f60e044b26b85
Bug 1082551 - log4moz: this._fos() should be this._fos. r=mkmelin