Skip to content

Commit f8a28a7

Browse files
committed
feat(devframe): add onServerError to observe post-bind server errors
startHttpAndWs's owned httpServer has no error listener once past the bind window this PR's base fixes — the temporary listener is removed the instant listen() succeeds. A later runtime error (e.g. a transient EMFILE while accepting a connection) still crashes the process, and StartedServer exposes no handle a caller could attach their own listener to (unlike the shared-server path, where the caller already owns the object). Add an onServerError option instead of exposing the raw httpServer — handing out the raw object would let a caller call close() on it directly, bypassing the wrapper's close() and leaking the WS transport exactly like the bug this PR's base fixes. The callback fires for the bind-time error too (alongside the rejection), so it's one place to observe every error the server ever emits, bind or lifetime. The lifetime-forwarding wire-up (httpServer.on('error', ...)) is a one-line, logic-free pass-through of Node's own documented event and isn't covered by a dedicated test: StartedServer deliberately doesn't expose the raw server, so a test has no way to trigger a genuine post-bind error without a test-only seam. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent c770b07 commit f8a28a7

2 files changed

Lines changed: 38 additions & 1 deletion

File tree

packages/devframe/src/node/__tests__/server.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,4 +115,22 @@ describe('startHttpAndWs listen failures', () => {
115115
await first.close()
116116
}
117117
})
118+
119+
it('forwards the bind error to onServerError alongside the rejection', async () => {
120+
const host = '127.0.0.1'
121+
const first = await startHttpAndWs({ context: await createTestContext(), host, port: 0, auth: false })
122+
const onServerError = vi.fn()
123+
124+
try {
125+
await expect(
126+
startHttpAndWs({ context: await createTestContext(), host, port: first.port, auth: false, onServerError }),
127+
).rejects.toThrow(expect.objectContaining({ code: 'DF0052' }))
128+
129+
expect(onServerError).toHaveBeenCalledOnce()
130+
expect((onServerError.mock.calls[0]![0] as Error & { code?: string }).code).toBe('EADDRINUSE')
131+
}
132+
finally {
133+
await first.close()
134+
}
135+
})
118136
})

packages/devframe/src/node/server.ts

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,16 @@ export interface StartHttpAndWsOptions {
109109
* own startup banner. Devframe does not print one itself.
110110
*/
111111
onReady?: (info: { origin: string, port: number, app: H3 }) => void | Promise<void>
112+
/**
113+
* Called for every error the owned HTTP server emits — during the initial
114+
* bind (alongside the rejection `startHttpAndWs` already throws for that
115+
* case) and for any error afterward, for the server's whole running life
116+
* (e.g. a transient `EMFILE` while accepting a connection). Ignored when a
117+
* `server` is supplied — the caller already owns that object and can
118+
* listen on it directly. Without this, a post-bind error has no listener
119+
* and crashes the process, matching today's behavior.
120+
*/
121+
onServerError?: (error: Error) => void
112122
}
113123

114124
export interface StartedServer {
@@ -256,7 +266,10 @@ export async function startHttpAndWs(options: StartHttpAndWsOptions): Promise<St
256266
if (ownsHttpServer) {
257267
try {
258268
await new Promise<void>((resolve, reject) => {
259-
const onError = (error: Error): void => reject(error)
269+
const onError = (error: Error): void => {
270+
options.onServerError?.(error)
271+
reject(error)
272+
}
260273
// Without this listener a failed bind emits `error` with nobody
261274
// attached — an uncaughtException — and the `listen` callback never
262275
// fires, so this promise never settles.
@@ -273,6 +286,12 @@ export async function startHttpAndWs(options: StartHttpAndWsOptions): Promise<St
273286
await closeWs().catch(() => {})
274287
throw diagnostics.DF0052({ host: bindHost, port, reason: (error as Error).message, cause: error as Error })
275288
}
289+
// The bind succeeded, so the temporary listener above is gone. Attach a
290+
// persistent one for the server's remaining life — otherwise a later
291+
// error (e.g. a transient EMFILE while accepting a connection) has no
292+
// listener and crashes the process the same way an unguarded bind used to.
293+
if (options.onServerError)
294+
httpServer.on('error', options.onServerError)
276295
}
277296

278297
const address = httpServer.address()

0 commit comments

Comments
 (0)