From a87c9236ff0ee3aaf558f7a8f7eb983b2c795c84 Mon Sep 17 00:00:00 2001 From: Nick Sullivan <142708+TechNickAI@users.noreply.github.com> Date: Mon, 17 Aug 2026 06:09:47 -0500 Subject: [PATCH] Database settings page returns HTTP 500 when SQLite lacks the optional dbstat table (#10558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(db): tolerate a SQLite build without the dbstat virtual table getDatabaseStats() queried `dbstat` once per table with no guard. `dbstat` is compile-time optional (ENABLE_DBSTAT_VTAB) and is absent from sql.js/WASM builds, so on those runtimes the query throws and the error propagates out of getDatabaseStats(). Every caller dies with it. Most visibly, GET and PATCH /api/settings/database return HTTP 500, which makes the entire database settings page unusable — users cannot read or change page size, cache size, or vacuum settings. The function already anticipated missing virtual-table modules: the COUNT(*) lookup a few lines above swallows "no such module:" errors. The dbstat query simply sat outside that guard. Probe dbstat once per call and skip the per-table size lookups when it is unavailable, reporting size 0. Database-level figures (total size, page count, cache size) come from pragmas and stay accurate; only per-table byte sizes are lost, which is the correct trade against a hard 500. Unrelated failures (I/O errors, corruption) still propagate. Both spellings are handled: sql.js reports "no such module: dbstat" while better-sqlite3 can surface "no such table: dbstat". * test(db): cover prefixed driver errors and dbstat edge cases Review follow-up on the previous commit. The guard is deliberately unanchored because real drivers stringify errors with their class name attached ("SqliteError: no such table: dbstat", "RuntimeError: ..."). Nothing pinned that, so anchoring the regex would have passed the suite while silently breaking every real driver. Add a case for the prefixed form; it fails if a caret is introduced. Also cover three shapes the fake previously could not express: - a database with no user tables, which is what a fresh install hits first - SUM(pgsize) returning NULL for a table occupying no pages - dbstat answering the probe but failing on a later table, which documents that a mid-iteration fault still propagates rather than being mistaken for an absent module Correct the source comment: the two error spellings track the SQLite build, not the driver package, so the earlier attribution to better-sqlite3 was wrong. * docs(changelog): add fragment for the dbstat availability guard Registers the new test with Stryker alongside the sibling db suites and adds the changelog fragment for this fix. --------- Co-authored-by: Nick Sullivan --- changelog.d/fixes/dbstat-optional-vtab.md | 1 + src/lib/db/stats.ts | 39 +++- stryker.conf.json | 1 + tests/unit/db/stats-dbstat-optional.test.ts | 189 ++++++++++++++++++++ 4 files changed, 225 insertions(+), 5 deletions(-) create mode 100644 changelog.d/fixes/dbstat-optional-vtab.md create mode 100644 tests/unit/db/stats-dbstat-optional.test.ts diff --git a/changelog.d/fixes/dbstat-optional-vtab.md b/changelog.d/fixes/dbstat-optional-vtab.md new file mode 100644 index 0000000000..5d56e93d1a --- /dev/null +++ b/changelog.d/fixes/dbstat-optional-vtab.md @@ -0,0 +1 @@ +- **fix(db):** database settings API no longer returns HTTP 500 on SQLite builds compiled without the optional `dbstat` virtual table (sql.js/WASM); per-table sizes degrade to 0 instead of failing the whole stats call diff --git a/src/lib/db/stats.ts b/src/lib/db/stats.ts index e1a824ad90..e640231794 100644 --- a/src/lib/db/stats.ts +++ b/src/lib/db/stats.ts @@ -24,6 +24,28 @@ export interface DatabaseStats { cacheSize: number; } +/** + * `dbstat` is a compile-time-optional SQLite virtual table (ENABLE_DBSTAT_VTAB). + * Builds without it — sql.js/WASM among them — reject the query with either + * "no such module: dbstat" or "no such table: dbstat" depending on the build, + * and drivers prefix their error class onto the message, so match loosely. + * + * Per-table byte sizes are a nice-to-have, so probe once and degrade to 0 + * rather than failing the whole stats call — and with it every caller, + * including the database settings API. + */ +function isDbstatAvailable(db: SqliteAdapter): boolean { + try { + db.prepare(`SELECT SUM(pgsize) as size FROM dbstat WHERE name = ?`).get("sqlite_master"); + return true; + } catch (error) { + if (error instanceof Error && /no such (module|table): dbstat/i.test(error.message)) { + return false; + } + throw error; + } +} + export function getDatabaseStats(db: SqliteAdapter = getDbInstance()): DatabaseStats { const pageSize = db.pragma("page_size", { simple: true }) as number; const pageCount = db.pragma("page_count", { simple: true }) as number; @@ -36,12 +58,15 @@ export function getDatabaseStats(db: SqliteAdapter = getDbInstance()): DatabaseS ) .all() as Array<{ name: string }>; + const dbstatAvailable = isDbstatAvailable(db); + const tableStats = tables.map((table) => { let rowCount = 0; try { const quotedName = `"${table.name.replaceAll('"', '""')}"`; const row = db.prepare(`SELECT COUNT(*) as count FROM ${quotedName}`).get() as - { count: number } | undefined; + | { count: number } + | undefined; rowCount = row?.count ?? 0; } catch (error) { if (!(error instanceof Error) || !error.message.startsWith("no such module:")) { @@ -50,14 +75,18 @@ export function getDatabaseStats(db: SqliteAdapter = getDbInstance()): DatabaseS // Optional virtual-table modules may be unavailable on this connection. } - const tableSize = db - .prepare(`SELECT SUM(pgsize) as size FROM dbstat WHERE name = ?`) - .get(table.name) as { size: number | null }; + let size = 0; + if (dbstatAvailable) { + const tableSize = db + .prepare(`SELECT SUM(pgsize) as size FROM dbstat WHERE name = ?`) + .get(table.name) as { size: number | null } | undefined; + size = tableSize?.size || 0; + } return { name: table.name, rowCount, - size: tableSize?.size || 0, + size, }; }); diff --git a/stryker.conf.json b/stryker.conf.json index 2a5cc5eefc..13f95dd37c 100644 --- a/stryker.conf.json +++ b/stryker.conf.json @@ -336,6 +336,7 @@ "tests/unit/sse-auth-antigravity-credits.test.ts", "tests/unit/sse-auth-resource-404.test.ts", "tests/unit/sse-auth.test.ts", + "tests/unit/db/stats-dbstat-optional.test.ts", "tests/unit/stream-early-eof-breaker.test.ts", "tests/unit/stream-readiness.test.ts", "tests/unit/strict-random-deck.test.ts", diff --git a/tests/unit/db/stats-dbstat-optional.test.ts b/tests/unit/db/stats-dbstat-optional.test.ts new file mode 100644 index 0000000000..7f92c1a062 --- /dev/null +++ b/tests/unit/db/stats-dbstat-optional.test.ts @@ -0,0 +1,189 @@ +/** + * getDatabaseStats() must survive a SQLite build without the `dbstat` virtual + * table. + * + * `dbstat` is compile-time optional (ENABLE_DBSTAT_VTAB) and is absent from + * sql.js/WASM builds. Before the fix, the unguarded per-table `SELECT SUM(pgsize) + * FROM dbstat` threw, which propagated out of getDatabaseStats() and made + * GET/PATCH /api/settings/database return HTTP 500 — the whole database settings + * page became unusable on those runtimes. + */ + +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { getDatabaseStats } from "@/lib/db/stats"; +import type { PreparedStatement, SqliteAdapter } from "@/lib/db/adapters/types"; + +type FakeOptions = { + /** Error message thrown by any statement touching `dbstat`. */ + dbstatError?: string; + /** Tables reported by sqlite_master. */ + tables?: string[]; + /** Make the dbstat probe succeed but fail for this specific table. */ + failOnlyOn?: string; + /** Return `{ size: null }` from dbstat, as SUM() does over an empty table. */ + nullSize?: boolean; +}; + +/** + * Minimal in-memory SqliteAdapter double. Only the surface getDatabaseStats() + * actually touches is implemented; everything else throws so an accidental new + * dependency shows up loudly instead of silently passing. + */ +function createFakeDb({ + dbstatError, + tables = ["alpha", "beta"], + failOnlyOn, + nullSize, +}: FakeOptions = {}): SqliteAdapter { + const prepare = (sql: string): PreparedStatement => { + const touchesDbstat = /\bdbstat\b/i.test(sql); + + return { + run() { + throw new Error(`unexpected run(): ${sql}`); + }, + get(...params: unknown[]) { + if (touchesDbstat) { + const probing = params[0] === "sqlite_master"; + // `failOnlyOn` models a driver that answers the probe but fails later. + if (failOnlyOn) { + if (params[0] === failOnlyOn) throw new Error(dbstatError ?? "no such table: dbstat"); + } else if (dbstatError) { + throw new Error(dbstatError); + } + if (probing) return { size: 0 }; + return { size: nullSize ? null : 4096 }; + } + if (/COUNT\(\*\)/i.test(sql)) return { count: 7 }; + throw new Error(`unexpected get(): ${sql}`); + }, + all() { + if (/type='table'/i.test(sql)) return tables.map((name) => ({ name })); + if (/type='index'/i.test(sql)) { + return tables.length ? [{ name: "idx_alpha", tableName: "alpha" }] : []; + } + throw new Error(`unexpected all(): ${sql}`); + }, + }; + }; + + return { + driver: "sql.js", + open: true, + name: ":memory:", + prepare, + exec() {}, + pragma(pragmaStr: string) { + if (pragmaStr === "page_size") return 4096; + if (pragmaStr === "page_count") return 100; + if (pragmaStr === "cache_size") return -65536; + throw new Error(`unexpected pragma: ${pragmaStr}`); + }, + transaction(fn: (...args: unknown[]) => T) { + return fn; + }, + immediate(fn: () => void) { + fn(); + }, + async backup() {}, + checkpoint() {}, + close() {}, + raw: null, + } satisfies SqliteAdapter; +} + +test("getDatabaseStats reports per-table sizes when dbstat is available", () => { + const stats = getDatabaseStats(createFakeDb()); + + assert.equal(stats.totalSize, 4096 * 100); + assert.deepEqual( + stats.tables.map((t) => [t.name, t.rowCount, t.size]), + [ + ["alpha", 7, 4096], + ["beta", 7, 4096], + ] + ); +}); + +test("getDatabaseStats degrades to size 0 when dbstat module is missing", () => { + const stats = getDatabaseStats(createFakeDb({ dbstatError: "no such module: dbstat" })); + + // The call must succeed; only per-table byte sizes are lost. + assert.deepEqual( + stats.tables.map((t) => [t.name, t.rowCount, t.size]), + [ + ["alpha", 7, 0], + ["beta", 7, 0], + ] + ); + // Database-level numbers come from pragmas and stay accurate. + assert.equal(stats.totalSize, 4096 * 100); + assert.equal(stats.pageCount, 100); + assert.equal(stats.cacheSize, -65536); + assert.equal(stats.indexes.length, 1); +}); + +test("getDatabaseStats degrades when the driver reports dbstat as a missing table", () => { + // SQLite builds lacking ENABLE_DBSTAT_VTAB commonly report this variant. + const stats = getDatabaseStats(createFakeDb({ dbstatError: "no such table: dbstat" })); + + assert.deepEqual( + stats.tables.map((t) => t.size), + [0, 0] + ); +}); + +test("getDatabaseStats degrades when the driver prefixes its error class", () => { + // Real drivers stringify as "SqliteError: ..." / "RuntimeError: ...", so the + // guard must not be anchored to the start of the message. + for (const message of [ + "SqliteError: no such table: dbstat", + "RuntimeError: no such module: dbstat", + ]) { + const stats = getDatabaseStats(createFakeDb({ dbstatError: message })); + assert.deepEqual( + stats.tables.map((t) => t.size), + [0, 0], + `expected degradation for ${message}` + ); + } +}); + +test("getDatabaseStats handles a database with no user tables", () => { + // The shape a fresh install hits before any migration has run. + const stats = getDatabaseStats(createFakeDb({ tables: [] })); + + assert.deepEqual(stats.tables, []); + assert.deepEqual(stats.indexes, []); + assert.equal(stats.totalSize, 4096 * 100); +}); + +test("getDatabaseStats maps a NULL dbstat sum to 0", () => { + // SUM(pgsize) returns NULL when a table occupies no pages. + const stats = getDatabaseStats(createFakeDb({ nullSize: true })); + + assert.deepEqual( + stats.tables.map((t) => t.size), + [0, 0] + ); +}); + +test("getDatabaseStats propagates a dbstat failure that appears after the probe", () => { + // Documents current behaviour: the probe establishes availability once, so a + // later per-table failure is treated as a genuine fault rather than a missing + // module. Anything else would mask real I/O errors mid-iteration. + assert.throws( + () => getDatabaseStats(createFakeDb({ failOnlyOn: "beta" })), + /no such table: dbstat/ + ); +}); + +test("getDatabaseStats still propagates unrelated dbstat failures", () => { + // A genuine fault (disk I/O, corruption) must not be silently swallowed. + assert.throws( + () => getDatabaseStats(createFakeDb({ dbstatError: "database disk image is malformed" })), + /database disk image is malformed/ + ); +});