Skip to content

Commit

Permalink
Merge pull request #61 from dmiddlecamp/fix_econnrefused_exit
Browse files Browse the repository at this point in the history
Patch to try and address #20 / #49 / #56, peacefully log connection errors …
  • Loading branch information
troy committed Nov 7, 2016
2 parents 62160a4 + 501634c commit d152796
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 6 deletions.
34 changes: 28 additions & 6 deletions lib/winston-papertrail.js
Expand Up @@ -229,7 +229,7 @@ var Papertrail = exports.Papertrail = function (options) {
// make sure we prevent simultaneous attempts to connect and handle errors
self._erroring = true;

self.emit('error', err);
self._silentErrorEmitter(err);

// We may be disconnected from the papertrail endpoint for any number of reasons;
// i.e. inactivity, network problems, etc, and we need to be resilient against this
Expand All @@ -248,11 +248,11 @@ var Papertrail = exports.Papertrail = function (options) {

// Stop buffering messages after a fixed number of retries.
// This is to keep the buffer from growing unbounded
if (self.loggingEnabled &&
(self.totalRetries >= (self.maximumAttempts))) {
self.loggingEnabled = false;
self.emit('error', new Error('Max entries eclipsed, disabling buffering'));
}
if (self.loggingEnabled && (self.totalRetries >= (self.maximumAttempts)))
{
self.loggingEnabled = false;
self._silentErrorEmitter(new Error('Max entries eclipsed, disabling buffering'));
}

// continue
self._erroring = false;
Expand Down Expand Up @@ -456,3 +456,25 @@ Papertrail.prototype.close = function() {
});
}
};

/**
* The goal here is to ignore connection errors by default without triggering an uncaughtException.
* You can still bind your own error handler as normal, but if you haven't overridden it, connection errors
* will be logged to the console by default.
*
* Notes: This is meant to fix usability issues #20, and #49
*
* @param err
* @private
*/
Papertrail.prototype._silentErrorEmitter = function(err) {
var count = this.listeners('error').length;
if (count > 0) {
// the goal here is to ensure someone is catching this event, instead of potentially
// causing the process to exit.
this.emit('error', err);
}
else {
console.error('Papertrail connection error: ', err);
}
};
16 changes: 16 additions & 0 deletions test/papertrail-test.js
Expand Up @@ -33,6 +33,22 @@ describe('connection tests', function() {
});
});

it('should not exit process when it fails to connect', function (done) {
var pt = new Papertrail({
host: 'this.wont.resolve',
port: 12345,
attemptsBeforeDecay: 0,
connectionDelay: 10000
});

// NOTE! We intentionally left off the error handler here, this proves
// the process won't exit on a connection error.

setTimeout(function() {
done();
}, 125);
});

it.skip('should timeout', function (done) {
var pt = new Papertrail({
host: '8.8.8.8', // TODO Figure out how to enable a timeout test
Expand Down

0 comments on commit d152796

Please sign in to comment.