From 5baf48f526e23c44c4e5d3242b5c6db82f4fc635 Mon Sep 17 00:00:00 2001 From: Daeyeon Jeong Date: Sat, 27 Aug 2022 21:40:29 +0900 Subject: [PATCH] stream: fix setting abort reason in `ReadableStream.pipeTo()` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- lib/internal/webstreams/readablestream.js | 11 +++++++-- .../test-webstream-readablestream-pipeto.js | 24 +++++++++++++++++++ test/wpt/status/streams.json | 9 ------- 3 files changed, 33 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-webstream-readablestream-pipeto.js diff --git a/lib/internal/webstreams/readablestream.js b/lib/internal/webstreams/readablestream.js index a718e5404e94cb..4577d791d65229 100644 --- a/lib/internal/webstreams/readablestream.js +++ b/lib/internal/webstreams/readablestream.js @@ -28,6 +28,7 @@ const { } = primordials; const { + AbortError, codes: { ERR_ILLEGAL_CONSTRUCTOR, ERR_INVALID_ARG_VALUE, @@ -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( diff --git a/test/parallel/test-webstream-readablestream-pipeto.js b/test/parallel/test-webstream-readablestream-pipeto.js new file mode 100644 index 00000000000000..95929c5197a275 --- /dev/null +++ b/test/parallel/test-webstream-readablestream-pipeto.js @@ -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; + }); +} diff --git a/test/wpt/status/streams.json b/test/wpt/status/streams.json index 7f76e697a648a9..6f9a806a33abf1 100644 --- a/test/wpt/status/streams.json +++ b/test/wpt/status/streams.json @@ -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" ] }