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

tyoshino commented on this pull request.



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

Factor out L201-208 and call it from L191 and L192?

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

what's the reason why we can omit `if (shuttingDown == true`) here?

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

shouldn't we take the original error if any?

>        }
 
-      // 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) {

Can we describe the difference between this and performShutdown in the name?

performShutdownWithAction?

> @@ -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();

Shouldn't this case handled as an error as it's already closed when we tried to propagate the close signal?

-- 
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#pullrequestreview-4242170