Re: [whatwg/streams] Unified error handling for WritableStream (#721)

domenic commented on this pull request.

Found some nits :)

> -  1. Perform ! WritableStreamRejectAbortRequestIfPending(_stream_).
-  1. Perform ! WritableStreamRejectPromisesInReactionToError(_stream_).
+  1. Assert: _stream_.[[storedError]] is *undefined*.
+  1. Assert: _stream_.[[state]] is `"writable"`.
+  1. Let _controller_ be _stream_.[[writableStreamController]].
+  1. Assert: _controller_ is not *undefined*.
+  1. Set _stream_.[[state]] to `"erroring"`.
+  1. Set _stream_.[[storedError]] to _reason_.
+  1. Let _writer_ be _stream_.[[writer]].
+  1. If _writer_ is not *undefined*, perform !
+     WritableStreamDefaultWriterEnsureReadyPromiseRejected(_writer_, _reason_).
+  1. If ! WritableStreamHasOperationMarkedInFlight(_stream_) is *false* and _controller_.[[started]] is *true*, perform
+     ! WritableStreamFinishError(_stream_).
+</emu-alg>
+
+<h4 id="writable-stream-finish-error" aoid="WritableStreamFinishError" nothrow>WritableStreamFinishError

Maybe the abstract ops should be named "WritableStreamStartErroring" and "WritableStreamFinishErroring"?

> +  1. Let _writer_ be _stream_.[[writer]].
+  1. If _writer_ is not *undefined*, perform !
+     WritableStreamDefaultWriterEnsureReadyPromiseRejected(_writer_, _reason_).
+  1. If ! WritableStreamHasOperationMarkedInFlight(_stream_) is *false* and _controller_.[[started]] is *true*, perform
+     ! WritableStreamFinishError(_stream_).
+</emu-alg>
+
+<h4 id="writable-stream-finish-error" aoid="WritableStreamFinishError" nothrow>WritableStreamFinishError
+( <var>stream</var> )</h4>
+
+<emu-alg>
+  1. Assert: _stream_.[[state]] is `"erroring"`.
+  1. Assert: ! WritableStreamHasOperationMarkedInFlight(_stream_) is *false*.
+  1. Set _stream_.[[state]] to `"errored"`.
+  1. Perform ! _stream_.[[writableStreamController]].[[ErrorSteps]]().
+  1. Let _storedError_ by _stream_.[[storedError]];

Semicolon should be period. "by" should be "be"

> +<h4 id="writable-stream-finish-error" aoid="WritableStreamFinishError" nothrow>WritableStreamFinishError
+( <var>stream</var> )</h4>
+
+<emu-alg>
+  1. Assert: _stream_.[[state]] is `"erroring"`.
+  1. Assert: ! WritableStreamHasOperationMarkedInFlight(_stream_) is *false*.
+  1. Set _stream_.[[state]] to `"errored"`.
+  1. Perform ! _stream_.[[writableStreamController]].[[ErrorSteps]]().
+  1. Let _storedError_ by _stream_.[[storedError]];
+  1. Repeat for each _writeRequest_ that is an element of _stream_.[[writeRequests]],
+    1. <a>Reject</a> _writeRequest_ with _storedError_.
+  1. Set _stream_.[[writeRequest]] to an empty List.
+  1. if _stream_.[[pendingAbortRequest]] is *undefined*,
+    1. Perform !  WritableStreamRejectCloseAndClosedPromiseIfNeeded(_stream_).
+    1. Return.
+  1. Let _abortRequest_ by _stream_.[[pendingAbortRequest]].

"by" should be "be". Might be worth running a regex for "Let ... by".

> +
+<emu-alg>
+  1. Assert: _stream_.[[state]] is `"erroring"`.
+  1. Assert: ! WritableStreamHasOperationMarkedInFlight(_stream_) is *false*.
+  1. Set _stream_.[[state]] to `"errored"`.
+  1. Perform ! _stream_.[[writableStreamController]].[[ErrorSteps]]().
+  1. Let _storedError_ by _stream_.[[storedError]];
+  1. Repeat for each _writeRequest_ that is an element of _stream_.[[writeRequests]],
+    1. <a>Reject</a> _writeRequest_ with _storedError_.
+  1. Set _stream_.[[writeRequest]] to an empty List.
+  1. if _stream_.[[pendingAbortRequest]] is *undefined*,
+    1. Perform !  WritableStreamRejectCloseAndClosedPromiseIfNeeded(_stream_).
+    1. Return.
+  1. Let _abortRequest_ by _stream_.[[pendingAbortRequest]].
+  1. Set _stream_.[[pendingAbortRequest]] to *undefined*.
+  1. Let _promise_ be stream.[[writableStreamController]].[[AbortSteps]](_abortRequest_.[[reason]]).

Missing ! (although I admit it's unclear whether these internal methods return completion values).

> +<emu-alg>
+  1. Assert: _stream_.[[state]] is `"erroring"`.
+  1. Assert: ! WritableStreamHasOperationMarkedInFlight(_stream_) is *false*.
+  1. Set _stream_.[[state]] to `"errored"`.
+  1. Perform ! _stream_.[[writableStreamController]].[[ErrorSteps]]().
+  1. Let _storedError_ by _stream_.[[storedError]];
+  1. Repeat for each _writeRequest_ that is an element of _stream_.[[writeRequests]],
+    1. <a>Reject</a> _writeRequest_ with _storedError_.
+  1. Set _stream_.[[writeRequest]] to an empty List.
+  1. if _stream_.[[pendingAbortRequest]] is *undefined*,
+    1. Perform !  WritableStreamRejectCloseAndClosedPromiseIfNeeded(_stream_).
+    1. Return.
+  1. Let _abortRequest_ by _stream_.[[pendingAbortRequest]].
+  1. Set _stream_.[[pendingAbortRequest]] to *undefined*.
+  1. Let _promise_ be stream.[[writableStreamController]].[[AbortSteps]](_abortRequest_.[[reason]]).
+    1. <a>Upon fulfillment</a> of _promise_, <a>resolve</a> _abortRequest_.[[promise]] with *undefined*.

We're a bit inconsistent but we seem to lean toward not indenting "Upon fulfillment" and "Upon rejection" steps.

>    1. Return ! WritableStreamDefaultWriterClose(_writer_).
 </emu-alg>
 
+<h4 id="writable-stream-default-writer-ensure-closed-promise-rejected"
+aoid="WritableStreamDefaultWriterEnsureClosedPromiseRejected"
+nothrow>WritableStreamDefaultWriterEnsureClosedPromiseRejected( <var>writer</var>, <var>error</var> )</h4>

It occurs to me we could abstract this out into a utility abstract op like EnsureStatePromiseRejected(promise, error). ("State" included to justify marking it as handled.)

Not sure if worth it.

> @@ -3582,10 +3568,13 @@ aoid="WritableStreamDefaultControllerAdvanceQueueIfNeeded" nothrow>WritableStrea
 
 <emu-alg>
   1. Let _stream_ be _controller_.[[controlledWritableStream]].
+  1. if _controller_.[[started]] is *false*, return.

Capitalize

-- 
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/721#pullrequestreview-30985292