diff --git a/src/server/handlers/request/ssr/error-page-fallback.test.ts b/src/server/handlers/request/ssr/error-page-fallback.test.ts index 991520d8f9..e0c0f61b4f 100644 --- a/src/server/handlers/request/ssr/error-page-fallback.test.ts +++ b/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(); + 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 { + 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 = { diff --git a/src/server/handlers/request/ssr/error-page-fallback.ts b/src/server/handlers/request/ssr/error-page-fallback.ts index 7f62734cfc..24a87d25cd 100644 --- a/src/server/handlers/request/ssr/error-page-fallback.ts +++ b/src/server/handlers/request/ssr/error-page-fallback.ts @@ -141,6 +141,24 @@ async function setCachedPath(cacheKey: string, path: string | null): Promise { + if (!canCacheMiss(ctx)) return; + await setCachedPath(cacheKey, null); +} + async function deleteCachedPath(cacheKey: string): Promise { 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; } diff --git a/src/server/handlers/request/ssr/ssr.handler.test.ts b/src/server/handlers/request/ssr/ssr.handler.test.ts index 759e2f2ea7..42c941945e 100644 --- a/src/server/handlers/request/ssr/ssr.handler.test.ts +++ b/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; 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: "dev overlay", + 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({ diff --git a/src/server/handlers/request/ssr/ssr.handler.ts b/src/server/handlers/request/ssr/ssr.handler.ts index f288903192..d94c65d191 100644 --- a/src/server/handlers/request/ssr/ssr.handler.ts +++ b/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; }