Skip to content

Commit

Permalink
Fix Issue where winston removes transport on error (winstonjs#1364) (w…
Browse files Browse the repository at this point in the history
…instonjs#1714)

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

* Fix lint error to cause rebuild
  • Loading branch information
yinzara authored and DABH committed Jan 1, 2020
1 parent 7c3dbbb commit f61fa32
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 4 deletions.
4 changes: 4 additions & 0 deletions lib/winston/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
8 changes: 4 additions & 4 deletions lib/winston/tail-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
118 changes: 118 additions & 0 deletions test/transports/error.test.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
});

0 comments on commit f61fa32

Please sign in to comment.