keymetrics / pm2-logrotate

Automatically rotate all applications logs managed by PM2
1.24k stars 138 forks source link

Bad "this" reference when notifying of file error. (this._interpretError is not a function) #41

Open dangerbacon opened 7 years ago

dangerbacon commented 7 years ago

logrotate encountered a file permissions issue when rotating a file to expected and normal file permissions issues. That one is expected.

However, after that error, a second error inside pm2-logrotate causes logrotate to fail. This is the stack trace:

TypeError: this._interpretError is not a function
    at WriteStream.Notify.notify (/home/pm2user/.pm2/node_modules/pmx/lib/notify.js:106:22)
    at emitOne (events.js:101:20)
    at WriteStream.emit (events.js:188:7)
    at WriteStream.<anonymous> (fs.js:2109:12)
    at FSReqWrap.oncomplete (fs.js:123:15)

This is when executing of this fragment of code:

Notify.notify = function(err) {

  var ret_err = this._interpretError(err);

When called, "this" is not a Notify object, so "this._interpretError" fails. I believe this is caused by these lines in logrotate app.js:

  // listen for error
        readStream.on('error', pmx.notify);
  writeStream.on('error', pmx.notify);
  if (COMPRESSION)
    GZIP.on('error', pmx.notify);

When these encounter errors, they call the right function, but the "this" reference is incorrect.

I believe you can fix it by doing something like this:

  // listen for error
        readStream.on('error', pmx.notify.bind(pmx));
  writeStream.on('error', pmx.notify.bind(pmx));
  if (COMPRESSION)
    GZIP.on('error', pmx.notify.bind(pmx));

The "bind" will cause these events to call the notify function with the pmx object as the "this" reference.

farin99 commented 7 years ago

Any updates on this issue? I'm seeing this error when using node 7.8