fix(client): reload when an accept handler errors during apply (#2360)

* fix(client): reload when an accept handler errors during apply

An error thrown inside a module.hot.accept handler was logged and
ignored, leaving the page running stale code with no recovery. When the
`reload` option is enabled (the default), onErrored now falls back to a
full page reload, like the other unrecoverable update paths.

The HMR runtime and page-reload calls are isolated behind small utils
so process-update finally has direct test coverage. A missing HMR
runtime is now reported with a single actionable error instead of
throwing at bundle evaluation.

Ref webpack/webpack-hot-middleware#334

* fixup!

* fixup!

Author: bjohansebas

client-src/process-update.js

@@ -4,20 +4,15 @@ import getHot from "./utils/get-hot.js";
 import { log } from "./utils/log.js";
 import reloadPage from "./utils/reload.js";
 
-const maybeHot = getHot();
-
-if (!maybeHot) {
-  throw new Error("[HMR] Hot Module Replacement is disabled.");
-}
-
-const hot = maybeHot;
-
 const HMR_DOCS_URL = "https://webpack.js.org/concepts/hot-module-replacement/";
 
 /** @type {string | undefined} */
 let lastHash;
 /** @type {Record<string, number>} */
 const failureStatuses = { abort: 1, fail: 1 };
+// Set per applyUpdate() call from the client's `reload` option.
+let reloadOnErrored = false;
+let loggedRuntimeMissing = false;
 
 /** @type {webpack.ApplyOptions} */
 const applyOptions = {
@@ -39,6 +34,12 @@ const applyOptions = {
     log.warn(
       `Ignored an error while updating module ${event.moduleId} (${event.type})`,
     );
+    // An error thrown inside an accept handler leaves the app in an
+    // undefined state — fall back to a full page reload.
+    if (reloadOnErrored) {
+      log.warn("Reloading page");
+      reloadPage();
+    }
   },
 };
 
@@ -56,8 +57,25 @@ function upToDate(hash) {
  * @param {{ reload?: boolean }} options client options
  */
 export default function applyUpdate(hash, options) {
+  const hot = getHot();
+
+  if (!hot) {
+    // Logged (once) instead of thrown: this runs inside the SSE message
+    // handler, whose catch would mislabel a throw as an invalid message.
+    if (!loggedRuntimeMissing) {
+      loggedRuntimeMissing = true;
+      log.error(
+        "[HMR] Hot Module Replacement is disabled. " +
+          "Add HotModuleReplacementPlugin to the webpack configuration.",
+      );
+    }
+    return;
+  }
+
   const { reload } = options;
 
+  reloadOnErrored = Boolean(reload);
+
   /**
    * Trigger a full page reload when HMR cannot apply the update.
    */
@@ -72,6 +90,7 @@ export default function applyUpdate(hash, options) {
    * @param {Error} err error
    */
   function handleError(err) {
+    // @ts-expect-error function declarations are hoisted, so the `!hot` guard narrowing is lost
     if (hot.status() in failureStatuses) {
       log.warn("Cannot check for update (Full reload needed)");
       log.warn(err.stack || err.message);
@@ -126,6 +145,7 @@ export default function applyUpdate(hash, options) {
    * Ask webpack for the next chunk of HMR updates and apply them.
    */
   function check() {
+    // @ts-expect-error function declarations are hoisted, so the `!hot` guard narrowing is lost
     hot
       .check(false)
       .then((updatedModules) => {
@@ -136,6 +156,7 @@ export default function applyUpdate(hash, options) {
           return undefined;
         }
 
+        // @ts-expect-error function declarations are hoisted, so the `!hot` guard narrowing is lost
         return hot.apply(applyOptions).then((renewedModules) => {
           if (!upToDate()) check();
           logUpdates(updatedModules, renewedModules);

test/process-update.test.js

@@ -46,10 +46,9 @@ describe("process-update", () => {
   }
 
   /**
-   * Load a fresh process-update with the given fake runtime. The runtime is
-   * resolved at module evaluation, so the mock must be configured in the same
-   * module registry before process-update is required.
-   * @param {FakeHot} hot fake `import.meta.webpackHot`
+   * Load a fresh process-update with the given fake runtime configured in the
+   * same module registry.
+   * @param {FakeHot | undefined} hot fake `import.meta.webpackHot`
    * @returns {typeof import("../client-src/process-update").default} applyUpdate
    */
   function loadApplyUpdate(hot) {
@@ -84,17 +83,18 @@ describe("process-update", () => {
     jest.restoreAllMocks();
   });
 
-  it("throws at module evaluation when the HMR runtime is disabled", () => {
-    jest.isolateModules(() => {
-      /** @type {jest.Mock} */
-      const isolatedGetHot = require("../client-src/utils/get-hot");
+  it("logs an error once when the HMR runtime is disabled", () => {
+    applyUpdate = loadApplyUpdate(undefined);
 
-      isolatedGetHot.mockReturnValue(undefined);
+    expect(() => applyUpdate("h1", { reload: true })).not.toThrow();
+    applyUpdate("h2", { reload: true });
 
-      expect(() => require("../client-src/process-update")).toThrow(
-        /Hot Module Replacement is disabled/,
-      );
-    });
+    const runtimeErrors = console.error.mock.calls.filter((call) =>
+      call.join(" ").includes("Hot Module Replacement is disabled"),
+    );
+
+    expect(runtimeErrors).toHaveLength(1);
+    expect(reloadPage).not.toHaveBeenCalled();
   });
 
   it("checks and applies an update when the hash differs", async () => {
@@ -124,7 +124,7 @@ describe("process-update", () => {
     expect(reloadPage).toHaveBeenCalledTimes(1);
   });
 
-  it("ignores an error thrown by an accept handler during apply", async () => {
+  it("reloads when an accept handler errors during apply (onErrored)", async () => {
     applyUpdate = loadApplyUpdate(
       makeFakeHot({
         applyImpl: (applyOptions) => {
@@ -143,7 +143,28 @@ describe("process-update", () => {
     globalThis.__webpack_hash__ = "new-hash";
     await flushPromises();
 
-    // The error is logged but does not trigger a reload.
+    expect(reloadPage).toHaveBeenCalledTimes(1);
+  });
+
+  it("does not reload on errored apply when reload is disabled", async () => {
+    applyUpdate = loadApplyUpdate(
+      makeFakeHot({
+        applyImpl: (applyOptions) => {
+          applyOptions.onErrored({
+            error: new Error("accept handler failed"),
+            moduleId: "./a.js",
+            type: "accept-errored",
+          });
+
+          return Promise.resolve(["./a.js"]);
+        },
+      }),
+    );
+
+    applyUpdate("new-hash", { reload: false });
+    globalThis.__webpack_hash__ = "new-hash";
+    await flushPromises();
+
     expect(reloadPage).not.toHaveBeenCalled();
   });