Re: [whatwg/streams] Add TransformStreamDefaultController terminate() method (#818)

domenic approved this pull request.

I think this looks good, but it brought up some larger questions for me about throwing exceptions from controller methods. Maybe you have that all in hand already and I just need to read the code more closely...

> +}, 'controller.enqueue() should throw after controller.terminate()');
+
+const error1 = new Error('error1');
+error1.name = 'error1';
+
+promise_test(t => {
+  const ts = new TransformStream({
+    start(controller) {
+      controller.enqueue(0);
+      controller.terminate();
+      controller.error(error1);
+    }
+  });
+  return Promise.all([
+    promise_rejects(t, new TypeError(), ts.writable.abort(), 'abort() should reject with a TypeError'),
+    promise_rejects(t, error1, ts.readable.cancel(), 'cancel() should reject with error1')

Should we test more than .cancel() to test if it's errored properly?

> +      controller.enqueue(0);
+      controller.terminate();
+      controller.error(error1);
+    }
+  });
+  return Promise.all([
+    promise_rejects(t, new TypeError(), ts.writable.abort(), 'abort() should reject with a TypeError'),
+    promise_rejects(t, error1, ts.readable.cancel(), 'cancel() should reject with error1')
+  ]);
+}, 'controller.error() after controller.terminate() with queued chunk should error the readable');
+
+test(() => {
+  new TransformStream({
+    start(controller) {
+      controller.terminate();
+      assert_throws(new TypeError(), () => controller.error(error1), 'error() should throw');

Opened https://github.com/whatwg/streams/issues/821.

In general, have we tested/are we OK with the behavior for when controller methods throw? I think a guiding principle is that you shouldn't throw if it's not "your fault", e.g. if someone using the public API of the stream can transition the state in some way as to cause an exception. Does that sound familiar from previous discussions? Are we following it here, e.g. if the writable side is aborted or the readable side canceled?

-- 
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/818#pullrequestreview-66561827