Re: [whatwg/streams] TransformStream cleanup using "Transformer.cancel" (PR #1283)

@MattiasBuelens requested changes on this pull request.



> +<div algorithm>
+ <dfn abstract-op lt="TransformStreamUnblockWrite"

Nit-pick: these abstract ops are ordered alphabetically, so this new one should go after `TransformStreamSetBackpressure`.

> + 1. Let |cancelAlgorithmWrapper| be an algorithm that runs these steps given a value |reason|:
+  1. Let |result| be the result of running |cancelAlgorithm| given |reason|, if |cancelAlgorithm|
+     was given, or null otherwise. If this throws an exception |e|, return
+     [=a promise rejected with=] |e|.

The handling of `result` is missing, like we do in `transformAlgorithmWrapper` and `flushAlgorithmWrapper`:
```suggestion
 1. Let |cancelAlgorithmWrapper| be an algorithm that runs these steps given a value |reason|:
  1. Let |result| be the result of running |cancelAlgorithm| given |reason|, if |cancelAlgorithm|
     was given, or null otherwise. If this throws an exception |e|, return
     [=a promise rejected with=] |e|.
  1. If |result| is a {{Promise}}, then return |result|.
  1. Return [=a promise resolved with=] undefined.
```

> @@ -221,32 +231,61 @@ function TransformStreamDefaultSinkWriteAlgorithm(stream, chunk) {
 }
 
 function TransformStreamDefaultSinkAbortAlgorithm(stream, reason) {
-  // abort() is not called synchronously, so it is possible for abort() to be called when the stream is already
-  // errored.
-  TransformStreamError(stream, reason);
-  return promiseResolvedWith(undefined);
+  verbose('TransformStreamDefaultSinkAbortAlgorithm()');
+
+  const controller = stream._controller;
+  if (controller._finishPromise) {

```suggestion
  if (controller._finishPromise !== undefined) {
```
We try to be very explicit in the reference implementation. 😉

>  }
 
 function TransformStreamDefaultSinkCloseAlgorithm(stream) {
   verbose('TransformStreamDefaultSinkCloseAlgorithm()');
 
+  const controller = stream._controller;
+  if (controller._finishPromise) {

```suggestion
  if (controller._finishPromise !== undefined) {
```

> @@ -264,3 +303,35 @@ function TransformStreamDefaultSourcePullAlgorithm(stream) {
   // Prevent the next pull() call until there is backpressure.
   return stream._backpressureChangePromise;
 }
+
+function TransformStreamDefaultSourceCancelAlgorithm(stream, reason) {
+  verbose('TransformStreamDefaultSourceCancelAlgorithm()');
+
+  const controller = stream._controller;
+  if (controller._finishPromise) {

```suggestion
  if (controller._finishPromise !== undefined) {
```

> -   1. If |readable|.[=ReadableStream/[[state]]=] is "`errored`", throw
-      |readable|.[=ReadableStream/[[storedError]]=].

Why was this check removed?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/streams/pull/1283#pullrequestreview-1531754015
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/streams/pull/1283/review/1531754015@github.com>

Received on Sunday, 16 July 2023 14:59:49 UTC