Skip to content

Commit 86c890f

Browse files
authored
fix: ensure File transport flushes all data before emitting finish (#2594)
Fixes #1504 The File transport was emitting the 'finish' event before all buffered data was actually written to disk. This caused data loss when calling logger.end() followed by process.exit(). The root cause was that the transport's internal PassThrough stream could finish before the underlying fs.WriteStream had flushed to disk. This adds a _final() method that waits for the destination file stream to complete before signaling that the transport has finished.
1 parent 3b8be02 commit 86c890f

File tree

2 files changed

+111
-0
lines changed

2 files changed

+111
-0
lines changed

lib/winston/transports/file.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,38 @@ module.exports = class File extends TransportStream {
109109
}
110110
}
111111

112+
/**
113+
* Called by Node.js Writable stream before emitting 'finish'.
114+
* Ensures all buffered data is flushed to the underlying file stream
115+
* before the transport signals completion.
116+
* @param {Function} callback - Callback to signal completion.
117+
* @private
118+
*/
119+
_final(callback) {
120+
// If still opening, wait for the file to be opened first
121+
if (this._opening) {
122+
this.once('open', () => this._final(callback));
123+
return;
124+
}
125+
126+
// End the PassThrough stream
127+
this._stream.end();
128+
129+
// No destination stream, call callback immediately
130+
if (!this._dest) {
131+
return callback();
132+
}
133+
134+
// Destination is already finished
135+
if (this._dest.writableFinished) {
136+
return callback();
137+
}
138+
139+
// Wait for destination stream to finish writing
140+
this._dest.once('finish', callback);
141+
this._dest.once('error', callback);
142+
}
143+
112144
/**
113145
* Core logging method exposed to Winston. Metadata is optional.
114146
* @param {Object} info - TODO: add param description.

test/unit/winston/transports/file.test.js

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,85 @@ describe('File Transport', function () {
399399
it.todo('should write to the stream when logged to with expected object');
400400
});
401401

402+
describe('Flushing on end (issue #1504)', function () {
403+
it('should flush all messages to file when logger.end() is called', async function () {
404+
const expectedFilename = 'test-flush.log';
405+
const expectedMessageCount = 100;
406+
407+
const logger = winston.createLogger({
408+
transports: [
409+
new winston.transports.File({
410+
filename: expectedFilename,
411+
dirname: testLogFixturesPath
412+
})
413+
]
414+
});
415+
416+
// Log multiple messages
417+
for (let i = 0; i < expectedMessageCount; i++) {
418+
logger.info(`message ${i}`);
419+
}
420+
logger.info('THE LAST MESSAGE');
421+
422+
// Wait for logger to finish
423+
await new Promise((resolve) => {
424+
logger.on('finish', resolve);
425+
logger.end();
426+
});
427+
428+
// Verify all messages were written
429+
await waitForFile(expectedFilename);
430+
const fileContents = fs.readFileSync(getFilePath(expectedFilename), 'utf8');
431+
const lines = fileContents.split('\n').filter(l => l.trim());
432+
433+
assert.strictEqual(
434+
lines.length,
435+
expectedMessageCount + 1,
436+
`Expected ${expectedMessageCount + 1} lines, got ${lines.length}`
437+
);
438+
assert.ok(
439+
fileContents.includes('THE LAST MESSAGE'),
440+
'File should contain THE LAST MESSAGE'
441+
);
442+
});
443+
444+
it('should flush all messages when transport finish event is used', async function () {
445+
const expectedFilename = 'test-flush-transport.log';
446+
const expectedMessageCount = 50;
447+
448+
const transport = new winston.transports.File({
449+
filename: expectedFilename,
450+
dirname: testLogFixturesPath
451+
});
452+
453+
const logger = winston.createLogger({
454+
transports: [transport]
455+
});
456+
457+
// Log multiple messages
458+
for (let i = 0; i < expectedMessageCount; i++) {
459+
logger.info(`message ${i}`);
460+
}
461+
462+
// Wait for transport to finish (original issue pattern)
463+
await new Promise((resolve) => {
464+
transport.on('finish', resolve);
465+
logger.end();
466+
});
467+
468+
// Verify all messages were written
469+
await waitForFile(expectedFilename);
470+
const fileContents = fs.readFileSync(getFilePath(expectedFilename), 'utf8');
471+
const lines = fileContents.split('\n').filter(l => l.trim());
472+
473+
assert.strictEqual(
474+
lines.length,
475+
expectedMessageCount,
476+
`Expected ${expectedMessageCount} lines, got ${lines.length}`
477+
);
478+
});
479+
});
480+
402481

403482
// TODO: Reintroduce these tests
404483
//

0 commit comments

Comments
 (0)