From be655c5c447bca839e498d11d03895d993b9cbfc Mon Sep 17 00:00:00 2001 From: Maruthan G Date: Sat, 25 Apr 2026 16:02:23 +0530 Subject: [PATCH 1/2] stream: pass ERR_STREAM_WRITE_AFTER_END to .end(cb) after end `stream.end(cb)` called on a stream that has already been ended did not propagate ERR_STREAM_WRITE_AFTER_END to the callback. Instead, the callback was queued for the eventual `finish` event and called with `null`, which was inconsistent with `.write(chunk, cb)` and `.end(chunk, cb)` after end, both of which correctly call `cb` with ERR_STREAM_WRITE_AFTER_END. In `Writable.prototype.end`, when the stream is already ending and not yet finished or destroyed and a user callback was supplied, set the error to ERR_STREAM_WRITE_AFTER_END so the existing `process.nextTick(cb, err)` path delivers it. The forgiving behavior of `.end()` without a callback being called multiple times is preserved (no call to errorOrDestroy). Fixes: https://github.com/nodejs/node/issues/33684 Signed-off-by: Maruthan G --- lib/internal/streams/writable.js | 8 ++ .../test-stream-writable-end-cb-after-end.js | 74 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 test/parallel/test-stream-writable-end-cb-after-end.js diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index 89d384e9f2fb8f..a30b4638337f71 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -845,6 +845,14 @@ Writable.prototype.end = function(chunk, encoding, cb) { err = new ERR_STREAM_ALREADY_FINISHED('end'); } else if ((state[kState] & kDestroyed) !== 0) { err = new ERR_STREAM_DESTROYED('end'); + } else if ((state[kState] & kEnding) !== 0 && typeof cb === 'function') { + // The stream is already ending (or ended) but not yet finished or + // destroyed. For consistency with `.write(chunk, cb)` after end (which + // invokes cb with ERR_STREAM_WRITE_AFTER_END), surface the same error + // through the user-supplied callback. Note: we do not call + // errorOrDestroy here in order to preserve the pre-existing forgiving + // behavior of `.end()` (no cb) called multiple times. + err = new ERR_STREAM_WRITE_AFTER_END(); } if (typeof cb === 'function') { diff --git a/test/parallel/test-stream-writable-end-cb-after-end.js b/test/parallel/test-stream-writable-end-cb-after-end.js new file mode 100644 index 00000000000000..112fe217a892c7 --- /dev/null +++ b/test/parallel/test-stream-writable-end-cb-after-end.js @@ -0,0 +1,74 @@ +'use strict'; + +// Regression test for https://github.com/nodejs/node/issues/33684 +// Calling `.end(cb)` on a stream that has already been ended must invoke +// the user-supplied callback with ERR_STREAM_WRITE_AFTER_END, mirroring +// the behavior of `.write(chunk, cb)` after end and `.end(chunk, cb)` +// after end. + +const common = require('../common'); +const assert = require('assert'); +const { Writable } = require('stream'); + +{ + // `.end()` followed by `.end(cb)` — cb must receive ERR_STREAM_WRITE_AFTER_END. + const w = new Writable({ + write(chunk, encoding, cb) { cb(); }, + }); + + w.end(); + w.end(common.mustCall((err) => { + assert.ok(err); + assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); + })); +} + +{ + // `.end()` followed by `.end('chunk', cb)` — cb must receive + // ERR_STREAM_WRITE_AFTER_END (existing behavior, included for parity). + const w = new Writable({ + write(chunk, encoding, cb) { cb(); }, + }); + + w.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); + })); + + w.end(); + w.end('chunk', common.mustCall((err) => { + assert.ok(err); + assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); + })); +} + +{ + // `.write(chunk, cb)` after end calls cb with ERR_STREAM_WRITE_AFTER_END — + // sanity check confirming that `.end(cb)` is now consistent with this path. + const w = new Writable({ + write(chunk, encoding, cb) { cb(); }, + }); + + w.on('error', common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); + })); + + w.end(); + w.write('chunk', common.mustCall((err) => { + assert.ok(err); + assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); + })); +} + +{ + // `.end(null, encoding, cb)` after end — cb must receive + // ERR_STREAM_WRITE_AFTER_END. + const w = new Writable({ + write(chunk, encoding, cb) { cb(); }, + }); + + w.end(); + w.end(null, null, common.mustCall((err) => { + assert.ok(err); + assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END'); + })); +} From 7f71f50d11e807d217a7a765dd1a02e4b20e8eb3 Mon Sep 17 00:00:00 2001 From: Maruthan G Date: Sat, 25 Apr 2026 19:39:19 +0530 Subject: [PATCH 2/2] fixup! stream: pass ERR_STREAM_WRITE_AFTER_END to .end(cb) after end The previous version always synthesized ERR_STREAM_WRITE_AFTER_END when kEnding was set, which broke test-stream-writable-end-cb-error block 1: a writable whose underlying _write errors with `_err`, and whose `.end(cb1)` and `.end(cb2)` are both expected to receive that underlying error via kOnFinished. Tighten the condition to only synthesize WRITE_AFTER_END when the stream has no in-flight write (kWriting), no buffered data (kBuffered), no stored error (kErrored), and an empty buffer length. In any of those cases there is real pending state that will surface a meaningful error through the existing kOnFinished cascade, so the cb should be queued and receive that real error instead. Signed-off-by: Maruthan G --- lib/internal/streams/writable.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js index a30b4638337f71..d0d666d387b922 100644 --- a/lib/internal/streams/writable.js +++ b/lib/internal/streams/writable.js @@ -845,13 +845,15 @@ Writable.prototype.end = function(chunk, encoding, cb) { err = new ERR_STREAM_ALREADY_FINISHED('end'); } else if ((state[kState] & kDestroyed) !== 0) { err = new ERR_STREAM_DESTROYED('end'); - } else if ((state[kState] & kEnding) !== 0 && typeof cb === 'function') { - // The stream is already ending (or ended) but not yet finished or - // destroyed. For consistency with `.write(chunk, cb)` after end (which - // invokes cb with ERR_STREAM_WRITE_AFTER_END), surface the same error - // through the user-supplied callback. Note: we do not call - // errorOrDestroy here in order to preserve the pre-existing forgiving - // behavior of `.end()` (no cb) called multiple times. + } else if ((state[kState] & kEnding) !== 0 && typeof cb === 'function' && + (state[kState] & (kErrored | kWriting | kBuffered)) === 0 && + state.length === 0) { + // The stream is already ending and has no in-flight write, buffered + // data, or stored error that would surface a real error to the user. + // Synthesize ERR_STREAM_WRITE_AFTER_END so the user-supplied callback + // fires consistently with `.write(chunk, cb)` after end. When pending + // work or a stored error exists, fall through so the cb is queued on + // kOnFinished and receives that real error instead. err = new ERR_STREAM_WRITE_AFTER_END(); }