Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stream: fix setting abort reason in ReadableStream.pipeTo() #44418

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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