fix(ssr): honour a project's error page in dev, not just production (depends on #2999)

src/server/handlers/request/ssr/error-page-fallback.test.ts

@@ -445,6 +445,120 @@ describe("server/handlers/request/ssr/error-page-fallback", () => {
     });
   });
 
+  describe("negative caching", () => {
+    /** Records every write so the tests can see what was cached. */
+    function recordingRepo() {
+      const store = new Map<string, string>();
+      const writes: Array<{ key: string; value: string }> = [];
+
+      return {
+        writes,
+        repo: {
+          get: (key: string) => Promise.resolve(store.get(key) ?? null),
+          set: (key: string, value: string) => {
+            store.set(key, value);
+            writes.push({ key, value });
+            return Promise.resolve();
+          },
+          delete: (key: string) => {
+            store.delete(key);
+            return Promise.resolve();
+          },
+        },
+      };
+    }
+
+    function pagesDirOnly() {
+      return createMockAdapter({
+        stat: (path: string) =>
+          Promise.resolve({
+            isFile: false,
+            isDirectory: path.endsWith("pages"),
+            size: 0,
+            mtime: null,
+          }),
+        resolveFile: () => Promise.resolve(null),
+      });
+    }
+
+    async function runFallback(ctx: HandlerContext): Promise<Response | null> {
+      return await tryErrorPageFallback(
+        new Request("http://localhost/boom"),
+        ctx,
+        new ResponseBuilder(),
+        { statusCode: 500, pathname: "/boom" },
+      );
+    }
+
+    it("caches a miss for a deployed project", async () => {
+      const { repo, writes } = recordingRepo();
+      __injectCacheForTests(repo as never);
+
+      const result = await runFallback(
+        makeCtx({ adapter: pagesDirOnly(), isLocalProject: false }),
+      );
+
+      assertEquals(result, null);
+      assertEquals(writes.length > 0, true, "a deployed project should cache the miss");
+      assertEquals(writes.every((write) => write.value === "__NOT_FOUND__"), true);
+    });
+
+    // Regression: dev reaches this fallback now, and nothing invalidates the
+    // cache on a file change. A cached miss meant that creating pages/500.tsx
+    // mid-session kept showing the dev overlay until the server restarted.
+    it("does not cache a miss in dev", async () => {
+      const { repo, writes } = recordingRepo();
+      __injectCacheForTests(repo as never);
+
+      const result = await runFallback(
+        makeCtx({ adapter: pagesDirOnly(), isLocalProject: true }),
+      );
+
+      assertEquals(result, null);
+      assertEquals(writes.length, 0, "dev must re-probe the filesystem each time");
+    });
+
+    it("finds an error page created after a miss in dev", async () => {
+      const { repo } = recordingRepo();
+      __injectCacheForTests(repo as never);
+
+      let errorPageExists = false;
+      const adapter = createMockAdapter({
+        stat: (path: string) =>
+          Promise.resolve({
+            isFile: false,
+            isDirectory: path.endsWith("pages"),
+            size: 0,
+            mtime: null,
+          }),
+        resolveFile: (path: string) =>
+          Promise.resolve(errorPageExists && path.endsWith("500") ? "pages/500.tsx" : null),
+      });
+      const ctx = makeCtx({ adapter, isLocalProject: true });
+
+      assertEquals(await runFallback(ctx), null);
+
+      // The author creates pages/500.tsx without restarting the server.
+      errorPageExists = true;
+
+      let resolved = false;
+      const adapterAfter = createMockAdapter({
+        stat: adapter.fs.stat as never,
+        readFile: () => {
+          // Reaching the read proves the miss was not cached. Stop here rather
+          // than compiling a component, which is not what this test is about.
+          resolved = true;
+          return Promise.reject(new Error("stop after resolving the error page"));
+        },
+        resolveFile: adapter.fs.resolveFile as never,
+      });
+
+      await runFallback(makeCtx({ adapter: adapterAfter, isLocalProject: true }));
+
+      assertEquals(resolved, true, "the newly created error page must be picked up");
+    });
+  });
+
   describe("__injectCacheForTests", () => {
     it("can inject and reset cache repo", () => {
       const mockRepo = {

src/server/handlers/request/ssr/error-page-fallback.ts

@@ -141,6 +141,24 @@ async function setCachedPath(cacheKey: string, path: string | null): Promise<voi
   errorPagePathCache.set(cacheKey, path);
 }
 
+/**
+ * Whether a "this project has no error page" answer may be cached.
+ *
+ * In dev it may not. Nothing invalidates this cache when the filesystem
+ * changes, so caching the miss means a project that adds `pages/500.tsx` while
+ * the server is running keeps getting the dev overlay until restart, with no
+ * indication why. A miss costs one file probe on an error render, which is not
+ * a path worth optimising in dev.
+ */
+function canCacheMiss(ctx: HandlerContext): boolean {
+  return !ctx.isLocalProject;
+}
+
+async function setCachedMiss(cacheKey: string, ctx: HandlerContext): Promise<void> {
+  if (!canCacheMiss(ctx)) return;
+  await setCachedPath(cacheKey, null);
+}
+
 async function deleteCachedPath(cacheKey: string): Promise<void> {
   if (injectedCacheRepo) {
     await injectedCacheRepo.delete(cacheKey);
@@ -175,7 +193,7 @@ async function tryLoadErrorPage(
     try {
       const resolvedPath = await ctx.adapter.fs.resolveFile(basePath);
       if (!resolvedPath) {
-        await setCachedPath(cacheKey, null);
+        await setCachedMiss(cacheKey, ctx);
         return null;
       }
 
@@ -189,7 +207,7 @@ async function tryLoadErrorPage(
       // expected: resolveFile may fail, fall through to extension probing
     }
 
-    await setCachedPath(cacheKey, null);
+    await setCachedMiss(cacheKey, ctx);
     return null;
   }
 
@@ -209,7 +227,7 @@ async function tryLoadErrorPage(
     }
   }
 
-  await setCachedPath(cacheKey, null);
+  await setCachedMiss(cacheKey, ctx);
   return null;
 }
 

src/server/handlers/request/ssr/ssr.handler.test.ts

@@ -433,7 +433,43 @@ describe("server/handlers/request/ssr/ssr.handler", () => {
   });
 
   describe("handle - server error with dev overlay", () => {
-    it("skips custom error fallback when showDevOverlay is true", async () => {
+    function ctxWithRecordedStats(): { ctx: ReturnType<typeof makeCtx>; statted: string[] } {
+      const statted: string[] = [];
+      const adapter = createMockAdapter();
+      const inner = adapter.fs.stat;
+      adapter.fs.stat = (path: string) => {
+        statted.push(path);
+        return inner(path);
+      };
+      return { ctx: makeCtx({ adapter }), statted };
+    }
+
+    for (const errorType of ["server-error", "runtime"] as const) {
+      it(`looks for a custom error page for ${errorType} even with the dev overlay`, async () => {
+        const mockService = createMockSSRService({
+          renderPage: () =>
+            Promise.resolve({
+              status: 500,
+              html: "<html>dev overlay</html>",
+              isStreaming: false,
+              cacheStrategy: "no-cache" as const,
+              errorType,
+              showDevOverlay: true,
+              error: new Error("Oops"),
+              slug: "page",
+            }),
+        });
+        const { ctx, statted } = ctxWithRecordedStats();
+        const handler = new SSRHandler(mockService);
+
+        const result = await handler.handle(new Request("http://localhost/page"), ctx);
+
+        assertEquals(statted.some((path) => path.endsWith("/pages")), true);
+        assertEquals(result.response!.status, 500);
+      });
+    }
+
+    it("falls back to the dev overlay when no custom error page exists", async () => {
       const mockService = createMockSSRService({
         renderPage: () =>
           Promise.resolve({

src/server/handlers/request/ssr/ssr.handler.ts

@@ -235,7 +235,9 @@ export class SSRHandler extends BaseHandler {
           return this.handleNotFound(req, ctx, slug, nonce);
         }
 
-        if (result.errorType === "server-error" && !result.showDevOverlay) {
+        // Runtime errors use the dev overlay, but a project-owned error page
+        // should still take precedence when it exists.
+        if (result.errorType === "server-error" || result.errorType === "runtime") {
           const customResponse = await this.tryCustomErrorFallback(req, ctx, result, nonce);
           if (customResponse) return customResponse;
         }