Re: [whatwg/streams] Allow aborting an ongoing pipe operation using AbortSignals (#744)

ricea commented on this pull request.



>  </div>
 
 <emu-alg>
   1. If ! IsReadableStream(*this*) is *false*, return <a>a promise rejected with</a> a *TypeError* exception.
   1. If ! IsWritableStream(_dest_) is *false*, return <a>a promise rejected with</a> a *TypeError* exception.
   1. Set _preventClose_ to ! ToBoolean(_preventClose_), set _preventAbort_ to ! ToBoolean(_preventAbort_), and set
      _preventCancel_ to ! ToBoolean(_preventCancel_).
+  1. If _signal_ is not *undefined*, and _signal_ is not an instance of the {{AbortSignal}} interface, return <a>a

I'm concerned that this condition may not be strong enough to guarantee we can execute the [add](https://dom.spec.whatwg.org/#abortsignal-add) algorithm.

Maybe we could use [language similar to WebIDL](https://heycam.github.io/webidl/#es-interface), for example "and _signal_ is not a platform object or does not implement _AbortSignal_, then ..."

> @@ -791,6 +800,16 @@ option. If <code><a for="underlying source">type</a></code> is set to <code>unde
          1. If _preventCancel_ is *false*, <a href="#rs-pipeTo-shutdown-with-action">shutdown with an action</a> of !
             ReadableStreamCancel(*this*, _destClosed_) and with _destClosed_.
          1. Otherwise, <a href="#rs-pipeTo-shutdown">shutdown</a> with _destClosed_.
+     * <strong>Abort signals must stop activity:</strong> if _signal_ is not *undefined*, the following algorithm
+       _abortAlgorithm_ must be <a for="AbortSignal">added</a> to _signal_:
+         1. Let _error_ be a new "`<a idl>AbortError</a>`" `<a idl>DOMException</a>`.
+         1. Let _actions_ be an empty <a>ordered set</a>.
+         1. If _preventAbort_ is *false*, <a for="set">append</a> the action of performing !
+            WritableStreamAbort(_dest_, _error_) to _actions_.
+         1. If _preventCancel_ is *false*, <a for="set">append</a> the action of performing !
+            ReadableStreamCancel(*this*, _error_) to _actions_.
+         1. <a href="#rs-pipeTo-shutdown-with-action">Shutdown with an action</a> consisting of <a>waiting for all</a>

The [waiting for all](https://www.w3.org/2001/tag/doc/promises-guide#waiting-for-all) algorithm is problematic because it uses `Promise.all()`, which is very sensitive to changes to the global object.

We need to do something else, or change the definition of "waiting for all" in the promises guide.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/744#pullrequestreview-153270415