Skip to content

stream: prevent pipeline() callback after sync throw - #65165

Open
pacocartones wants to merge 1 commit into
nodejs:mainfrom
pacocartones:fix/pipeline-sync-throw-double-report
Open

stream: prevent pipeline() callback after sync throw#65165
pacocartones wants to merge 1 commit into
nodejs:mainfrom
pacocartones:fix/pipeline-sync-throw-double-report

Conversation

@pacocartones

Copy link
Copy Markdown

Fixes: #65127

Summary

When pipeline() throws synchronously while wiring streams together β€” for
example, when a middle stage returns an invalid value (ERR_INVALID_RETURN_VALUE)
β€” the stages already wired have already incremented finishCount. When those
stages complete afterwards, their finish callbacks invoke the completion
callback with no error, double-reporting the failure that the caller
already received as a synchronous exception.

This change tracks synchronous throws while wiring, destroys the stages wired
so far, and skips the completion callback in that case.

Repro (issue case A)

const { pipeline, Readable, Transform } = require('node:stream');
const r = Readable.from(['a']);
const t = new Transform({ transform(c, e, cb) { cb(null, c); } });
try {
  pipeline(r, t, () => 42, (err) => console.log('callback:', err ?? 'NO ERROR'));
} catch (err) {
  console.log('threw:', err.code);
}

Before: threw: ERR_INVALID_RETURN_VALUE and callback: NO ERROR (double report).
After: threw: ERR_INVALID_RETURN_VALUE only; the callback is never invoked.

Verification

  • Regression test added to test/parallel/test-stream-pipeline.js:
    • fails on the unfixed code (common.mustNotCall() gets invoked with success),
    • passes with the fix.
  • Behavior of normal success, stream-error propagation, generator sources and
    abort paths verified unchanged against the pristine implementation.
Checklist

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels Aug 9, 2026
When a stage in the middle of pipeline() throws synchronously (e.g. an
invalid return value), the stages already wired have already incremented
finishCount. When those stages later complete, their finish callbacks
invoke the completion callback with no error β€” double-reporting the
failure that the caller already received as an exception.

Track synchronous throws while wiring, destroy the stages wired so far,
and skip the completion callback in that case.

Fixes: nodejs#65127

Signed-off-by: pacocartones <pacocartones@users.noreply.github.com>
@pacocartones
pacocartones force-pushed the fix/pipeline-sync-throw-double-report branch from 88532ab to 4c49ce4 Compare August 9, 2026 17:17
MILLERMARRU

This comment was marked as low quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

stream.pipeline() can both throw synchronously and invoke the callback reporting success

3 participants