From c8dea480cad8a367841fe4a60fb011e6586459b6 Mon Sep 17 00:00:00 2001 From: Khang Date: Tue, 3 Dec 2024 13:19:49 +0100 Subject: [PATCH 1/4] 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/4] 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/4] 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/4] 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, + }); }, });