Skip to content

Commit d53479b

Browse files
kedrzuclaude
andcommitted
fix(dev): do not leak a rejection when a proxied upgrade fails
`DevServer.handleUpgrade` is async and both callers drop the promise it returns: the `upgrade` method handed out by `createDevServer`, and the `server.on("upgrade")` listener in `listen`. It rejects when no worker is available, and — far more often — from `NodeDevWorker.handleUpgrade`, which returns `proxy.ws(...)` unawaited. Replacing the dev worker kills every websocket proxied to it, so that promise rejects with `ECONNRESET` and nothing is there to catch it. For Nitro alone that is a stray unhandled rejection. Under Nuxt it ends the session, because the CLI treats any unhandled rejection as fatal and restarts the dev server; with a client that reconnects it loops. Make `handleUpgrade` never reject. There is nothing left to reply with once a handshake is in flight, so a failed upgrade closes the socket, and only errors that are not a peer disconnect are reported. Resolves #4493 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent da2849a commit d53479b

2 files changed

Lines changed: 46 additions & 8 deletions

File tree

src/core/dev-server/server.ts

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -290,15 +290,35 @@ class DevServer {
290290
return app;
291291
}
292292

293+
/**
294+
* Proxy a websocket upgrade to the current worker.
295+
*
296+
* Never rejects. Both callers -- `server.on("upgrade")` below and the
297+
* `upgrade` method exposed by `createDevServer` -- hand this to a callback
298+
* that drops the returned promise, so a rejection here surfaces as an
299+
* unhandled rejection instead of an error anyone can act on. There is also
300+
* nothing left to reply with once a handshake is in flight, so a failed
301+
* upgrade just closes the socket.
302+
*/
293303
async handleUpgrade(req: IncomingMessage, socket: Socket, head: any) {
294-
const worker = await this.getWorker();
295-
if (!worker) {
296-
throw createError({
297-
statusCode: 503,
298-
message: "No worker available.",
299-
});
304+
try {
305+
const worker = await this.getWorker();
306+
if (!worker) {
307+
throw createError({
308+
statusCode: 503,
309+
message: "No worker available.",
310+
});
311+
}
312+
await worker.handleUpgrade(req, socket, head);
313+
} catch (error) {
314+
// A websocket dying together with its connection is ordinary traffic: the
315+
// peer went away, or the worker it was proxied to has been replaced by a
316+
// reload. Only report what is not that.
317+
if (!isDisconnect(error)) {
318+
consola.warn("[nitro] [dev] Failed to proxy websocket upgrade.", error);
319+
}
320+
socket.destroy();
300321
}
301-
return worker.handleUpgrade(req, socket, head);
302322
}
303323

304324
#generateError() {
@@ -346,3 +366,17 @@ class DevServer {
346366
);
347367
}
348368
}
369+
370+
/**
371+
* Whether an error is only the other end of a connection going away, rather
372+
* than a fault worth reporting.
373+
*/
374+
function isDisconnect(error: unknown): boolean {
375+
const code = (error as NodeJS.ErrnoException | undefined)?.code;
376+
return (
377+
code === "ECONNRESET" ||
378+
code === "ECONNABORTED" ||
379+
code === "EPIPE" ||
380+
code === "ERR_STREAM_PREMATURE_CLOSE"
381+
);
382+
}

src/core/dev-server/worker.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,11 @@ export interface DevWorker {
2222
readonly closed: boolean;
2323
close(): Promise<void>;
2424
handleEvent: (event: H3Event) => Promise<void>;
25-
handleUpgrade: (req: IncomingMessage, socket: Socket, head: any) => void;
25+
handleUpgrade: (
26+
req: IncomingMessage,
27+
socket: Socket,
28+
head: any
29+
) => Promise<void> | void;
2630
}
2731

2832
export class NodeDevWorker implements DevWorker {

0 commit comments

Comments
 (0)