stream: fix TypeError in writev when writer.ready rejects - #62291
stream: fix TypeError in writev when writer.ready rejects#62291mango766 wants to merge 1 commit into
Conversation
The `done` callback in the `writev` handlers of both `newStreamWritableFromWritableStream` and `newStreamDuplexFromReadableWritablePair` unconditionally called `error.filter()`, assuming `error` is always an array from `SafePromiseAll`. However, `done` is also used as the rejection handler for `writer.ready`, which passes a single error value, not an array. This caused a `TypeError: error.filter is not a function` which became an unhandled rejection, crashing the process. Fix by checking whether `error` is an array before filtering, and also use `ArrayIsArray`/`ArrayPrototypeFilter` from primordials to follow Node.js internal conventions. Fixes: nodejs#62199
nodejs-github-bot
added
needs-ci
| // When error comes from SafePromiseAll, it is an array of | ||
| // errors (one per chunk). When it comes from writer.ready | ||
| // rejection, it is a single error. Normalize to handle both. |
There was a problem hiding this comment.
I don't believe this assumption was correct originally? If an array is returned by Promise.all, then it means that all of the passed Promises resolved successfully. If any of them fail, then Promise.all will reject with the first rejection reason, not an array.
|
This pull request has been marked as stale due to 90 days of inactivity. |
github-actions
Bot
added
the
stale
|
This pull request has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days). |
3 participants
The
donecallback in thewritevhandlers of bothnewStreamWritableFromWritableStreamandnewStreamDuplexFromReadableWritablePairunconditionally callederror.filter(), assumingerroris always an array (fromSafePromiseAll). However,doneis also used as the rejectionhandler for
writer.ready, which passes a single error value,not an array. This caused a
TypeError: Cannot read properties of null (reading 'filter')which became an unhandled rejection,crashing the process.
Root cause: When a
Duplex.fromWeb()stream is corked, writesare batched into a
_writevcall. Ifdestroy()is called in thesame microtask (after
uncork()), the underlyingWritableStreamwriter gets aborted, causing
writer.readyto reject. Thedonecallback then receives a single error instead of the array it
expects from
SafePromiseAll.Fix: Check whether
erroris an array before callingArrayPrototypeFilteron it. This handles both code paths:SafePromiseAllresolution/rejection → array of results/errorswriter.readyrejection → single error valueAlso uses
ArrayIsArray/ArrayPrototypeFilterfrom primordials,consistent with Node.js internal conventions.
Includes a regression test that reproduces the exact scenario from
the issue report (cork → write → write → uncork → destroy).
Fixes: #62199