Skip to content

Commit 2facf8b

Browse files
daeyeonRafaelGSS
authored andcommitted
stream: fix setting abort reason in ReadableStream.pipeTo()
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 daeyeon.dev@gmail.com PR-URL: #44418 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 5cefd02 commit 2facf8b

File tree

3 files changed

+33
-11
lines changed

3 files changed

+33
-11
lines changed

lib/internal/webstreams/readablestream.js

+9-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ const {
2828
} = primordials;
2929

3030
const {
31+
AbortError,
3132
codes: {
3233
ERR_ILLEGAL_CONSTRUCTOR,
3334
ERR_INVALID_ARG_VALUE,
@@ -1303,8 +1304,14 @@ function readableStreamPipeTo(
13031304
}
13041305

13051306
function abortAlgorithm() {
1306-
// Cannot use the AbortError class here. It must be a DOMException
1307-
const error = new DOMException('The operation was aborted', 'AbortError');
1307+
let error;
1308+
if (signal.reason instanceof AbortError) {
1309+
// Cannot use the AbortError class here. It must be a DOMException.
1310+
error = new DOMException(signal.reason.message, 'AbortError');
1311+
} else {
1312+
error = signal.reason;
1313+
}
1314+
13081315
const actions = [];
13091316
if (!preventAbort) {
13101317
ArrayPrototypePush(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
require('../common');
5+
const assert = require('node:assert');
6+
const { AbortError } = require('internal/errors');
7+
8+
// Purpose: pass an AbortError instance, which isn't the DOMException, as an
9+
// abort reason.
10+
11+
for (const message of [undefined, 'abc']) {
12+
const rs = new ReadableStream();
13+
const ws = new WritableStream();
14+
const ac = new AbortController();
15+
const reason = new AbortError(message);
16+
ac.abort(reason);
17+
18+
assert.rejects(rs.pipeTo(ws, { signal: ac.signal }), (e) => {
19+
assert(e instanceof DOMException);
20+
assert.strictEqual(e.name, 'AbortError');
21+
assert.strictEqual(e.message, reason.message);
22+
return true;
23+
});
24+
}

test/wpt/status/streams.json

-9
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,6 @@
22
"piping/abort.any.js": {
33
"fail": {
44
"expected": [
5-
"(reason: 'null') all the error objects should be the same object",
6-
"(reason: 'undefined') all the error objects should be the same object",
7-
"(reason: 'error1: error1') all the error objects should be the same object",
8-
"(reason: 'null') abort should prevent further reads",
9-
"(reason: 'undefined') abort should prevent further reads",
10-
"(reason: 'error1: error1') abort should prevent further reads",
11-
"(reason: 'null') all pending writes should complete on abort",
12-
"(reason: 'undefined') all pending writes should complete on abort",
13-
"(reason: 'error1: error1') all pending writes should complete on abort",
145
"pipeTo on a teed readable byte stream should only be aborted when both branches are aborted"
156
]
167
}

0 commit comments

Comments
 (0)