Skip to content

fix: ensure File transport flushes all data before emitting finish#2594

Merged
DABH merged 1 commit intowinstonjs:masterfrom
mldangelo:master
Dec 7, 2025
Merged

fix: ensure File transport flushes all data before emitting finish#2594
DABH merged 1 commit intowinstonjs:masterfrom
mldangelo:master

Conversation

@mldangelo
Copy link
Contributor

@mldangelo mldangelo commented Dec 5, 2025 •
edited
Loading

Fixes #1504 and relates to promptfoo/promptfoo#6511

When you call logger.end() and then process.exit(), log messages get lost. This has been an open issue since 2018.

The problem is that the File transport's finish event fires when the internal PassThrough drains, not when the actual file write completes. So you think you're safe to exit, but the OS hasn't finished writing yet.

The fix adds a _final() method (the standard Node.js stream hook for this) that waits for the fs.WriteStream to finish before letting the transport emit finish.

To verify the fix:

const fs = require('fs');
const winston = require('winston');

const transport = new winston.transports.File({ filename: 'test.log' });
const logger = winston.createLogger({ transports: [transport] });

(async () => {
  for (let i = 0; i < 1000; i++) logger.info(`message ${i}`);
  logger.info('THE LAST MESSAGE');

  await new Promise(resolve => {
    transport.on('finish', resolve);
    logger.end();
  });

  const lines = fs.readFileSync('test.log', 'utf8').split('\n').filter(Boolean);
  console.log(`Lines written: ${lines.length}`);  // should be 1001
})();

Before: ~1 line written. After: all 1001 lines written.

Also added regression tests under "Flushing on end (issue #1504)" in file.test.js.

@mldangelo
fix: ensure File transport flushes all data before emitting finish
1a8130f 
Fixes winstonjs#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.
@mldangelo
Copy link
Contributor Author

mldangelo commented Dec 5, 2025

@DABH would you mind taking a look when you get a chance? This is a long-standing issue that's been open since 2018.

@DABH
Copy link
Contributor

DABH commented Dec 7, 2025

Happy to trust code from a fellow ICME person ;) The argument seems reasonable here and I appreciate the MWE and test. I will look to ship this out as a new minor release. Thank you for your contribution to Winston!

@DABH DABH merged commit 86c890f into winstonjs:master Dec 7, 2025
4 checks passed
Doridian pushed a commit to Doridian/carvera-pendant that referenced this pull request Dec 7, 2025
@Doridian
chore(deps): update dependency winston to v3.19.0 (#39)
a4322b4 
This PR contains the following updates:

| Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) |
|---|---|---|---|
| [winston](https://github.com/winstonjs/winston) | [`3.18.3` -> `3.19.0`](https://renovatebot.com/diffs/npm/winston/3.18.3/3.19.0) | ![age](https://developer.mend.io/api/mc/badges/age/npm/winston/3.19.0?slim=true) | ![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/winston/3.18.3/3.19.0?slim=true) |

---

### Release Notes

<details>
<summary>winstonjs/winston (winston)</summary>

### [`v3.19.0`](https://github.com/winstonjs/winston/releases/tag/v3.19.0)

[Compare Source](winstonjs/winston@v3.18.3...v3.19.0)

- Run npm audit fix  [`e7ccdc4`](winstonjs/winston@e7ccdc4)
- Don't include jest.config.js in npm package  [`5a63c8c`](winstonjs/winston@5a63c8c)
- fix: append error cause when using `logger.child()` ([#&#8203;2467](winstonjs/winston#2467))  [`e74a7ae`](winstonjs/winston@e74a7ae)
- Bump rimraf from 5.0.1 to 5.0.10 ([#&#8203;2517](winstonjs/winston#2517))  [`8a956fd`](winstonjs/winston@8a956fd)
- fix: ensure File transport flushes all data before emitting finish ([#&#8203;2594](winstonjs/winston#2594))  [`86c890f`](winstonjs/winston@86c890f)
- Bump actions/setup-node from 4 to 6 ([#&#8203;2589](winstonjs/winston#2589))  [`3b8be02`](winstonjs/winston@3b8be02)
- Bump [@&#8203;babel/core](https://github.com/babel/core) from 7.28.0 to 7.28.5 ([#&#8203;2591](winstonjs/winston#2591))  [`f4c3e2c`](winstonjs/winston@f4c3e2c)
- Bump actions/checkout from 4 to 6 ([#&#8203;2593](winstonjs/winston#2593))  [`dd7906e`](winstonjs/winston@dd7906e)
- chore: migrate test runner from mocha to jest ([#&#8203;2567](winstonjs/winston#2567))  [`2e9eb18`](winstonjs/winston@2e9eb18)

***

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi4yNy4xIiwidXBkYXRlZEluVmVyIjoiNDIuMjcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==-->

Reviewed-on: https://git.foxden.network/FoxDen/carvera-pendant/pulls/39
Co-authored-by: Renovate <[email protected]>
Co-committed-by: Renovate <[email protected]>
This was referenced Dec 9, 2025
Merged
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

transports.File is not flushing on end.

2 participants

@mldangelo @DABH