'reemitErrorEvents' bool on Bunyan streams to control error event handling

Related to PR #318.

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
+- [pull #318] Add `reemitErrorEvents` optional boolean for streams added to a
-  *any stream with a `.on()`* -- which is any that inherits from EventEmitter.
+  Bunyan logger to control whether an "error" event on the stream will be
-  Before this change, 'error' events were only re-emitted on [`file`
+  re-emitted on the `Logger` instance.
-  streams](https://github.com/trentm/node-bunyan#stream-type-file).
+
-  (By Marc Udoff.)
+        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

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
+A Bunyan logger instance can be made to re-emit "error" events from its
-do this:
+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
+As of bunyan@1.7.0, the `reemitErrorEvents` field can be used when adding a
-it has a `.on()` -- e.g. if it inherits from EventEmitter.
+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
+    var EventEmitter = require('events').EventEmitter;
-as produced by `log.error(...)`.
+    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`

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 `<logger>.close()`, it should remove event
+        //      listeners to not leak Logger instances.
         s.stream.on('error', function onStreamError(err) {
             self.emit('error', err, s);
         });

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) {
+var BOGUS_PATH = '/this/path/is/bogus.log';
-    LOG_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) {
+test('error event on raw stream (reemitErrorEvents=undefined)', function (t) {
-    LOG_PATH = '/this/path/is/bogus.log'
+    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');