fix(overlay): keep a runtime error a successful build cannot vouch for

.changeset/runtime-error-survives-build.md

@@ -0,0 +1,5 @@
+---
+"webpack-dev-middleware": patch
+---
+
+Keep an uncaught runtime error in the overlay when a build succeeds — a successful compilation says nothing about an error the page threw on its own, and it used to dismiss one raised moments earlier by an entry that threw while it was still evaluating. A rebuild still clears it, since that replaces the code the error came from

client-src/globals.d.ts

@@ -10,6 +10,7 @@ declare module "ansi-html-community" {
 
 interface ClientReporter {
   cleanProblemsCache(name: string): void;
+  clearRuntimeProblems(): void;
   problems(
     type: "errors" | "warnings",
     obj: { errors: string[]; warnings: string[]; name?: string },

client-src/index.js

@@ -267,6 +267,7 @@ function getEventSourceWrapper() {
 /**
  * @returns {{
  * cleanProblemsCache: (name: string) => void,
+ * clearRuntimeProblems: () => void,
  * problems: (type: "errors" | "warnings", obj: HMRPayload) => boolean,
  * success: (obj?: HMRPayload) => void,
  * useCustomOverlay: (customOverlay: EXPECTED_ANY) => void,
@@ -361,10 +362,14 @@ function createReporter() {
       return false;
     }
 
-    // Clear only this client's problems and the runtime errors — other
-    // clients sharing the overlay keep theirs.
+    // Clear only this client's build problems — other clients sharing the
+    // overlay keep theirs, and runtime errors are not this event's to judge: a
+    // build that succeeded says nothing about an error the page threw on its
+    // own. Dropping them here dismissed an overlay raised milliseconds earlier
+    // by an entry that threw while it was still evaluating, because the
+    // handshake's catch-up sync arrives right behind it (webpack-dev-server
+    // #5024). A rebuild clears them instead, from `building` below.
     overlay.clear("");
-    overlay.clear("runtime");
     return true;
   };
 
@@ -409,6 +414,14 @@ function createReporter() {
       delete problemsByName[(obj && obj.name) || ""];
       renderOverlay();
     },
+    clearRuntimeProblems() {
+      // No overlay configured, or a custom one that does not take sources.
+      if (!overlay || !overlay.clear) {
+        return;
+      }
+
+      overlay.clear("runtime");
+    },
     useCustomOverlay(customOverlay) {
       overlay = customOverlay;
     },
@@ -441,6 +454,13 @@ function processMessage(obj) {
           obj.file ? ` (${obj.file} changed)` : ""
         }`,
       );
+      // A rebuild replaces the code a runtime error came from, so the error
+      // stops being worth showing — unlike a build that merely succeeded,
+      // which says nothing about it. Not scoped to this client's bundle: a
+      // runtime error is the page's, not one compilation's.
+      if (reporter) {
+        reporter.clearRuntimeProblems();
+      }
       if (options.progress && typeof document !== "undefined") {
         lastBuildingName = obj.name || "";
         indicator.show(

test/e2e/overlay.test.js

@@ -15,6 +15,8 @@ import {
 import createHotApp from "../helpers/hot-app";
 import runBrowser from "../helpers/run-browser";
 
+const CLIENT_ENTRY = require.resolve("../../client-src/index.js");
+
 jest.setTimeout(400000);
 
 describe("error overlay (browser)", () => {
@@ -1103,6 +1105,87 @@ describe("error overlay parity with webpack-dev-server (browser)", () => {
     ).toBe(0);
     expect(await page.evaluate(() => globalThis.xss)).toBeUndefined();
   });
+
+  it("keeps a runtime error the filter accepts and drops the one it rejects", async () => {
+    hotApp = await createHotApp({
+      // Encoded in both layers webpack-dev-server uses: the function through
+      // `encodeURIComponent`, then the whole parameter through
+      // `URLSearchParams`. The outer layer is what keeps a `!` out of the
+      // entry request, which webpack would otherwise read as its
+      // inline-loader separator.
+      query: `?${new URLSearchParams({
+        overlay: JSON.stringify({
+          runtimeErrors: encodeURIComponent(
+            "function(error){return !/Injected/.test(error.message)}",
+          ),
+        }),
+      })}`,
+      code: boomApp("v1"),
+    });
+    ({ page, browser } = await runBrowser());
+
+    await page.goto(hotApp.url);
+    await waitForAppText(page, "v1");
+    await waitForRuntimeListeners(page);
+
+    // Rejected by the filter, so nothing appears.
+    await page.evaluate(() => globalThis.boom("Injected error"));
+    await new Promise((resolve) => {
+      setTimeout(resolve, 500);
+    });
+
+    expect(await page.$(`#${OVERLAY_ID}`)).toBeNull();
+
+    // Accepted, so the filter is doing the deciding rather than suppressing
+    // everything — which a filter that failed to decode would also look like.
+    await page.evaluate(() => globalThis.boom("kept boom"));
+    const frame = await waitForOverlay(page);
+
+    expect(await frame.evaluate(() => document.body.textContent)).toContain(
+      "kept boom",
+    );
+  });
+
+  it("keeps a runtime error thrown while the entry was still evaluating", async () => {
+    hotApp = await createHotApp({
+      // Throws as the entry evaluates, which is before the transport's
+      // handshake completes. The build itself succeeded, so the catch-up sync
+      // that follows reports no problems at all.
+      //
+      // `subscribeAll` runs after the message has been fully processed, so it
+      // is the gate for "the sync has been handled" — waiting on a log line
+      // would not do, since a sync that changes nothing writes none.
+      code: `
+        globalThis.handled = [];
+        require(${JSON.stringify(CLIENT_ENTRY)}).subscribeAll((obj) => {
+          globalThis.handled.push(obj.action);
+        });
+        throw new Error("Injected error");
+      `,
+    });
+    ({ page, browser } = await runBrowser());
+
+    await page.goto(hotApp.url);
+
+    const frame = await waitForOverlay(page);
+
+    expect(await frame.evaluate(() => document.body.textContent)).toContain(
+      "Injected error",
+    );
+
+    // The sync must not take the overlay down with it: a build that succeeded
+    // says nothing about an error the page threw on its own. This is
+    // webpack-dev-server#5024.
+    await page.waitForFunction(
+      () => globalThis.handled && globalThis.handled.includes("sync"),
+      { timeout: 30000 },
+    );
+
+    expect(await page.$(`#${OVERLAY_ID}`)).not.toBeNull();
+    expect(await frame.evaluate(() => document.body.textContent)).toContain(
+      "Injected error",
+    );
+  });
 });
 
 describe("overlay shared state across bundled copies (browser)", () => {