Skip to content

Commit

Permalink
stream: fix setting abort reason in ReadableStream.pipeTo()
Browse files Browse the repository at this point in the history
In 14.2 in the specification, `error` should be signal’s abort reason.
The current behavior seems to assume that only an `AbortError` instance
is given as signal’s abort reason.

Refs: https://streams.spec.whatwg.org/#readable-stream-pipe-to

Signed-off-by: Daeyeon Jeong [email protected]
PR-URL: nodejs#44418
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
  • Loading branch information
daeyeon authored and Fyko committed Sep 15, 2022
1 parent 960b157 commit 76b4a24
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 11 deletions.
11 changes: 9 additions & 2 deletions lib/internal/webstreams/readablestream.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const {
} = primordials;

const {
AbortError,
codes: {
ERR_ILLEGAL_CONSTRUCTOR,
ERR_INVALID_ARG_VALUE,
Expand Down Expand Up @@ -1303,8 +1304,14 @@ function readableStreamPipeTo(
}

function abortAlgorithm() {
// Cannot use the AbortError class here. It must be a DOMException
const error = new DOMException('The operation was aborted', 'AbortError');
let error;
if (signal.reason instanceof AbortError) {
// Cannot use the AbortError class here. It must be a DOMException.
error = new DOMException(signal.reason.message, 'AbortError');
} else {
error = signal.reason;
}

const actions = [];
if (!preventAbort) {
ArrayPrototypePush(
Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-webstream-readablestream-pipeto.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Flags: --expose-internals
'use strict';

require('../common');
const assert = require('node:assert');
const { AbortError } = require('internal/errors');

// Purpose: pass an AbortError instance, which isn't the DOMException, as an
// abort reason.

for (const message of [undefined, 'abc']) {
const rs = new ReadableStream();
const ws = new WritableStream();
const ac = new AbortController();
const reason = new AbortError(message);
ac.abort(reason);

assert.rejects(rs.pipeTo(ws, { signal: ac.signal }), (e) => {
assert(e instanceof DOMException);
assert.strictEqual(e.name, 'AbortError');
assert.strictEqual(e.message, reason.message);
return true;
});
}
9 changes: 0 additions & 9 deletions test/wpt/status/streams.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,6 @@
"piping/abort.any.js": {
"fail": {
"expected": [
"(reason: 'null') all the error objects should be the same object",
"(reason: 'undefined') all the error objects should be the same object",
"(reason: 'error1: error1') all the error objects should be the same object",
"(reason: 'null') abort should prevent further reads",
"(reason: 'undefined') abort should prevent further reads",
"(reason: 'error1: error1') abort should prevent further reads",
"(reason: 'null') all pending writes should complete on abort",
"(reason: 'undefined') all pending writes should complete on abort",
"(reason: 'error1: error1') all pending writes should complete on abort",
"pipeTo on a teed readable byte stream should only be aborted when both branches are aborted"
]
}
Expand Down

0 comments on commit 76b4a24

Please sign in to comment.