fix: make Application listen promise resolve after aborted

[Thesephi]

Hi Oak community / cc @kitsonk,

---

Edited on 12th Dec 24: should close #686 and close #687

---

I'm using Oak heavily at work (kudos to all for the cool library of course 😀).

I notice (at current version v17.1.3) the server listen promise doesn't resolve itself even after already aborted.

Naive reproduction attempt:

const abortCtrl = new AbortController();
const { signal } = abortCtrl;
const app = new Application();
const p = app.listen({ signal });
setTimeout(() => abortCtrl.abort(), 2000); // simulate turning off the application after 2 seconds
await p;
console.log("application aborted, safe to shut down the parent process"); // <== unreachable line

If we do it like the above, then we never see the last line logged out in the console. This suggests the await p expression never resolves (forever staying at pending state).

I was a bit curious how it behaves differently from the doc, so I dug a bit & it resulted in this PR.

I mark the PR as ready for visibility - always happy to consider alternatives or feedback :)

---

Side note:

During the process, I also noticed a related unit test was disabled. I think it was disabled due to another (slightly related) reason: the server needs a bit of time to initialize itself, so if we call abort() right after listen() (as done in the originally disabled test), then the signal is already aborted by the time the server initializes, and so we "miss" the chance to properly abort it.

To this end, I also re-enabled that test, plus I added another test so we cover both scenarios. The PR content explains better than my words here tho 😀

---

Thank you everyone for your insights / reviews / discussions / objections etc. 🙌 !

[CLAassistant]

All committers have signed the CLA.

[Thesephi]

PS: apologies @kitsonk, I forgot deno fmt . Should be fixed at the latest PR commit :)

[nomocas]

Hey @Thesephi,

If you have the rights to do so, don't hesitate to reference my issue in the "development" section in the right column above.
Something as "close #686". It will close the issue automatically when this PR is merged.

Thank you!

[Copilot]

Pull request overview

This PR fixes a critical bug where the Application.listen() promise would never resolve after the server was aborted via an AbortSignal. The issue stemmed from the native HTTP server implementation not properly closing its ReadableStream when the abort signal fired, causing the for await loop in application.ts to hang indefinitely.

Changes:

• Added abort signal handling to close the ReadableStream in the native HTTP server implementation
• Added a premature abort check that rejects the listen promise if the signal is already aborted
• Re-enabled and improved the abort-related test, splitting it into two scenarios: abort before and after the listen event

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
http_server_native.ts	Added abort signal handling to close the ReadableStream controller when aborted, aligning with Bun and Node implementations
application.test.ts	Re-enabled previously disabled abort test, added premature abort check test, and added new test for abort after listen event

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

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.

[kitsonk]

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