Skip to content

fix: stop tail delivery after stream destruction - #2650

Open
sergioperezcheco wants to merge 1 commit into
winstonjs:masterfrom
sergioperezcheco:fix/tail-stream-cancellation
Open

sergioperezcheco wants to merge 1 commit into
winstonjs:masterfrom
sergioperezcheco:fix/tail-stream-cancellation

Conversation

@sergioperezcheco

Copy link
Copy Markdown

Stop tail delivery when the stream is destroyed

Destroying a file-backed Logger.stream() from its first log listener currently still delivers the remaining records in the same filesystem read. A read already in flight can also emit data and line after destruction. The existing destroyed check only runs before starting the next read.

Check destruction again when fs.read completes and while iterating its decoded lines. This preserves normal delivery and the existing descriptor cleanup path without changing file rotation, start offsets, or public APIs.

The regression tests cover cancellation from data and line listeners, the public File stream, delayed read completion, descriptor closure, and normal delivery. On the original source, four regressions fail and the delivery control passes; all five pass with this change. An independently written public Logger.stream() probe also reproduces the extra records on the original source and passes against both the fixed source and rebuilt Babel output.

Validation

  • Focused regression suite, lint, Babel build, integration suite, and TypeScript checks passed on Node 22/macOS.
  • The full unit suite passed with --runInBand --forceExit, including coverage thresholds. The ordinary unit command completes its assertions but does not exit because existing tail tests leave timers running; this also reproduces on the original source. This PR does not modify those unrelated tests.
  • Other Node versions and operating systems were not tested locally.

Authored and reviewed by autonomous Hermes agents, with no human review claimed.

Assisted-by: Hermes autonomous AI agent
Signed-off-by: sergioperezcheco <checo520@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant