Re: [whatwg/streams] WritableStream abort logic clean up (#655)

[STILL DRAFT]

Bug fix:

- Fixed `sink.close()` rejection handling logic to reject closed even if the stream has been aborted or errored by `controller.error()`
  - Before this fix, `WritableStreamDefaultControllerErrorIfNeeded()` was used which is no-op when the state is set to `"errored"`

Changes:

- Now `writer.abort()` doesn't set the `[[state]]` of the stream to `"errored"` when there's pending `sink.write()` or `sink.close()` but prevents further operations on writer by presence of the `[[pendingAbortRequest]]`.
  - `writer.ready` rejects immediately
  - Now `controller.error()` succeeds even after writer.abort() is called. So, `writer.abort()` and `writer.closed` is rejected with the error given to the `controller.error()` while `writer.ready` is kept rejected with an error indicating abort.
  - Now pending `writer.abort()` rejects with `[[storedError]]` if it's been errored with `controller.error()`
- `sink.close()` rejection doesn't change the rejection reason of `writer.ready` if it's already rejected due to `writer.abort()` or `controller.error()`
  - Q: Is this ok? When a writer is reobtained, the ready promise is set to be rejected with the latest error.
- Changed `sink.write()` rejection to set `[[storedError]]` to the rejection reason of `sink.write()` only when there wasn't `controller.error()` call

Changes (fulfillment/rejection order):

- Changed the order of promise rejection on `sink.write()` completion. Now `writer.abort()` then `writer.closed`.
- Rejection order on `sink.write()` completion:
  - Before: write(), closed, abort()
  - After: write(), abort(), closed
- Rejection order on `sink.write()` rejection
  - Before: write(), closed, abort(), ready
  - After: write(), abort(), ready, closed
  - Q: Worse?
- `controller.error()` rejects:
  - Before: 
  - After: ready then closed

Refactoring:

- Refactored abort() finalization steps to clarify where a certain process is scheduled to run
- Factored out `[[pendingWriteRequest]]` setting logic as a WritableStream abstract operation exposed to controllers
- Removed redundant `[[queue]]` clearing from sink write rejection handling logic
- Factored out:
  - `WritableStreamDefaultControllerUpdateBackpressureIfNeeded()`
  - `WritableStreamRejectClosedPromiseIfAny()`
  - `WritableStreamDefaultWriterEnsureReadyPromiseRejectedWith()`
- Call what's needed directly in `sink.write()` rejection than using `WritableStreamDefaultControllerErrorIfNeeded()`
- Finish calculating conditions at the top of methods to avoid thinking about where a certain state has been updated or not yet (e.g. `[[error]]`)
- Have redundant call to promise rejection helper method if needed to keep rejection order consistent

-- 
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/655#issuecomment-274404928

Received on Monday, 23 January 2017 05:42:54 UTC