Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/setup-probe-transient-error.md
Original file line number Diff line number Diff line change
@@ -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).
42 changes: 34 additions & 8 deletions packages/core/src/astro/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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<symbol, unknown>;

function isSetupVerified(): boolean {
return setupFlagStore[SETUP_VERIFIED_KEY] === true;
}

function markSetupVerified(): void {
setupFlagStore[SETUP_VERIFIED_KEY] = true;
}

/**
* Get EmDash configuration from virtual module
Expand Down Expand Up @@ -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");
Expand All @@ -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" });
}
Expand All @@ -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 = {
Expand Down Expand Up @@ -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
Expand Down
94 changes: 94 additions & 0 deletions packages/core/tests/unit/astro/middleware-prerender.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<symbol, unknown>)[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<string, unknown> = {}) {
const cookies = {
get: vi.fn((name: string) => {
Expand Down Expand Up @@ -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<string, unknown>,
redirect,
isPrerendered: false,
session: { get: vi.fn(async () => null) },
} as Record<string, unknown>,
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<typeof onRequest>[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<typeof onRequest>[0], next);

expect(redirect).not.toHaveBeenCalled();
expect(next).toHaveBeenCalledTimes(1);
expect(response.status).toBe(200);
});
});
Loading