Add TransformStreamDefaultController terminate() method #818
Conversation
Replace close() with terminate(). In addition to closing the readable side, terminate() also errors the writable side. This stops data from being produced after we are no longer interested in it. Modify existing tests to handle the new method, and add tests for the new functionality. Closes whatwg#774.
| }); | ||
| 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') |
There was a problem hiding this comment.
Should we test more than .cancel() to test if it's errored properly?
There was a problem hiding this comment.
.cancel() is actually pretty specific. I added reader.closed to the tests just to be sure. I don't want to get into testing Readablestream itself here.
| new TransformStream({ | ||
| start(controller) { | ||
| controller.terminate(); | ||
| assert_throws(new TypeError(), () => controller.error(error1), 'error() should throw'); |
There was a problem hiding this comment.
Opened #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?
There was a problem hiding this comment.
Thanks for bringing this up. I had forgotten about it actually.
I filed #822 to track the changes to transform stream.
Replace close() with terminate(). In addition to closing the readable
side, terminate() also errors the writable side. This stops data from
being produced after we are no longer interested in it.
Modify existing tests to handle the new method, and add tests for the new
functionality.
Closes #774.