Skip to content

fix: make Application listen promise resolve after aborted #685

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 38 additions & 4 deletions application.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -1118,11 +1116,47 @@ Deno.test({
const { signal } = controller;
const p = app.listen({ signal });
controller.abort();
await p;
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;
const p = app.listen({ signal });
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`,
);
},
});

Deno.test({
name: "Application load correct default server",
ignore: isNode(), // this just hangs on node, because we can't close down
Expand Down
11 changes: 11 additions & 0 deletions http_server_native.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ export class Server<AS extends State = Record<string, any>>
const { signal } = this.#options;
const { onListen, ...options } = this.#options;
const { promise, resolve } = createPromiseWithResolvers<Listener>();
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<NativeRequest>({
start: (controller) => {
this.#httpServer = serve?.({
Expand All @@ -96,6 +102,11 @@ export class Server<AS extends State = Record<string, any>>
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,
});
},
});

Expand Down