Skip to content
Open
Show file tree
Hide file tree
Changes from 8 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(
Comment thread
kitsonk marked this conversation as resolved.
Outdated
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());
Comment thread
kitsonk marked this conversation as resolved.
Outdated
const GRACEFUL_TIME = 1000;
let timer: number | undefined;
const raceResult = await Promise.race([
new Promise(async (resolve) => {
await p;
clearTimeout(timer);

Copilot AI Feb 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timer variable should be initialized before it's used in the clearTimeout call. If the first promise (the one that awaits p) wins the race, timer will still be undefined when clearTimeout(timer) is called on line 1143. This could lead to unexpected behavior. Consider initializing timer before the Promise.race call or restructuring the cleanup logic.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback

resolve("resolved cleanly");
Comment thread
kitsonk marked this conversation as resolved.
Outdated
}),
new Promise((resolve) =>
timer = setTimeout(
() => resolve("likely forever pending"),
GRACEFUL_TIME,
)
),
]);
assert(
raceResult === "resolved cleanly",
`'listen promise' should resolve before ${GRACEFUL_TIME} ms`,
);
Comment thread
kitsonk marked this conversation as resolved.
},
});

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
Comment thread
kitsonk marked this conversation as resolved.
Outdated
// https://developer.mozilla.org/en-US/docs/Web/API/ReadableStreamDefaultController/close
signal?.addEventListener("abort", () => controller.close(), {
once: true,
});
},
});

Expand Down
Loading