Fix Issue where winston removes transport on error (#1364) (#1714)

* Fix Issue where winston removes transport on error (#1364)

* Fix lint error to cause rebuild

lib/winston/logger.js

@@ -622,6 +622,10 @@ class Logger extends Transform {
    */
   _onEvent(event, transport) {
     function transportEvent(err) {
+      // https://github.com/winstonjs/winston/issues/1364
+      if (event === 'error' && !this.transports.includes(transport)) {
+        this.add(transport);
+      }
       this.emit(event, err, transport);
     }
 

lib/winston/tail-file.js

@@ -60,12 +60,12 @@ module.exports = (options, iter) => {
         return;
       }
 
-      return fs.read(fd, buffer, 0, buffer.length, pos, (err, bytes) => {
-        if (err) {
+      return fs.read(fd, buffer, 0, buffer.length, pos, (error, bytes) => {
+        if (error) {
           if (!iter) {
-            stream.emit('error', err);
+            stream.emit('error', error);
           } else {
-            iter(err);
+            iter(error);
           }
           stream.destroy();
           return;

test/transports/error.test.js

@@ -0,0 +1,118 @@
+const winston = require('../../lib/winston');
+const assume = require('assume');
+
+// https://github.com/winstonjs/winston/issues/1364
+describe('transports issue 1364', () => {
+  const mainError = 'Error logging!';
+  const otherError = 'Other error';
+  let logger;
+  let errorMessage;
+  let counter;
+  let maxCounter;
+  let logError;
+  let transport;
+  const newTransport = () =>
+    Object.assign(new winston.transports.Console(), {
+      log: (info, next) => {
+        if (counter === maxCounter) {
+          next(new Error(errorMessage));
+          return;
+        }
+        if (logError !== null) {
+          errorMessage = otherError;
+        }
+        counter = counter + 1;
+        next();
+        return;
+      }
+    });
+  beforeEach(() => {
+    errorMessage = mainError;
+    counter = 0;
+    maxCounter = 1;
+    logError = null;
+    transport = newTransport();
+    logger = winston.createLogger({
+      level: 'info',
+      transports: [transport]
+    });
+    logger.on('error', error => {
+      counter = 0;
+      logError = error;
+    });
+  });
+
+  describe('only log once', () => {
+    beforeEach(() => {
+      logger.info('log once');
+    });
+
+    it('logger transport has single correct transport', () => {
+      const transports = logger.transports;
+      assume(transports).is.an('array');
+      assume(transports).length(1);
+      assume(transports).contains(transport);
+    });
+
+    it("error didn't", () => {
+      assume(logError).not.exists();
+    });
+  });
+
+  describe('log twice', () => {
+    beforeEach(() => {
+      logger.info('log twice 1');
+      logger.info('log twice 2'); // this raises the `mainError` for the transport
+    });
+
+    it('logger transport has single correct transport', () => {
+      const transports = logger.transports;
+      assume(transports).is.an('array');
+      assume(transports).length(1);
+      assume(transports).contains(transport);
+    });
+
+    it('error occurred', () => {
+      assume(logError).property('message', mainError);
+    });
+  });
+
+  describe('log thrice', () => {
+    beforeEach(() => {
+      logger.info('log thrice 1');
+      logger.info('log thrice 2'); // this raises the `mainError` for the transport
+      logger.info('log thrice 3');
+    });
+
+    it('logger transport has single correct transport', () => {
+      const transports = logger.transports;
+      assume(transports).is.an('array');
+      assume(transports).length(1);
+      assume(transports).contains(transport);
+    });
+
+    it('error occurred', () => {
+      assume(logError).property('message', mainError);
+    });
+  });
+
+  describe('log four times', () => {
+    beforeEach(() => {
+      logger.info('log four times 1');
+      logger.info('log four times 2'); // this raises the `mainError` for the transport
+      logger.info('log four times 3');
+      logger.info('log four times 4'); // this raises the `otherError` for the transport
+    });
+
+    it('logger transport has single correct transport', () => {
+      const transports = logger.transports;
+      assume(transports).is.an('array');
+      assume(transports).length(1);
+      assume(transports).contains(transport);
+    });
+
+    it('other error occurred', () => {
+      assume(logError).property('message', otherError);
+    });
+  });
+});