Skip to content

Commit c761c4a

Browse files
committed
fix(hot): harden the SSE handshake and defer the overlay's Escape listener
The handshake now ends a response whose headers were already sent instead of crashing on writeHead, and the middleware routes handshake errors to next() — an exception there previously became an unhandled rejection that killed the process. The overlay's Escape listener on the host document is now attached lazily inside ensureOverlay (once per page, through the shared state), matching how webpack-dev-server registers it inside createOverlay, so importing the client in a non-DOM environment (SSR bundle, worker) no longer throws at evaluation time.
1 parent 1653e96 commit c761c4a

5 files changed

Lines changed: 82 additions & 8 deletions

File tree

client-src/overlay.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ const colors = {
126126
* @property {HTMLIFrameElement | null} frame overlay iframe
127127
* @property {HTMLElement | null} card visible panel inside the iframe
128128
* @property {boolean} runtimeListenersAttached whether the window listeners are attached
129+
* @property {boolean} hostKeydownAttached whether the host document's Escape listener is attached
129130
* @property {number} pageIndex page shown when paginating
130131
* @property {Record<string, { type: "errors" | "warnings", lines: string[] }>} problemsBySource each reporting source's problems
131132
* @property {{ type: "errors" | "warnings", lines: string[] } | null} currentProblems union of every source, as displayed
@@ -139,6 +140,7 @@ function createOverlayState() {
139140
frame: null,
140141
card: null,
141142
runtimeListenersAttached: false,
143+
hostKeydownAttached: false,
142144
pageIndex: 0,
143145
problemsBySource: {},
144146
currentProblems: null,
@@ -329,13 +331,6 @@ function linkify(html) {
329331
});
330332
}
331333

332-
// Dismiss the overlay when pressing Escape while the page has focus.
333-
document.addEventListener("keydown", (event) => {
334-
if (event.key === "Escape") {
335-
clear();
336-
}
337-
});
338-
339334
/**
340335
* Create (or return) the overlay iframe and the card inside it.
341336
* @returns {HTMLElement | null} the card element, or null when the frame
@@ -350,6 +345,19 @@ function ensureOverlay() {
350345
return null;
351346
}
352347

348+
// Dismiss the overlay when pressing Escape while the host page has focus —
349+
// the frame's own keydown listener only fires when the frame is focused.
350+
// Attached lazily on the first render (and once per page, the flag lives in
351+
// the shared state) so importing the module stays free of DOM side effects.
352+
if (!state.hostKeydownAttached) {
353+
state.hostKeydownAttached = true;
354+
document.addEventListener("keydown", (event) => {
355+
if (event.key === "Escape") {
356+
clear();
357+
}
358+
});
359+
}
360+
353361
// Enable Trusted Types if they are available in the current browser.
354362
if (window.trustedTypes && !state.trustedTypesPolicy) {
355363
state.trustedTypesPolicy = window.trustedTypes.createPolicy(

src/hot.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,15 @@ function createEventStream(heartbeat, logger) {
9999
clients = new Map();
100100
},
101101
handler(req, res) {
102+
// A response another middleware already started can no longer become an
103+
// SSE stream — end it instead of crashing on writeHead.
104+
if (res.headersSent) {
105+
if (!res.writableEnded) {
106+
res.end();
107+
}
108+
return;
109+
}
110+
102111
/** @type {Record<string, string>} */
103112
const headers = {
104113
"Access-Control-Allow-Origin": "*",

src/middleware.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,7 +380,11 @@ function wrapper(context) {
380380
return async function middleware(req, res, next) {
381381
// Intercept Server-Sent Events handshake when the `hot` option is enabled.
382382
if (context.hot && hotPathMatch(getRequestURL(req), context.hot.path)) {
383-
context.hot.handle(req, res);
383+
try {
384+
context.hot.handle(req, res);
385+
} catch (error) {
386+
return next(/** @type {NodeJS.ErrnoException} */ (error));
387+
}
384388
return;
385389
}
386390

test/hot.test.js

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import http from "node:http";
2+
13
import createHot, {
24
createEventStream,
35
formatErrors,
@@ -347,6 +349,39 @@ describe("createHot", () => {
347349
hot.close();
348350
});
349351

352+
it("ends a response whose headers were already sent instead of registering it", async () => {
353+
const compiler = makeFakeCompiler();
354+
const hot = createHot(compiler, {});
355+
356+
const server = http.createServer((req, res) => {
357+
// Another middleware already started this response before the SSE
358+
// handshake sees it.
359+
res.writeHead(200, { "Content-Type": "text/plain" });
360+
res.write("already-started");
361+
362+
hot.handle(req, res);
363+
// Were the response wrongly registered as a client, this frame would
364+
// show up in the body below.
365+
hot.publish({ action: "custom" });
366+
});
367+
368+
await new Promise((resolve) => {
369+
server.listen(0, resolve);
370+
});
371+
const { port } = server.address();
372+
373+
const response = await fetch(`http://127.0.0.1:${port}/__webpack_hmr`);
374+
const body = await response.text();
375+
376+
expect(response.headers.get("content-type")).toBe("text/plain");
377+
expect(body).toBe("already-started");
378+
379+
hot.close();
380+
await new Promise((resolve) => {
381+
server.close(resolve);
382+
});
383+
});
384+
350385
it("includes the compilation name in the building payload", () => {
351386
const compiler = makeFakeCompiler();
352387
compiler.name = "main";

test/middleware.test.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7010,6 +7010,24 @@ describe.each([
70107010
);
70117011
});
70127012

7013+
it("routes SSE handshake errors to the framework error handler", async () => {
7014+
const compiler = getCompiler(webpackConfig);
7015+
[server, req, instance] = await frameworkFactory(
7016+
name,
7017+
framework,
7018+
compiler,
7019+
{ hot: true },
7020+
);
7021+
7022+
instance.context.hot.handle = () => {
7023+
throw new Error("handshake failed");
7024+
};
7025+
7026+
const response = await req.get("/__webpack_hmr");
7027+
7028+
expect(response.statusCode).toBe(500);
7029+
});
7030+
70137031
it("does not hang hot requests made after instance.close()", async () => {
70147032
const compiler = getCompiler(webpackConfig);
70157033
[server, req, instance] = await frameworkFactory(

0 commit comments

Comments
 (0)