From 55205af870f7de98062a554b51b52bd0f1c7cd50 Mon Sep 17 00:00:00 2001 From: Adam Rice Date: Thu, 5 Oct 2017 01:06:33 +0900 Subject: [PATCH] Make error() and terminate() do nothing when stream is errored Previously error() and terminate() would throw an exception if the readable was closed, errored or pending close. This wasn't useful, so make them just do nothing in those cases instead. Closes #822. --- .../lib/transform-stream.js | 12 ++---- .../transform-streams/errors.js | 40 ++++++++++++------- .../transform-streams/general.js | 18 +++++---- .../transform-streams/terminate.js | 13 ++++-- 4 files changed, 48 insertions(+), 35 deletions(-) diff --git a/reference-implementation/lib/transform-stream.js b/reference-implementation/lib/transform-stream.js index f1f9a0cf3..14f5e43ce 100644 --- a/reference-implementation/lib/transform-stream.js +++ b/reference-implementation/lib/transform-stream.js @@ -152,10 +152,6 @@ class TransformStreamDefaultController { throw defaultControllerBrandCheckException('error'); } - if (this._controlledTransformStream._readable._state !== 'readable') { - throw new TypeError('TransformStream is not in a state that can be errored'); - } - TransformStreamDefaultControllerError(this, reason); } } @@ -179,11 +175,11 @@ function TransformStreamDefaultControllerTerminate(controller) { const stream = controller._controlledTransformStream; const readableController = stream._readable._readableStreamController; - if (ReadableStreamDefaultControllerCanCloseOrEnqueue(readableController) === false) { - throw new TypeError('Readable side is not in a state that can be closed'); + + if (ReadableStreamDefaultControllerCanCloseOrEnqueue(readableController) === true) { + ReadableStreamDefaultControllerClose(readableController); } - ReadableStreamDefaultControllerClose(readableController); WritableStreamDefaultControllerErrorIfNeeded(stream._writable._writableStreamController, new TypeError('TransformStream terminated')); if (stream._backpressure === true) { @@ -223,8 +219,6 @@ function TransformStreamDefaultControllerEnqueue(controller, chunk) { function TransformStreamDefaultControllerError(controller, e) { const stream = controller._controlledTransformStream; - assert(stream._readable._state === 'readable', 'stream.[[readable]].[[state]] is "readable"'); - TransformStreamError(stream, e); } diff --git a/reference-implementation/to-upstream-wpts/transform-streams/errors.js b/reference-implementation/to-upstream-wpts/transform-streams/errors.js index 0e691d7b4..83f4984c5 100644 --- a/reference-implementation/to-upstream-wpts/transform-streams/errors.js +++ b/reference-implementation/to-upstream-wpts/transform-streams/errors.js @@ -234,28 +234,35 @@ promise_test(t => { }, 'abort should set the close reason for the writable when it happens before cancel during underlying sink write, ' + 'but cancel should still succeed'); -test(() => { - new TransformStream({ +const ignoredError = new Error('ignoredError'); +ignoredError.name = 'ignoredError'; + +promise_test(t => { + const ts = new TransformStream({ start(controller) { controller.error(thrownError); - assert_throws(new TypeError(), () => controller.error(), 'error() should throw'); + controller.error(ignoredError); } }); -}, 'controller.error() should throw the second time it is called'); + return promise_rejects(t, thrownError, ts.writable.abort(), 'abort() should reject with thrownError'); +}, 'controller.error() should do nothing the second time it is called'); -promise_test(() => { +promise_test(t => { let controller; const ts = new TransformStream({ start(c) { controller = c; } }); - const cancelPromise = ts.readable.cancel(); - assert_throws(new TypeError(), () => controller.error(), 'error() should throw'); - return cancelPromise; -}, 'controller.error() should throw after readable.cancel() but the cancel should still succeed'); + const cancelPromise = ts.readable.cancel(thrownError); + controller.error(ignoredError); + return Promise.all([ + cancelPromise, + promise_rejects(t, thrownError, ts.writable.getWriter().closed, 'closed should reject with thrownError') + ]); +}, 'controller.error() should do nothing after readable.cancel()'); -promise_test(() => { +promise_test(t => { let controller; const ts = new TransformStream({ start(c) { @@ -263,9 +270,10 @@ promise_test(() => { } }); return ts.writable.abort().then(() => { - assert_throws(new TypeError(), () => controller.error(), 'error() should throw'); + controller.error(ignoredError); + return promise_rejects(t, new TypeError(), ts.writable.getWriter().closed, 'closed should reject with a TypeError'); }); -}, 'controller.error() should throw after writable.abort() has completed'); +}, 'controller.error() should do nothing after writable.abort() has completed'); promise_test(t => { let controller; @@ -277,10 +285,12 @@ promise_test(t => { throw thrownError; } }, undefined, { highWaterMark: Infinity }); - return promise_rejects(t, thrownError, ts.writable.getWriter().write(), 'write() should reject').then(() => { - assert_throws(new TypeError(), () => controller.error(), 'error() should throw'); + const writer = ts.writable.getWriter(); + return promise_rejects(t, thrownError, writer.write(), 'write() should reject').then(() => { + controller.error(); + return promise_rejects(t, thrownError, writer.closed, 'closed should reject with thrownError'); }); -}, 'controller.error() should throw after a transformer method has thrown an exception'); +}, 'controller.error() should do nothing after a transformer method has thrown an exception'); promise_test(t => { let controller; diff --git a/reference-implementation/to-upstream-wpts/transform-streams/general.js b/reference-implementation/to-upstream-wpts/transform-streams/general.js index 8ad427ad7..028e270c1 100644 --- a/reference-implementation/to-upstream-wpts/transform-streams/general.js +++ b/reference-implementation/to-upstream-wpts/transform-streams/general.js @@ -398,22 +398,26 @@ test(() => { new TransformStream({ start(controller) { controller.terminate(); - assert_throws(new TypeError(), () => controller.terminate(), 'terminate should throw'); + controller.terminate(); } }); -}, 'controller.terminate() should throw the second time it is called'); +}, 'controller.terminate() should do nothing the second time it is called'); -promise_test(() => { +promise_test(t => { let controller; const ts = new TransformStream({ start(c) { controller = c; } }); - const cancelPromise = ts.readable.cancel(); - assert_throws(new TypeError(), () => controller.terminate(), 'terminate should throw'); - return cancelPromise; -}, 'terminate() should throw after readable.cancel()'); + const cancelReason = { name: 'cancelReason' }; + const cancelPromise = ts.readable.cancel(cancelReason); + controller.terminate(); + return Promise.all([ + cancelPromise, + promise_rejects(t, cancelReason, ts.writable.getWriter().closed, 'closed should reject with cancelReason') + ]); +}, 'terminate() should do nothing after readable.cancel()'); promise_test(() => { let calls = 0; diff --git a/reference-implementation/to-upstream-wpts/transform-streams/terminate.js b/reference-implementation/to-upstream-wpts/transform-streams/terminate.js index b60762e66..e0e1b8788 100644 --- a/reference-implementation/to-upstream-wpts/transform-streams/terminate.js +++ b/reference-implementation/to-upstream-wpts/transform-streams/terminate.js @@ -74,13 +74,18 @@ promise_test(t => { ]); }, 'controller.error() after controller.terminate() with queued chunk should error the readable'); -test(() => { - new TransformStream({ +promise_test(t => { + const ts = new TransformStream({ start(controller) { controller.terminate(); - assert_throws(new TypeError(), () => controller.error(error1), 'error() should throw'); + controller.error(error1); } }); -}, 'controller.error() after controller.terminate() without queued chunk should throw'); + return Promise.all([ + promise_rejects(t, new TypeError(), ts.writable.abort(), 'abort() should reject with a TypeError'), + ts.readable.cancel(), + ts.readable.getReader().closed + ]); +}, 'controller.error() after controller.terminate() without queued chunk should do nothing'); done();