Skip to content

Commit

Permalink
stream: don't push null from closed promise #42694
Browse files Browse the repository at this point in the history
closed promise is subscribed to first so will be
resolved first, before any read promise.

This causes data after EOF error to be thrown.

Remove the push null from the closed promise handler.
The push null gets done from the read handler
when it detects done.

PR-URL: #45026
Fixes: #42694
Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
davedoesdev authored and danielleadams committed Jan 3, 2023
1 parent 896b48b commit c514751
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 5 deletions.
5 changes: 0 additions & 5 deletions lib/internal/webstreams/adapters.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ const {
const {
isDestroyed,
isReadable,
isReadableEnded,
isWritable,
isWritableEnded,
} = require('internal/streams/utils');
Expand Down Expand Up @@ -528,8 +527,6 @@ function newStreamReadableFromReadableStream(readableStream, options = kEmptyObj
reader.closed,
() => {
closed = true;
if (!isReadableEnded(readable))
readable.push(null);
},
(error) => {
closed = true;
Expand Down Expand Up @@ -794,8 +791,6 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options =
reader.closed,
() => {
readableClosed = true;
if (!isReadableEnded(duplex))
duplex.push(null);
},
(error) => {
writableClosed = true;
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-readable-from-web-enqueue-then-close.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';
const { mustCall } = require('../common');
const { Readable, Duplex } = require('stream');
const { strictEqual } = require('assert');

function start(controller) {
controller.enqueue(new Uint8Array(1));
controller.close();
}

Readable.fromWeb(new ReadableStream({ start }))
.on('data', mustCall((d) => {
strictEqual(d.length, 1);
}))
.on('end', mustCall())
.resume();

Duplex.fromWeb({
readable: new ReadableStream({ start }),
writable: new WritableStream({ write(chunk) {} })
})
.on('data', mustCall((d) => {
strictEqual(d.length, 1);
}))
.on('end', mustCall())
.resume();

0 comments on commit c514751

Please sign in to comment.