Re: [whatwg/streams] Update TransformStream API & misc. fixups (#519)

tyoshino commented on this pull request.

Thanks for the patch

> -    } else {
-      try {
-        transformStream._transformer.flush(
-            transformStream._enqueueFunction,
-            transformStream._closeFunction,
-            transformStream._errorFunction);
-      } catch (e) {
-        if (transformStream._errored === false) {
-          TransformStreamErrorInternal(transformStream, e);
-          throw e;
-        }
-      }
-    }
+    const flushPromise = PromiseInvokeOrNoop(transformStream._transformer, 'flush', [transformStream._controller]);
+    // Return a promise that is fulfilled with undefined on success.
+    return flushPromise.then(() => TransformStreamCloseReadableInternal(transformStream),

What if controller.error() is invoked inside flush() but flush() resolved? I think this path should perform TransformStreamCloseReadableInternal() when the transform stream is neither errored nor closed.

> @@ -16,6 +17,17 @@ function TransformStreamCloseReadable(transformStream) {
     throw new TypeError('Readable side is already closed');
   }
 
+  if (transformStream._writableDone === true) {
+    throw new TypeError('TransformStream is already closing');
+  }

Did you try to limit the way to close the readable side to only by resolving flush() once flush() is invoked?  What's the motivation for that?

-- 
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/519#pullrequestreview-691206