From c8dea480cad8a367841fe4a60fb011e6586459b6 Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 3 Dec 2024 13:19:49 +0100 Subject: [PATCH 1/9] fix: http_server_native to close ReadableStream on abort signal --- http_server_native.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/http_server_native.ts b/http_server_native.ts index 34984060..216f731e 100644 --- a/http_server_native.ts +++ b/http_server_native.ts @@ -96,6 +96,9 @@ export class Server> signal, ...options, }); + // closinng stream, so that the Application listen promise can resolve itself + // https://developer.mozilla.org/en-US/docs/Web/API/ReadableStreamDefaultController/close + signal?.addEventListener("abort", () => controller.close(), { once: true }); }, }); From a05a85b677eb01256667dfa40761e64f6153248d Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 3 Dec 2024 13:19:49 +0100 Subject: [PATCH 2/9] chore: throw if aborted prematurely before listen() is invoked --- http_server_native.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/http_server_native.ts b/http_server_native.ts index 216f731e..2e49f6ed 100644 --- a/http_server_native.ts +++ b/http_server_native.ts @@ -79,6 +79,10 @@ export class Server> const { signal } = this.#options; const { onListen, ...options } = this.#options; const { promise, resolve } = createPromiseWithResolvers(); + if (signal?.aborted) { + // if user somehow aborted before `listen` is invoked, we throw + return Promise.reject(new Error("aborted prematurely before 'listen' event")); + } this.#stream = new ReadableStream({ start: (controller) => { this.#httpServer = serve?.({ From 19f37f7492aea26c03aaa0559afdf05e04d71915 Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 3 Dec 2024 13:19:49 +0100 Subject: [PATCH 3/9] chore: updated test suite --- application.test.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/application.test.ts b/application.test.ts index 16ed7dd0..bcf3865b 100644 --- a/application.test.ts +++ b/application.test.ts @@ -1105,10 +1105,8 @@ Deno.test({ }); Deno.test({ - name: "Application.listen() - no options", + name: "Application.listen() - no options - aborted before onListen", // ignore: isNode(), - ignore: true, // there is a challenge with serve and the abort controller that - // needs to be isolated async fn() { const controller = new AbortController(); const app = new Application(); @@ -1118,6 +1116,22 @@ Deno.test({ const { signal } = controller; const p = app.listen({ signal }); controller.abort(); + assertRejects(async () => await p, "aborted prematurely before 'listen' event"); + teardown(); + }, +}); + +Deno.test({ + name: "Application.listen() - no options - aborted after onListen", + async fn() { + const controller = new AbortController(); + const app = new Application(); + app.use((ctx) => { + ctx.response.body = "hello world"; + }); + const { signal } = controller; + app.addEventListener("listen", () => controller.abort()) + const p = app.listen({ signal }); await p; teardown(); }, From 8d7104e1a977f44a5b78bcbd6876f2da33899809 Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 3 Dec 2024 19:59:10 +0100 Subject: [PATCH 4/9] chore: more explicit logic in test --- application.test.ts | 28 ++++++++++++++++++++++++---- http_server_native.ts | 8 ++++++-- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/application.test.ts b/application.test.ts index bcf3865b..499246d5 100644 --- a/application.test.ts +++ b/application.test.ts @@ -1116,7 +1116,10 @@ Deno.test({ const { signal } = controller; const p = app.listen({ signal }); controller.abort(); - assertRejects(async () => await p, "aborted prematurely before 'listen' event"); + assertRejects( + async () => await p, + "aborted prematurely before 'listen' event", + ); teardown(); }, }); @@ -1130,10 +1133,27 @@ Deno.test({ ctx.response.body = "hello world"; }); const { signal } = controller; - app.addEventListener("listen", () => controller.abort()) const p = app.listen({ signal }); - await p; - teardown(); + app.addEventListener("listen", async () => controller.abort()); + const GRACEFUL_TIME = 1000; + let timer: number | undefined; + const raceResult = await Promise.race([ + new Promise(async (resolve) => { + await p; + clearTimeout(timer); + resolve("resolved cleanly"); + }), + new Promise((resolve) => + timer = setTimeout( + () => resolve("likely forever pending"), + GRACEFUL_TIME, + ) + ), + ]); + assert( + raceResult === "resolved cleanly", + `'listen promise' should resolve before ${GRACEFUL_TIME} ms`, + ); }, }); diff --git a/http_server_native.ts b/http_server_native.ts index 2e49f6ed..1bb24e0a 100644 --- a/http_server_native.ts +++ b/http_server_native.ts @@ -81,7 +81,9 @@ export class Server> const { promise, resolve } = createPromiseWithResolvers(); if (signal?.aborted) { // if user somehow aborted before `listen` is invoked, we throw - return Promise.reject(new Error("aborted prematurely before 'listen' event")); + return Promise.reject( + new Error("aborted prematurely before 'listen' event"), + ); } this.#stream = new ReadableStream({ start: (controller) => { @@ -102,7 +104,9 @@ export class Server> }); // closinng stream, so that the Application listen promise can resolve itself // https://developer.mozilla.org/en-US/docs/Web/API/ReadableStreamDefaultController/close - signal?.addEventListener("abort", () => controller.close(), { once: true }); + signal?.addEventListener("abort", () => controller.close(), { + once: true, + }); }, }); From 8526d5622dc14c3cff44d5f0e00ca183cdc9c955 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Sun, 22 Feb 2026 14:16:31 +1100 Subject: [PATCH 5/9] Update application.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- application.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application.test.ts b/application.test.ts index f404d956..a69d46fa 100644 --- a/application.test.ts +++ b/application.test.ts @@ -1134,7 +1134,7 @@ Deno.test({ }); const { signal } = controller; const p = app.listen({ signal }); - app.addEventListener("listen", async () => controller.abort()); + app.addEventListener("listen", () => controller.abort()); const GRACEFUL_TIME = 1000; let timer: number | undefined; const raceResult = await Promise.race([ From 6e014044f327bc97f27326656625c72676556cf6 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Sun, 22 Feb 2026 14:16:41 +1100 Subject: [PATCH 6/9] Update application.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- application.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/application.test.ts b/application.test.ts index a69d46fa..05b1fe71 100644 --- a/application.test.ts +++ b/application.test.ts @@ -1154,6 +1154,7 @@ Deno.test({ raceResult === "resolved cleanly", `'listen promise' should resolve before ${GRACEFUL_TIME} ms`, ); + teardown(); }, }); From 22f4b9ec159d44f203c975031b6c9062eb34ea6d Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Sun, 22 Feb 2026 14:17:04 +1100 Subject: [PATCH 7/9] Update application.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- application.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/application.test.ts b/application.test.ts index 05b1fe71..abeec537 100644 --- a/application.test.ts +++ b/application.test.ts @@ -1138,10 +1138,11 @@ Deno.test({ const GRACEFUL_TIME = 1000; let timer: number | undefined; const raceResult = await Promise.race([ - new Promise(async (resolve) => { - await p; - clearTimeout(timer); - resolve("resolved cleanly"); + new Promise((resolve) => { + p.then(() => { + clearTimeout(timer); + resolve("resolved cleanly"); + }); }), new Promise((resolve) => timer = setTimeout( From 04d4a298134054263f21a648229e8e4f5e194df4 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Sun, 22 Feb 2026 14:17:17 +1100 Subject: [PATCH 8/9] Update http_server_native.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- http_server_native.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http_server_native.ts b/http_server_native.ts index 61c114e2..26047f8c 100644 --- a/http_server_native.ts +++ b/http_server_native.ts @@ -102,7 +102,7 @@ export class Server> signal, ...options, }); - // closinng stream, so that the Application listen promise can resolve itself + // closing stream, so that the Application listen promise can resolve itself // https://developer.mozilla.org/en-US/docs/Web/API/ReadableStreamDefaultController/close signal?.addEventListener("abort", () => controller.close(), { once: true, From 72e54ff0ac6d6581701c49370f1814424741d884 Mon Sep 17 00:00:00 2001 From: Kitson Kelly Date: Sun, 22 Feb 2026 14:17:31 +1100 Subject: [PATCH 9/9] Update application.test.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- application.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/application.test.ts b/application.test.ts index abeec537..d4ceab61 100644 --- a/application.test.ts +++ b/application.test.ts @@ -1116,7 +1116,7 @@ Deno.test({ const { signal } = controller; const p = app.listen({ signal }); controller.abort(); - assertRejects( + await assertRejects( async () => await p, "aborted prematurely before 'listen' event", );