MattiasBuelens / web-streams-polyfill

Web Streams, based on the WHATWG spec reference implementation
MIT License
289 stars 29 forks source link

`pipeTo` doesn't use `signal.reason` as error #115

Closed yume-chan closed 2 years ago

yume-chan commented 2 years ago

Repro:

import { ReadableStream, WritableStream } from 'web-streams-polyfill';

const abortController = new AbortController();
new ReadableStream().pipeTo(new WritableStream(), { signal: abortController.signal }).catch(e => console.log(e));
abortController.abort('Hello');

Expected (Microsoft Edge Version 102.0.1235.1 native implementation):

Hello

Actual (web-streams-polyfill@4.0.0.-beta.2):

e [AbortError]: Aborted

WPT in https://github.com/web-platform-tests/wpt/blob/73220c738c29bba700e608b235122613d5225b57/streams/piping/abort.any.js also fails:

piping/abort.any.html

  √ a signal argument 'null' should cause pipeTo() to reject
  √ a signal argument 'AbortSignal' should cause pipeTo() to reject
  √ a signal argument 'true' should cause pipeTo() to reject
  √ a signal argument '-1' should cause pipeTo() to reject
  √ a signal argument '[object AbortSignal]' should cause pipeTo() to reject
  √ an aborted signal should cause the writable stream to reject with an AbortError
  × (reason: 'null') all the error objects should be the same object (UNEXPECTED FAILURE)

    promise_rejects_exactly: pipeTo rejects with abort reason function "function() { throw e }" threw object "AbortError: Aborted" but we expected it to throw null
        at runMicrotasks (<anonymous>)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        at async Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:65:7)
  × (reason: 'undefined') all the error objects should be the same object (UNEXPECTED FAILURE)

    assert_equals: signal.reason should be error expected object "AbortError: Aborted" but got object "AbortError: The operation was aborted."
        at Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:72:5)
        at runMicrotasks (<anonymous>)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
  × (reason: 'error1: error1') all the error objects should be the same object (UNEXPECTED FAILURE)

    promise_rejects_exactly: pipeTo rejects with abort reason function "function() { throw e }" threw object "AbortError: Aborted" but we expected it to throw object "error1: error1"
        at runMicrotasks (<anonymous>)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        at async Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:65:7)
  √ preventCancel should prevent canceling the readable
  √ preventAbort should prevent aborting the readable
  √ preventCancel and preventAbort should prevent canceling the readable and aborting the readable
  × (reason: 'null') abort should prevent further reads (UNEXPECTED FAILURE)

    promise_rejects_exactly: pipeTo rejects with abort reason function "function() { throw e }" threw object "AbortError: Aborted" but we expected it to throw null
        at runMicrotasks (<anonymous>)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        at async Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:135:7)
  × (reason: 'undefined') abort should prevent further reads (UNEXPECTED FAILURE)

    assert_equals: signal.reason should be error expected object "AbortError: Aborted" but got object "AbortError: The operation was aborted."
        at Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:140:5)
        at runMicrotasks (<anonymous>)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
  × (reason: 'error1: error1') abort should prevent further reads (UNEXPECTED FAILURE)

    promise_rejects_exactly: pipeTo rejects with abort reason function "function() { throw e }" threw object "AbortError: Aborted" but we expected it to throw object "error1: error1"
        at runMicrotasks (<anonymous>)
        at processTicksAndRejections (node:internal/process/task_queues:96:5)
        at async Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:135:7)
  × (reason: 'null') all pending writes should complete on abort (UNEXPECTED FAILURE)

    promise_rejects_exactly: pipeTo rejects with abort reason function "function() { throw e }" threw object "AbortError: Aborted" but we expected it to throw null
        at async Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:174:7)
  × (reason: 'undefined') all pending writes should complete on abort (UNEXPECTED FAILURE)

    assert_equals: signal.reason should be error expected object "AbortError: Aborted" but got object "AbortError: The operation was aborted."
        at Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:179:5)
        at runNextTicks (node:internal/process/task_queues:61:5)
        at processTimers (node:internal/timers:497:9)
  × (reason: 'error1: error1') all pending writes should complete on abort (UNEXPECTED FAILURE)

    promise_rejects_exactly: pipeTo rejects with abort reason function "function() { throw e }" threw object "AbortError: Aborted" but we expected it to throw object "error1: error1"
        at async Test.<anonymous> (http://127.0.0.1:64298/streams/piping/abort.any.js:174:7)
  √ a rejection from underlyingSource.cancel() should be returned by pipeTo()
  √ a rejection from underlyingSink.abort() should be returned by pipeTo()
  √ a rejection from underlyingSink.abort() should be preferred to one from underlyingSource.cancel()
  √ abort signal takes priority over closed readable
  √ abort signal takes priority over errored readable
  √ abort signal takes priority over closed writable
  √ abort signal takes priority over errored writable
  √ abort should do nothing after the readable is closed
  √ abort should do nothing after the readable is errored
  √ abort should do nothing after the readable is errored, even with pending writes
  √ abort should do nothing after the writable is errored
MattiasBuelens commented 2 years ago

Thanks for reporting! The polyfill is indeed a little bit behind the latest specification. We currently test against https://github.com/web-platform-tests/wpt/commit/96ca25f0f7526282c0d47e6bf6a7edd439da1968 from October 2021, whereas AbortSignal.reason was integrated into pipeTo() in https://github.com/web-platform-tests/wpt/commit/b724cac4269298cfa57064bd5751e7e6e7593727 from November 2021.

I'll look into bringing us back up to date. 👍

MattiasBuelens commented 2 years ago

Fixed in #117. You can already give it a try in v4.0.0-beta.3.