From ff3c6983f2119403e2c52ec9064ffb9fb8ecc07a Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Thu, 8 Jun 2023 18:10:30 +0200 Subject: [PATCH 1/6] Streams: tests for `Transformer.cancel` --- streams/transform-streams/cancel.any.js | 92 +++++++++++++++++++++++++ streams/transform-streams/errors.any.js | 17 +++-- 2 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 streams/transform-streams/cancel.any.js diff --git a/streams/transform-streams/cancel.any.js b/streams/transform-streams/cancel.any.js new file mode 100644 index 00000000000000..8176119db9b17c --- /dev/null +++ b/streams/transform-streams/cancel.any.js @@ -0,0 +1,92 @@ +// META: global=window,worker +// META: script=../resources/test-utils.js +'use strict'; + +const thrownError = new Error('bad things are happening!'); +thrownError.name = 'error1'; + +promise_test(async t => { + let cancelled = undefined; + const ts = new TransformStream({ + cancel(reason) { + cancelled = reason; + } + }); + const res = await ts.readable.cancel(thrownError) + assert_equals(res, undefined, 'readable.cancel() should return undefined'); + assert_equals(cancelled, thrownError, 'transformer.cancel() should be called with the passed reason'); +}, 'cancelling the readable side should call transformer.cancel()'); + +promise_test(async t => { + const originalReason = new Error('original reason'); + const ts = new TransformStream({ + cancel(reason) { + assert_equals(reason, originalReason, 'transformer.cancel() should be called with the passed reason'); + throw thrownError; + } + }); + const writer = ts.writable.getWriter(); + const cancelPromise = ts.readable.cancel(originalReason) + await promise_rejects_exactly(t, thrownError, cancelPromise, 'readable.cancel() should reject with thrownError'); + await promise_rejects_exactly(t, originalReason, writer.closed, 'writer.closed should reject with original reason'); +}, 'cancelling the readable side should reject if transformer.cancel() throws'); + +promise_test(async t => { + let aborted = undefined; + const ts = new TransformStream({ + cancel(reason) { + aborted = reason; + }, + flush: t.unreached_func('flush should not be called') + }); + const res = await ts.writable.abort(thrownError) + assert_equals(res, undefined, 'writable.abort() should return undefined'); + assert_equals(aborted, thrownError, 'transformer.abort() should be called with the passed reason'); +}, 'aborting the writable side should call transformer.abort()'); + +promise_test(async t => { + const originalReason = new Error('original reason'); + const ts = new TransformStream({ + cancel(reason) { + assert_equals(reason, originalReason, 'transformer.cancel() should be called with the passed reason'); + throw thrownError; + }, + flush: t.unreached_func('flush should not be called') + }); + const reader = ts.readable.getReader(); + const abortPromise = ts.writable.abort(originalReason) + await promise_rejects_exactly(t, thrownError, abortPromise, 'writable.abort() should reject with thrownError'); + await promise_rejects_exactly(t, originalReason, reader.closed, 'reader.closed should reject with original reason'); +}, 'aborting the writable side should reject if transformer.cancel() throws'); + +promise_test(async t => { + const originalReason = new Error('original reason'); + const ts = new TransformStream({ + async cancel(reason) { + assert_equals(reason, originalReason, 'transformer.cancel() should be called with the passed reason'); + throw thrownError; + }, + flush: t.unreached_func('flush should not be called') + }); + const cancelPromise = ts.readable.cancel(originalReason); + const closePromise = ts.writable.close(); + await Promise.all([ + promise_rejects_exactly(t, thrownError, cancelPromise, 'cancelPromise should reject with thrownError'), + promise_rejects_exactly(t, originalReason, closePromise, 'closePromise should reject with thrownError'), + ]); +}, 'closing the writable side should reject if a parallel transformer.cancel() throws'); + +promise_test(async t => { + let flushed = false; + const ts = new TransformStream({ + flush() { + flushed = true; + }, + cancel: t.unreached_func('cancel should not be called') + }); + const closePromise = ts.writable.close(); + await delay(0); + const cancelPromise = ts.readable.cancel(thrownError); + await Promise.all([closePromise, cancelPromise]); + assert_equals(flushed, true, 'transformer.flush() should be called'); +}, 'closing the writable side should call transformer.flush() and a parallel readable.cancel() should not reject'); diff --git a/streams/transform-streams/errors.any.js b/streams/transform-streams/errors.any.js index 0cca4c75479d6d..a792c766839f06 100644 --- a/streams/transform-streams/errors.any.js +++ b/streams/transform-streams/errors.any.js @@ -9,7 +9,8 @@ promise_test(t => { const ts = new TransformStream({ transform() { throw thrownError; - } + }, + cancel: t.unreached_func('cancel should not be called') }); const reader = ts.readable.getReader(); @@ -34,7 +35,8 @@ promise_test(t => { }, flush() { throw thrownError; - } + }, + cancel: t.unreached_func('cancel should not be called') }); const reader = ts.readable.getReader(); @@ -54,13 +56,14 @@ promise_test(t => { ]); }, 'TransformStream errors thrown in flush put the writable and readable in an errored state'); -test(() => { +test(t => { new TransformStream({ start(c) { c.enqueue('a'); c.error(new Error('generic error')); assert_throws_js(TypeError, () => c.enqueue('b'), 'enqueue() should throw'); - } + }, + cancel: t.unreached_func('cancel should not be called') }); }, 'errored TransformStream should not enqueue new chunks'); @@ -72,7 +75,8 @@ promise_test(t => { }); }, transform: t.unreached_func('transform should not be called'), - flush: t.unreached_func('flush should not be called') + flush: t.unreached_func('flush should not be called'), + cancel: t.unreached_func('cancel should not be called') }); const writer = ts.writable.getWriter(); @@ -96,7 +100,8 @@ promise_test(t => { }); }, transform: t.unreached_func('transform should never be called if start() fails'), - flush: t.unreached_func('flush should never be called if start() fails') + flush: t.unreached_func('flush should never be called if start() fails'), + cancel: t.unreached_func('cancel should never be called if start() fails') }); const writer = ts.writable.getWriter(); From 41567169c2658d99482a75214a60c575808e7ef2 Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Mon, 12 Jun 2023 22:42:31 +0200 Subject: [PATCH 2/6] Version 2: run cancel before cancelling --- streams/transform-streams/cancel.any.js | 12 ++++---- streams/transform-streams/errors.any.js | 28 ++++++++++++++----- streams/transform-streams/flush.any.js | 4 +-- streams/transform-streams/general.any.js | 19 +++++++++++-- .../reentrant-strategies.any.js | 4 +++ 5 files changed, 50 insertions(+), 17 deletions(-) diff --git a/streams/transform-streams/cancel.any.js b/streams/transform-streams/cancel.any.js index 8176119db9b17c..8af5e1249b695f 100644 --- a/streams/transform-streams/cancel.any.js +++ b/streams/transform-streams/cancel.any.js @@ -5,6 +5,9 @@ const thrownError = new Error('bad things are happening!'); thrownError.name = 'error1'; +const originalReason = new Error('original reason'); +originalReason.name = 'error2'; + promise_test(async t => { let cancelled = undefined; const ts = new TransformStream({ @@ -18,7 +21,6 @@ promise_test(async t => { }, 'cancelling the readable side should call transformer.cancel()'); promise_test(async t => { - const originalReason = new Error('original reason'); const ts = new TransformStream({ cancel(reason) { assert_equals(reason, originalReason, 'transformer.cancel() should be called with the passed reason'); @@ -28,7 +30,7 @@ promise_test(async t => { const writer = ts.writable.getWriter(); const cancelPromise = ts.readable.cancel(originalReason) await promise_rejects_exactly(t, thrownError, cancelPromise, 'readable.cancel() should reject with thrownError'); - await promise_rejects_exactly(t, originalReason, writer.closed, 'writer.closed should reject with original reason'); + await promise_rejects_exactly(t, thrownError, writer.closed, 'writer.closed should reject with thrownError'); }, 'cancelling the readable side should reject if transformer.cancel() throws'); promise_test(async t => { @@ -45,7 +47,6 @@ promise_test(async t => { }, 'aborting the writable side should call transformer.abort()'); promise_test(async t => { - const originalReason = new Error('original reason'); const ts = new TransformStream({ cancel(reason) { assert_equals(reason, originalReason, 'transformer.cancel() should be called with the passed reason'); @@ -56,11 +57,10 @@ promise_test(async t => { const reader = ts.readable.getReader(); const abortPromise = ts.writable.abort(originalReason) await promise_rejects_exactly(t, thrownError, abortPromise, 'writable.abort() should reject with thrownError'); - await promise_rejects_exactly(t, originalReason, reader.closed, 'reader.closed should reject with original reason'); + await promise_rejects_exactly(t, thrownError, reader.closed, 'reader.closed should reject with thrownError'); }, 'aborting the writable side should reject if transformer.cancel() throws'); promise_test(async t => { - const originalReason = new Error('original reason'); const ts = new TransformStream({ async cancel(reason) { assert_equals(reason, originalReason, 'transformer.cancel() should be called with the passed reason'); @@ -72,7 +72,7 @@ promise_test(async t => { const closePromise = ts.writable.close(); await Promise.all([ promise_rejects_exactly(t, thrownError, cancelPromise, 'cancelPromise should reject with thrownError'), - promise_rejects_exactly(t, originalReason, closePromise, 'closePromise should reject with thrownError'), + promise_rejects_exactly(t, thrownError, closePromise, 'closePromise should reject with thrownError'), ]); }, 'closing the writable side should reject if a parallel transformer.cancel() throws'); diff --git a/streams/transform-streams/errors.any.js b/streams/transform-streams/errors.any.js index a792c766839f06..37ffbc7de8f2fb 100644 --- a/streams/transform-streams/errors.any.js +++ b/streams/transform-streams/errors.any.js @@ -203,13 +203,14 @@ promise_test(t => { // The microtask following transformer.start() hasn't completed yet, so the abort is queued and not notified to the // TransformStream yet. const abortPromise = writer.abort(thrownError); - const cancelPromise = ts.readable.cancel(new Error('cancel reason')); + const cancelPromise = ts.readable.cancel(new Error('cancel reason')) return Promise.all([ abortPromise, cancelPromise, - promise_rejects_exactly(t, thrownError, writer.closed, 'writer.closed should reject with thrownError')]); -}, 'abort should set the close reason for the writable when it happens before cancel during start, but cancel should ' + - 'still succeed'); + promise_rejects_exactly(t, thrownError, writer.closed, 'writer.closed should reject'), + ]); +}, 'abort should set the close reason for the writable when it happens before cancel during start, and cancel should ' + + 'reject'); promise_test(t => { let resolveTransform; @@ -256,13 +257,26 @@ promise_test(t => { controller = c; } }); - const cancelPromise = ts.readable.cancel(thrownError); - controller.error(ignoredError); + const cancelPromise = ts.readable.cancel(ignoredError); + controller.error(thrownError); return Promise.all([ cancelPromise, promise_rejects_exactly(t, thrownError, ts.writable.getWriter().closed, 'closed should reject with thrownError') ]); -}, 'controller.error() should do nothing after readable.cancel()'); +}, 'controller.error() should close writable immediately after readable.cancel()'); + +promise_test(t => { + let controller; + const ts = new TransformStream({ + start(c) { + controller = c; + } + }); + return ts.readable.cancel(thrownError).then(() => { + controller.error(ignoredError); + return promise_rejects_exactly(t, thrownError, ts.writable.getWriter().closed, 'closed should reject with thrownError'); + }); +}, 'controller.error() should do nothing after readable.cancel() resolves'); promise_test(t => { let controller; diff --git a/streams/transform-streams/flush.any.js b/streams/transform-streams/flush.any.js index 9287f6f5eb78eb..f52569ba32f5a0 100644 --- a/streams/transform-streams/flush.any.js +++ b/streams/transform-streams/flush.any.js @@ -127,5 +127,5 @@ promise_test(t => { controller.error(error1); } }); - return promise_rejects_exactly(t, error1, ts.writable.getWriter().close(), 'close() should reject'); -}, 'error() during flush should cause writer.close() to reject'); + return ts.writable.getWriter().close(); +}, 'error() during flush should not cause writer.close() to reject'); diff --git a/streams/transform-streams/general.any.js b/streams/transform-streams/general.any.js index c95691f7bf49df..dff2e7e8a70d72 100644 --- a/streams/transform-streams/general.any.js +++ b/streams/transform-streams/general.any.js @@ -388,9 +388,24 @@ promise_test(t => { controller.terminate(); return Promise.all([ cancelPromise, - promise_rejects_exactly(t, cancelReason, ts.writable.getWriter().closed, 'closed should reject with cancelReason') + promise_rejects_js(t, TypeError, ts.writable.getWriter().closed, 'closed should reject with TypeError') ]); -}, 'terminate() should do nothing after readable.cancel()'); +}, 'terminate() should abort writable immediately after readable.cancel()'); + +promise_test(t => { + let controller; + const ts = new TransformStream({ + start(c) { + controller = c; + } + }); + const cancelReason = { name: 'cancelReason' }; + return ts.readable.cancel(cancelReason).then(() => { + controller.terminate(); + return promise_rejects_exactly(t, cancelReason, ts.writable.getWriter().closed, 'closed should reject with TypeError'); + }) +}, 'terminate() should do nothing after readable.cancel() resolves'); + promise_test(() => { let calls = 0; diff --git a/streams/transform-streams/reentrant-strategies.any.js b/streams/transform-streams/reentrant-strategies.any.js index fc2f91886659f6..306cce8fc8b70b 100644 --- a/streams/transform-streams/reentrant-strategies.any.js +++ b/streams/transform-streams/reentrant-strategies.any.js @@ -314,6 +314,10 @@ promise_test(t => { // call to TransformStreamDefaultSink. return delay(0).then(() => { controller.enqueue('a'); + return reader.read(); + }).then(({ value, done }) => { + assert_false(done, 'done should be false'); + assert_equals(value, 'a', 'value should be correct'); return Promise.all([promise_rejects_exactly(t, error1, reader.read(), 'read() should reject'), abortPromise]); }); }, 'writer.abort() inside size() should work'); From 1f2f6c8912f414763e9ec177c8dfe316809a48c5 Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Mon, 17 Jul 2023 16:08:45 +0200 Subject: [PATCH 3/6] Fixes --- streams/transform-streams/cancel.any.js | 42 +++++++++++++++++++------ streams/transform-streams/flush.any.js | 19 +++++++++-- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/streams/transform-streams/cancel.any.js b/streams/transform-streams/cancel.any.js index 8af5e1249b695f..d28c40a886c15b 100644 --- a/streams/transform-streams/cancel.any.js +++ b/streams/transform-streams/cancel.any.js @@ -77,16 +77,40 @@ promise_test(async t => { }, 'closing the writable side should reject if a parallel transformer.cancel() throws'); promise_test(async t => { - let flushed = false; + let controller; const ts = new TransformStream({ - flush() { - flushed = true; + start(c) { + controller = c; }, - cancel: t.unreached_func('cancel should not be called') + async cancel(reason) { + assert_equals(reason, originalReason, 'transformer.cancel() should be called with the passed reason'); + controller.error(thrownError); + }, + flush: t.unreached_func('flush should not be called') }); + const cancelPromise = ts.readable.cancel(originalReason); const closePromise = ts.writable.close(); - await delay(0); - const cancelPromise = ts.readable.cancel(thrownError); - await Promise.all([closePromise, cancelPromise]); - assert_equals(flushed, true, 'transformer.flush() should be called'); -}, 'closing the writable side should call transformer.flush() and a parallel readable.cancel() should not reject'); + await Promise.all([ + promise_rejects_exactly(t, thrownError, cancelPromise, 'cancelPromise should reject with thrownError'), + promise_rejects_exactly(t, thrownError, closePromise, 'closePromise should reject with thrownError'), + ]); +}, 'readable.cancel() and a parallel writable.close() should reject if a transformer.cancel() calls controller.error()'); + +promise_test(async t => { + let controller; + const ts = new TransformStream({ + start(c) { + controller = c; + }, + async cancel(reason) { + assert_equals(reason, originalReason, 'transformer.cancel() should be called with the passed reason'); + controller.error(thrownError); + }, + flush: t.unreached_func('flush should not be called') + }); + const cancelPromise = ts.writable.abort(originalReason); + await promise_rejects_exactly(t, thrownError, cancelPromise, 'cancelPromise should reject with thrownError'); + const closePromise = ts.readable.cancel(1); + await promise_rejects_exactly(t, thrownError, closePromise, 'closePromise should reject with thrownError'); +}, 'writable.abort() and readable.cancel() should reject if a transformer.cancel() calls controller.error()'); + \ No newline at end of file diff --git a/streams/transform-streams/flush.any.js b/streams/transform-streams/flush.any.js index f52569ba32f5a0..487de1c93b0fd9 100644 --- a/streams/transform-streams/flush.any.js +++ b/streams/transform-streams/flush.any.js @@ -127,5 +127,20 @@ promise_test(t => { controller.error(error1); } }); - return ts.writable.getWriter().close(); -}, 'error() during flush should not cause writer.close() to reject'); + return promise_rejects_exactly(t, error1, ts.writable.getWriter().close(), 'close() should reject'); +}, 'error() during flush should cause writer.close() to reject'); + +promise_test(async t => { + let flushed = false; + const ts = new TransformStream({ + flush() { + flushed = true; + }, + cancel: t.unreached_func('cancel should not be called') + }); + const closePromise = ts.writable.close(); + await delay(0); + const cancelPromise = ts.readable.cancel(error1); + await Promise.all([closePromise, cancelPromise]); + assert_equals(flushed, true, 'transformer.flush() should be called'); +}, 'closing the writable side should call transformer.flush() and a parallel readable.cancel() should not reject'); From 1fbae6aea036486baff02271db92a1c24df978b2 Mon Sep 17 00:00:00 2001 From: Mattias Buelens Date: Mon, 17 Jul 2023 23:39:13 +0200 Subject: [PATCH 4/6] Add semicolons --- streams/transform-streams/cancel.any.js | 8 ++++---- streams/transform-streams/errors.any.js | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/streams/transform-streams/cancel.any.js b/streams/transform-streams/cancel.any.js index d28c40a886c15b..acec9ccc04be69 100644 --- a/streams/transform-streams/cancel.any.js +++ b/streams/transform-streams/cancel.any.js @@ -15,7 +15,7 @@ promise_test(async t => { cancelled = reason; } }); - const res = await ts.readable.cancel(thrownError) + const res = await ts.readable.cancel(thrownError); assert_equals(res, undefined, 'readable.cancel() should return undefined'); assert_equals(cancelled, thrownError, 'transformer.cancel() should be called with the passed reason'); }, 'cancelling the readable side should call transformer.cancel()'); @@ -28,7 +28,7 @@ promise_test(async t => { } }); const writer = ts.writable.getWriter(); - const cancelPromise = ts.readable.cancel(originalReason) + const cancelPromise = ts.readable.cancel(originalReason); await promise_rejects_exactly(t, thrownError, cancelPromise, 'readable.cancel() should reject with thrownError'); await promise_rejects_exactly(t, thrownError, writer.closed, 'writer.closed should reject with thrownError'); }, 'cancelling the readable side should reject if transformer.cancel() throws'); @@ -41,7 +41,7 @@ promise_test(async t => { }, flush: t.unreached_func('flush should not be called') }); - const res = await ts.writable.abort(thrownError) + const res = await ts.writable.abort(thrownError); assert_equals(res, undefined, 'writable.abort() should return undefined'); assert_equals(aborted, thrownError, 'transformer.abort() should be called with the passed reason'); }, 'aborting the writable side should call transformer.abort()'); @@ -55,7 +55,7 @@ promise_test(async t => { flush: t.unreached_func('flush should not be called') }); const reader = ts.readable.getReader(); - const abortPromise = ts.writable.abort(originalReason) + const abortPromise = ts.writable.abort(originalReason); await promise_rejects_exactly(t, thrownError, abortPromise, 'writable.abort() should reject with thrownError'); await promise_rejects_exactly(t, thrownError, reader.closed, 'reader.closed should reject with thrownError'); }, 'aborting the writable side should reject if transformer.cancel() throws'); diff --git a/streams/transform-streams/errors.any.js b/streams/transform-streams/errors.any.js index 37ffbc7de8f2fb..7efe894f4887fb 100644 --- a/streams/transform-streams/errors.any.js +++ b/streams/transform-streams/errors.any.js @@ -203,7 +203,7 @@ promise_test(t => { // The microtask following transformer.start() hasn't completed yet, so the abort is queued and not notified to the // TransformStream yet. const abortPromise = writer.abort(thrownError); - const cancelPromise = ts.readable.cancel(new Error('cancel reason')) + const cancelPromise = ts.readable.cancel(new Error('cancel reason')); return Promise.all([ abortPromise, cancelPromise, From 9fa9a0c40d5aff6b45f0aa47759b17705ce5f6f2 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Fri, 29 Sep 2023 13:52:56 +0900 Subject: [PATCH 5/6] Add shadowrealm --- streams/transform-streams/cancel.any.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/streams/transform-streams/cancel.any.js b/streams/transform-streams/cancel.any.js index acec9ccc04be69..bcb26838e5f3fc 100644 --- a/streams/transform-streams/cancel.any.js +++ b/streams/transform-streams/cancel.any.js @@ -1,4 +1,4 @@ -// META: global=window,worker +// META: global=window,worker,shadowrealm // META: script=../resources/test-utils.js 'use strict'; From ab3f840bab5a145a9c1237615ece524739810427 Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Fri, 29 Sep 2023 16:09:02 +0900 Subject: [PATCH 6/6] lint --- streams/transform-streams/cancel.any.js | 1 - 1 file changed, 1 deletion(-) diff --git a/streams/transform-streams/cancel.any.js b/streams/transform-streams/cancel.any.js index bcb26838e5f3fc..5c7fc4eae5d55b 100644 --- a/streams/transform-streams/cancel.any.js +++ b/streams/transform-streams/cancel.any.js @@ -113,4 +113,3 @@ promise_test(async t => { const closePromise = ts.readable.cancel(1); await promise_rejects_exactly(t, thrownError, closePromise, 'closePromise should reject with thrownError'); }, 'writable.abort() and readable.cancel() should reject if a transformer.cancel() calls controller.error()'); - \ No newline at end of file