mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix(cli): fall back to node:sqlite when better-sqlite3 is unavailable in omniroute doctor (#7586) (#8501)
bin/cli/sqlite.mjs::loadSqlite() had no fallback beyond better-sqlite3, unlike the real server's driver cascade (src/lib/db/adapters/driverFactory.ts::tryOpenSync, which tries bun:sqlite -> better-sqlite3 -> node:sqlite). On machines without a working better-sqlite3 native binary, every `omniroute doctor` DB check reported a false FAIL even when the actual server was healthy via its own driver cascade. openSqliteDatabase() now falls back to tryOpenSync() when better-sqlite3 fails to import, reusing the same already-tested cascade the real server uses instead of re-deriving a second one. Co-authored-by: ikelvingo <im.kelvinwong@gmail.com>
This commit is contained in:
committed by
GitHub
parent
55ff236023
commit
13cc128dae
@@ -5,15 +5,34 @@ import { ensureSettingsSchema, hashManagementPassword, updateSettings } from "./
|
||||
|
||||
async function loadSqlite() {
|
||||
if (process.versions.bun) {
|
||||
return (await import("bun:sqlite")).Database;
|
||||
return { Database: (await import("bun:sqlite")).Database };
|
||||
}
|
||||
try {
|
||||
return (await import("better-sqlite3")).default;
|
||||
} catch {
|
||||
throw new Error("better-sqlite3 is not installed. Run npm install before using setup.");
|
||||
return { Database: (await import("better-sqlite3")).default };
|
||||
} catch (error) {
|
||||
return { error };
|
||||
}
|
||||
}
|
||||
|
||||
// #7586: unlike the real server (src/lib/db/adapters/driverFactory.ts::tryOpenSync),
|
||||
// this CLI helper historically had NO fallback beyond better-sqlite3 — so on any
|
||||
// machine where better-sqlite3's native binary is unavailable (Windows without a
|
||||
// prebuilt addon, etc.), every `omniroute doctor` DB check reported a false FAIL
|
||||
// even when the actual server was healthy via its own (correct) driver cascade.
|
||||
// Reuse that same cascade here instead of re-deriving it.
|
||||
async function openWithSyncDriverFallback(dbPath, options, importError) {
|
||||
try {
|
||||
const { tryOpenSync } = await import("../../src/lib/db/adapters/driverFactory.ts");
|
||||
const adapter = tryOpenSync(dbPath, options);
|
||||
if (adapter) {
|
||||
return adapter;
|
||||
}
|
||||
} catch {
|
||||
// fall through to the original better-sqlite3 error below
|
||||
}
|
||||
throw createSqliteNativeError(importError);
|
||||
}
|
||||
|
||||
function openBunSqlite(Database, dbPath, options) {
|
||||
const raw = new Database(dbPath, options);
|
||||
const prepare = (sql) => {
|
||||
@@ -91,19 +110,25 @@ export function createSqliteNativeError(error) {
|
||||
}
|
||||
|
||||
async function openSqliteDatabase(dbPath, options = {}) {
|
||||
const Database = await loadSqlite();
|
||||
const loaded = await loadSqlite();
|
||||
if (process.versions.bun) {
|
||||
if (options.fileMustExist && !fs.existsSync(dbPath)) {
|
||||
throw new Error(`SQLite file does not exist: ${dbPath}`);
|
||||
}
|
||||
options = options.readonly
|
||||
const bunOptions = options.readonly
|
||||
? { readonly: true }
|
||||
: { readwrite: true, create: options.fileMustExist !== true };
|
||||
try {
|
||||
return openBunSqlite(loaded.Database, dbPath, bunOptions);
|
||||
} catch (error) {
|
||||
throw createSqliteNativeError(error);
|
||||
}
|
||||
}
|
||||
if (loaded.error) {
|
||||
return openWithSyncDriverFallback(dbPath, options, loaded.error);
|
||||
}
|
||||
try {
|
||||
return process.versions.bun
|
||||
? openBunSqlite(Database, dbPath, options)
|
||||
: new Database(dbPath, options);
|
||||
return new loaded.Database(dbPath, options);
|
||||
} catch (error) {
|
||||
throw createSqliteNativeError(error);
|
||||
}
|
||||
|
||||
1
changelog.d/fixes/7586-windows-sqlite-launch.md
Normal file
1
changelog.d/fixes/7586-windows-sqlite-launch.md
Normal file
@@ -0,0 +1 @@
|
||||
- fix(cli): fall back to the node:sqlite driver cascade in `bin/cli/sqlite.mjs` so `omniroute doctor` no longer reports a false "FAIL Database"/"FAIL Storage/encryption" on machines without a working better-sqlite3 native binary (#7586)
|
||||
81
tests/unit/cli-sqlite-no-fallback-7586.test.ts
Normal file
81
tests/unit/cli-sqlite-no-fallback-7586.test.ts
Normal file
@@ -0,0 +1,81 @@
|
||||
import { test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { register } from "node:module";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { DatabaseSync } from "node:sqlite";
|
||||
|
||||
// #7586: `omniroute doctor`'s "Database" and "Storage/encryption" checks call
|
||||
// readDatabaseHealth()/readEncryptedCredentialSamples() from bin/cli/sqlite.mjs,
|
||||
// which — unlike the real server's driver cascade
|
||||
// (src/lib/db/adapters/driverFactory.ts::tryOpenSync, which tries
|
||||
// bun:sqlite -> better-sqlite3 -> node:sqlite) — used to have NO fallback beyond
|
||||
// better-sqlite3. On any machine where better-sqlite3's native binary is
|
||||
// unavailable (Windows without a prebuilt addon, per @jmaxdev's report), doctor
|
||||
// would ALWAYS report "FAIL Database" / "FAIL Storage/encryption" even when the
|
||||
// real server was perfectly healthy via its own resilient driver selection.
|
||||
//
|
||||
// This test simulates that exact machine by intercepting the ESM specifier
|
||||
// "better-sqlite3" (the one `bin/cli/sqlite.mjs::loadSqlite()` imports) so it
|
||||
// throws the same "Could not locate the bindings file" error jmax hit, then
|
||||
// proves readDatabaseHealth() still succeeds by falling back to node:sqlite.
|
||||
register(
|
||||
"data:text/javascript," +
|
||||
encodeURIComponent(`
|
||||
export async function resolve(specifier, context, nextResolve) {
|
||||
if (specifier === "better-sqlite3") {
|
||||
return {
|
||||
url:
|
||||
"data:text/javascript," +
|
||||
encodeURIComponent(
|
||||
"throw new Error(" +
|
||||
JSON.stringify(
|
||||
"Could not locate the bindings file. Tried:\\n \\u2192 /fake/path/better_sqlite3.node"
|
||||
) +
|
||||
");"
|
||||
),
|
||||
shortCircuit: true,
|
||||
};
|
||||
}
|
||||
return nextResolve(specifier, context);
|
||||
}
|
||||
`),
|
||||
import.meta.url
|
||||
);
|
||||
|
||||
const { readDatabaseHealth } = await import("../../bin/cli/sqlite.mjs");
|
||||
|
||||
test("#7586: readDatabaseHealth() falls back to node:sqlite when better-sqlite3 is unavailable", async (t) => {
|
||||
const dbPath = path.join(os.tmpdir(), `cli-sqlite-no-fallback-7586-${Date.now()}.sqlite`);
|
||||
t.after(() => {
|
||||
try {
|
||||
fs.unlinkSync(dbPath);
|
||||
} catch {}
|
||||
});
|
||||
|
||||
// Seed a normal, healthy DB using node:sqlite directly — simulating what the
|
||||
// ACTUAL server (which DOES have a node:sqlite fallback) would have created
|
||||
// on a machine without a better-sqlite3 native binary.
|
||||
const seed = new DatabaseSync(dbPath);
|
||||
seed.exec(
|
||||
"CREATE TABLE _omniroute_migrations (version TEXT PRIMARY KEY, name TEXT NOT NULL, applied_at TEXT);" +
|
||||
"INSERT INTO _omniroute_migrations (version, name, applied_at) VALUES ('001', 'initial_schema', datetime('now'));"
|
||||
);
|
||||
seed.close();
|
||||
|
||||
// Exercise the CLI helper's own health check — the exact function
|
||||
// `omniroute doctor` calls (bin/cli/commands/doctor.mjs:checkDatabase ->
|
||||
// readDatabaseHealth). Before the fix this threw "better-sqlite3 is not
|
||||
// installed..." even though the DB is perfectly healthy.
|
||||
const result = await readDatabaseHealth(dbPath);
|
||||
|
||||
assert.equal(
|
||||
result.quickCheckValue,
|
||||
"ok",
|
||||
"readDatabaseHealth() should report a healthy DB via the node:sqlite fallback, " +
|
||||
"not throw, when better-sqlite3 is unavailable (#7586)"
|
||||
);
|
||||
assert.equal(result.hasMigrationTable, true);
|
||||
assert.deepEqual(result.appliedMigrationVersions, ["001"]);
|
||||
});
|
||||
Reference in New Issue
Block a user