diff --git a/CHANGES.md b/CHANGES.md index f82ce3f..201127a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,11 +8,41 @@ Known issues: ## 1.7.0 (not yet released) -- [pull #318] Re-emit Bunyan stream 'error' events on the Logger instance from - *any stream with a `.on()`* -- which is any that inherits from EventEmitter. - Before this change, 'error' events were only re-emitted on [`file` - streams](https://github.com/trentm/node-bunyan#stream-type-file). - (By Marc Udoff.) +- [pull #318] Add `reemitErrorEvents` optional boolean for streams added to a + Bunyan logger to control whether an "error" event on the stream will be + re-emitted on the `Logger` instance. + + var log = bunyan.createLogger({ + name: 'foo', + streams: [ + { + type: 'raw', + stream: new MyCustomStream(), + reemitErrorEvents: true + } + ] + }); + + Before this change, "error" events were re-emitted on [`file` + streams](https://github.com/trentm/node-bunyan#stream-type-file) only. The new + behaviour is as follows: + + - `reemitErrorEvents` not specified: `file` streams will re-emit error events + on the Logger instance. + - `reemitErrorEvents: true`: error events will be re-emitted on the Logger + for any stream with a `.on()` function -- which includes file streams, + process.stdout/stderr, and any object that inherits from EventEmitter. + - `reemitErrorEvents: false`: error events will not be re-emitted for any + streams. + + Dev Note: Bunyan `Logger` objects don't currently have a `.close()` method + in which registered error event handlers can be *un*registered. That means + that a (presumably rare) situation where code adds dozens of Bunyan Logger + streams to, e.g. process.stdout, and with `reemitErrorEvents: true`, could + result in leaking Logger objects. + + Original work for allowing "error" re-emitting on non-file streams is + by Marc Udoff in pull #318. ## 1.6.0 diff --git a/README.md b/README.md index 8613d0f..fea7f9e 100644 --- a/README.md +++ b/README.md @@ -612,9 +612,9 @@ follow (feedback from actual users welcome). # Streams -A "stream" is Bunyan's name for an output for log messages (the equivalent +A "stream" is Bunyan's name for where it outputs log messages (the equivalent to a log4j Appender). Ultimately Bunyan uses a -[Writable Stream](http://nodejs.org/docs/latest/api/all.html#writable_Stream) +[Writable Stream](https://nodejs.org/docs/latest/api/all.html#writable_Stream) interface, but there are some additional attributes used to create and manage the stream. A Bunyan Logger instance has one or more streams. In general streams are specified with the "streams" option: @@ -654,8 +654,9 @@ type "stream" emitting to `process.stdout` at the "info" level. ## stream errors -Bunyan re-emits "error" events from the created `WriteStream`. So you can -do this: +A Bunyan logger instance can be made to re-emit "error" events from its +streams. Bunyan does so by defualt for [`type === "file"` +streams](#stream-type-file), so you can do this: ```js var log = bunyan.createLogger({name: 'mylog', streams: [{path: LOG_PATH}]}); @@ -664,11 +665,45 @@ log.on('error', function (err, stream) { }); ``` -As of bunyan@1.7.0, "error" events are re-emitted for any stream, as long as -it has a `.on()` -- e.g. if it inherits from EventEmitter. +As of bunyan@1.7.0, the `reemitErrorEvents` field can be used when adding a +stream to control whether "error" events are re-emitted on the Logger. For +example: -Note: This error eventi is **not** related to log records at the "error" level -as produced by `log.error(...)`. + var EventEmitter = require('events').EventEmitter; + var util = require('util'); + + function MyFlakyStream() {} + util.inherits(MyFlakyStream, EventEmitter); + + MyFlakyStream.prototype.write = function (rec) { + this.emit('error', new Error('boom')); + } + + var log = bunyan.createLogger({ + name: 'this-is-flaky', + streams: [ + { + type: 'raw', + stream: new MyFlakyStream(), + reemitErrorEvents: true + } + ] + }); + log.info('hi there'); + +The behaviour is as follows: + +- `reemitErrorEvents` not specified: `file` streams will re-emit error events + on the Logger instance. +- `reemitErrorEvents: true`: error events will be re-emitted on the Logger + for any stream with a `.on()` function -- which includes file streams, + process.stdout/stderr, and any object that inherits from EventEmitter. +- `reemitErrorEvents: false`: error events will not be re-emitted for any + streams. + +Note: "error" events are **not** related to log records at the "error" level +as produced by `log.error(...)`. See [the node.js docs on error +events](https://nodejs.org/api/events.html#events_error_events) for details. ## stream type: `stream` diff --git a/lib/bunyan.js b/lib/bunyan.js index 0741759..8263f58 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -518,7 +518,6 @@ Logger.prototype.addStream = function addStream(s, defaultLevel) { s = objCopy(s); // Implicit 'type' from other args. - var type = s.type; if (!s.type) { if (s.stream) { s.type = 'stream'; @@ -544,6 +543,9 @@ Logger.prototype.addStream = function addStream(s, defaultLevel) { } break; case 'file': + if (s.reemitErrorEvents === undefined) { + s.reemitErrorEvents = true; + } if (!s.stream) { s.stream = fs.createWriteStream(s.path, {flags: 'a', encoding: 'utf8'}); @@ -576,7 +578,9 @@ Logger.prototype.addStream = function addStream(s, defaultLevel) { throw new TypeError('unknown stream type "' + s.type + '"'); } - if (typeof (s.stream.on) === 'function') { + if (s.reemitErrorEvents && typeof (s.stream.on) === 'function') { + // TODO: When we have `.close()`, it should remove event + // listeners to not leak Logger instances. s.stream.on('error', function onStreamError(err) { self.emit('error', err, s); }); diff --git a/test/error-event.test.js b/test/error-event.test.js index af9beca..b981244 100644 --- a/test/error-event.test.js +++ b/test/error-event.test.js @@ -1,5 +1,5 @@ /* - * Copyright 2016 Trent Mick. All rights reserved. + * Copyright 2016 Trent Mick * * Test emission and handling of 'error' event in a logger with a 'path' * stream. @@ -19,42 +19,126 @@ var before = tap4nodeunit.before; var test = tap4nodeunit.test; -test('error event on log write', function (t) { - LOG_PATH = '/this/path/is/bogus.log' +var BOGUS_PATH = '/this/path/is/bogus.log'; + +test('error event on file stream (reemitErrorEvents=undefined)', function (t) { var log = bunyan.createLogger( - {name: 'error-event', streams: [ {path: LOG_PATH} ]}); + {name: 'error-event-1', streams: [ {path: BOGUS_PATH} ]}); log.on('error', function (err, stream) { t.ok(err, 'got err in error event: ' + err); t.equal(err.code, 'ENOENT', 'error code is ENOENT'); t.ok(stream, 'got a stream argument'); - t.equal(stream.path, LOG_PATH); + t.equal(stream.path, BOGUS_PATH); t.equal(stream.type, 'file'); t.end(); }); log.info('info log message'); }); +test('error event on file stream (reemitErrorEvents=true)', function (t) { + var log = bunyan.createLogger({ + name: 'error-event-2', + streams: [{ + path: BOGUS_PATH, + reemitErrorEvents: true + }] + }); + log.on('error', function (err, stream) { + t.ok(err, 'got err in error event: ' + err); + t.equal(err.code, 'ENOENT', 'error code is ENOENT'); + t.ok(stream, 'got a stream argument'); + t.equal(stream.path, BOGUS_PATH); + t.equal(stream.type, 'file'); + t.end(); + }); + log.info('info log message'); +}); -function MyErroringStream() { +test('error event on file stream (reemitErrorEvents=false)', + function (t) { + var log = bunyan.createLogger({ + name: 'error-event-3', + streams: [{ + path: BOGUS_PATH, + reemitErrorEvents: false + }] + }); + // Hack into the underlying created file stream to catch the error event. + log.streams[0].stream.on('error', function (err) { + t.ok(err, 'got error event on the file stream'); + t.end(); + }); + log.on('error', function (err, stream) { + t.fail('should not have gotten error event on logger'); + t.end(); + }); + log.info('info log message'); +}); -} + +function MyErroringStream() {} util.inherits(MyErroringStream, EventEmitter); - MyErroringStream.prototype.write = function (rec) { this.emit('error', new Error('boom')); } -test('error event on log write (raw stream)', function (t) { - LOG_PATH = '/this/path/is/bogus.log' +test('error event on raw stream (reemitErrorEvents=undefined)', function (t) { + var estream = new MyErroringStream(); var log = bunyan.createLogger({ name: 'error-event-raw', streams: [ { - stream: new MyErroringStream(), + stream: estream, type: 'raw' } ] }); + estream.on('error', function (err) { + t.ok(err, 'got error event on the raw stream'); + t.end(); + }); + log.on('error', function (err, stream) { + t.fail('should not have gotten error event on logger'); + t.end(); + }); + log.info('info log message'); +}); + +test('error event on raw stream (reemitErrorEvents=false)', function (t) { + var estream = new MyErroringStream(); + var log = bunyan.createLogger({ + name: 'error-event-raw', + streams: [ + { + stream: estream, + type: 'raw', + reemitErrorEvents: false + } + ] + }); + estream.on('error', function (err) { + t.ok(err, 'got error event on the raw stream'); + t.end(); + }); + log.on('error', function (err, stream) { + t.fail('should not have gotten error event on logger'); + t.end(); + }); + log.info('info log message'); +}); + +test('error event on raw stream (reemitErrorEvents=true)', function (t) { + var estream = new MyErroringStream(); + var log = bunyan.createLogger({ + name: 'error-event-raw', + streams: [ + { + stream: estream, + type: 'raw', + reemitErrorEvents: true + } + ] + }); log.on('error', function (err, stream) { t.ok(err, 'got err in error event: ' + err); t.equal(err.message, 'boom');