Re: [whatwg/streams] Rigorously specify and test pipeTo (#512)

domenic commented on this pull request.



>        }
 
-      // We don't use writer.closed. pipeTo() is responsible only for what it has written.
-      _lastWrite.then(
-        () => {
-          releaseWriter();
-          finishWithFulfillment();
-        },
-        reason => {
-          releaseWriter();
-          finishWithRejection(reason);
+      function performAsyncShutdown(action, originalIsError, originalError) {

Sure, will rename.

>          }
-      );
-      _state = 'waitingLastWriteOnReadableClosed';
-    }
-
-    function doPipe() {
-      // console.log('pipeTo(): doPipe()');
+        shuttingDown = true;
+
+        waitForCurrentWrite().then(() => {
+          action().then(
+            () => performShutdown(originalIsError, originalError),
+            newError => performShutdown(true, newError)

Hmm. An interesting question, but I don't think so. This way makes sure both errors are seen by someone. The original error is seen by the action (abort or cancel) and then the error thrown (or propagated via a rejected promise) from the action is seen by the caller of pipeTo. I think that is best.

>  
-      _lastRead = _reader.read();
+      function performShutdown(isError, error) {
+        shuttingDown = true;

_shuttingDown_ might be true here, if we are coming from async shutdown. So that would be expected, not surprising.

_shuttingDown_ is basically meant to prevent a second async shutdown from happening if one is already in progress.

>  
-      Promise.all([_lastRead, _writer.ready]).then(
-        ([{ value, done }]) => {
-          if (_state !== 'piping') {
-            return;
-          }
+        waitForCurrentWrite().then(() => {
+          WritableStreamDefaultWriterRelease(writer);

Done

> @@ -359,6 +370,24 @@ function WritableStreamDefaultWriterClose(writer) {
   return promise;
 }
 
+
+function WritableStreamDefaultWriterCloseWithErrorPropagation(writer) {
+  const stream = writer._ownerWritableStream;
+
+  assert(stream !== undefined);
+
+  const state = stream._state;
+  if (state === 'closed' || state === 'closing') {
+    return Promise.resolve();

Great catch. This is never hit, by design, so I changed it to an assert

-- 
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/512