diff --git a/.changeset/setup-probe-transient-error.md b/.changeset/setup-probe-transient-error.md new file mode 100644 index 000000000..4b9011588 --- /dev/null +++ b/.changeset/setup-probe-transient-error.md @@ -0,0 +1,5 @@ +--- +"emdash": patch +--- + +Fix frontend pages redirecting to `/_emdash/admin/setup` on a fully set-up site. The anonymous fast-path "setup probe" in the Astro middleware queries `_emdash_migrations` to detect a fresh, un-migrated database, but its `catch` block treated **every** error as "fresh install" — so a transient DB failure (D1 connection loss, replica unavailable, query timeout, cold-start race, locked SQLite) wrongly bounced real visitors to the setup wizard. The probe now only redirects when the error is a genuinely-missing table (via the shared `isMissingTableError` helper) and otherwise renders the page normally. The `setupVerified` flag is also moved onto a `globalThis` `Symbol.for` singleton so it isn't duplicated across SSR chunks, which had caused the probe to re-run far more often than intended (and each re-run was another chance to hit the bug). diff --git a/packages/core/src/astro/middleware.ts b/packages/core/src/astro/middleware.ts index 0ec49008d..1c5c78098 100644 --- a/packages/core/src/astro/middleware.ts +++ b/packages/core/src/astro/middleware.ts @@ -50,6 +50,7 @@ import { type RequestMetrics, runWithContext, } from "../request-context.js"; +import { isMissingTableError } from "../utils/db-errors.js"; import type { EmDashConfig } from "./integration/runtime.js"; import { createPublicPluginApiRouteHandler } from "./public-plugin-api-routes.js"; import type { EmDashHandlers } from "./types.js"; @@ -69,8 +70,24 @@ let i18nInitialized = false; * would query an empty database and crash. Once verified (or once the runtime * has initialized via an admin/API request), this stays true for the worker's * lifetime. + * + * Stored on globalThis behind a Symbol key so the flag is a true singleton + * even when the bundler duplicates this module across SSR chunks (same + * pattern as request-cache.ts). A plain module-scoped `let` becomes multiple + * independent variables, which would make the setup probe re-run far more + * often than intended — and every re-run is another chance for a transient + * DB error to be misread as "fresh install" and bounce visitors to setup. */ -let setupVerified = false; +const SETUP_VERIFIED_KEY = Symbol.for("emdash:setup-verified"); +const setupFlagStore = globalThis as Record; + +function isSetupVerified(): boolean { + return setupFlagStore[SETUP_VERIFIED_KEY] === true; +} + +function markSetupVerified(): void { + setupFlagStore[SETUP_VERIFIED_KEY] = true; +} /** * Get EmDash configuration from virtual module @@ -338,7 +355,7 @@ export const onRequest = defineMiddleware(async (context, next) => { // Do a one-time lightweight probe using the same getDb() instance the // page will use: if the migrations table doesn't exist, no migrations // have ever run -- redirect to the setup wizard. - if (!setupVerified) { + if (!isSetupVerified()) { const t0 = performance.now(); try { const { getDb } = await import("../loader.js"); @@ -348,10 +365,19 @@ export const onRequest = defineMiddleware(async (context, next) => { .selectAll() .limit(1) .execute(); - setupVerified = true; - } catch { - // Table doesn't exist -> fresh database, redirect to setup - return context.redirect("/_emdash/admin/setup"); + markSetupVerified(); + } catch (error) { + // Only a genuinely-missing migrations table means a fresh, + // un-set-up database — redirect to the setup wizard. + if (isMissingTableError(error)) { + return context.redirect("/_emdash/admin/setup"); + } + // Any other failure (transient D1/replica error, timeout, cold-start + // race, locked SQLite) must NOT be read as "fresh install" — doing so + // bounces real visitors on a set-up site to /_emdash/admin/setup. + // Leave the flag unset so a later request can re-verify, and fall + // through to render the page normally. + console.error("Setup probe failed (non-fatal):", error); } timings.push({ name: "setup", dur: performance.now() - t0, desc: "Setup probe" }); } @@ -368,7 +394,7 @@ export const onRequest = defineMiddleware(async (context, next) => { const t0 = performance.now(); try { const runtime = await getRuntime(config, initSubTimings); - setupVerified = true; + markSetupVerified(); const handlePublicPluginApiRoute = createPublicPluginApiRouteHandler(runtime); // eslint-disable-next-line typescript/no-unsafe-type-assertion -- partial object; getPageRuntime() only checks for the page-contribution methods locals.emdash = { @@ -448,7 +474,7 @@ export const onRequest = defineMiddleware(async (context, next) => { for (const sub of initSubTimings) timings.push(sub); // Runtime init runs migrations, so the DB is guaranteed set up - setupVerified = true; + markSetupVerified(); // The manifest is no longer pre-loaded here. It's admin-only // content that public/anonymous requests never read, and diff --git a/packages/core/tests/unit/astro/middleware-prerender.test.ts b/packages/core/tests/unit/astro/middleware-prerender.test.ts index 292505c8a..013171f7c 100644 --- a/packages/core/tests/unit/astro/middleware-prerender.test.ts +++ b/packages/core/tests/unit/astro/middleware-prerender.test.ts @@ -140,8 +140,30 @@ vi.mock("../../../src/loader.js", () => ({ import { createRequestScopedDb } from "virtual:emdash/dialect"; import onRequest from "../../../src/astro/middleware.js"; +import { getDb } from "../../../src/loader.js"; import { getRequestContext } from "../../../src/request-context.js"; +/** Reset the globalThis-backed "setup verified" singleton between tests. */ +const SETUP_VERIFIED_KEY = Symbol.for("emdash:setup-verified"); +function resetSetupVerified() { + delete (globalThis as Record)[SETUP_VERIFIED_KEY]; +} + +/** A getDb stub whose migrations-probe query throws `error`. */ +function getDbThatFailsProbe(error: Error) { + return { + selectFrom: () => ({ + selectAll: () => ({ + limit: () => ({ + execute: async () => { + throw error; + }, + }), + }), + }), + }; +} + function createAnonymousPublicPageContext(locals: Record = {}) { const cookies = { get: vi.fn((name: string) => { @@ -496,3 +518,75 @@ describe("astro middleware request-scoped db", () => { }); }); }); + +describe("astro middleware setup probe", () => { + beforeEach(() => { + // The "setup verified" flag is a globalThis singleton that latches once a + // probe (or runtime init) succeeds. Reset it so each test exercises a + // fresh probe. + resetSetupVerified(); + vi.mocked(createRequestScopedDb).mockReset().mockReturnValue(null); + vi.mocked(getDb).mockReset(); + }); + + /** Anonymous GET to a public frontend page (e.g. a category page). */ + function anonymousCategoryPageContext() { + const cookies = { + get: vi.fn((name: string) => { + if (name === "astro-session") return undefined; + return undefined; + }), + set: vi.fn(), + }; + const redirect = vi.fn( + (location: string) => new Response(null, { status: 302, headers: { Location: location } }), + ); + return { + context: { + request: new Request("https://example.com/category/news"), + url: new URL("https://example.com/category/news"), + cookies, + locals: {} as Record, + redirect, + isPrerendered: false, + session: { get: vi.fn(async () => null) }, + } as Record, + redirect, + }; + } + + it("redirects to setup when the migrations table is genuinely missing", async () => { + // Fresh, un-migrated database: the probe query reports a missing table. + vi.mocked(getDb).mockResolvedValue( + getDbThatFailsProbe(new Error("no such table: _emdash_migrations")) as never, + ); + + const { context, redirect } = anonymousCategoryPageContext(); + const next = vi.fn(async () => new Response("page")); + + const response = await onRequest(context as Parameters[0], next); + + expect(redirect).toHaveBeenCalledWith("/_emdash/admin/setup"); + expect(response.status).toBe(302); + expect(response.headers.get("Location")).toBe("/_emdash/admin/setup"); + expect(next).not.toHaveBeenCalled(); + }); + + it("does NOT redirect to setup on a transient DB error (regression)", async () => { + // A set-up site whose probe hits a transient failure (D1 connection + // loss, replica unavailable, timeout, locked SQLite) must keep serving + // the page — never bounce real visitors to the setup wizard. + vi.mocked(getDb).mockResolvedValue( + getDbThatFailsProbe(new Error("D1_ERROR: Network connection lost")) as never, + ); + + const { context, redirect } = anonymousCategoryPageContext(); + const next = vi.fn(async () => new Response("page")); + + const response = await onRequest(context as Parameters[0], next); + + expect(redirect).not.toHaveBeenCalled(); + expect(next).toHaveBeenCalledTimes(1); + expect(response.status).toBe(200); + }); +});