Skip to content

Commit 300ceb8

Browse files
betegonBYK
authored andcommitted
fix(db): return empty arrays from all()/values() on readonly database
The readonly error handler used a bare `return` for all traced methods, yielding `undefined` even for all() and values() which callers expect to return arrays. This would crash on .map()/.length if a write query ever used these methods (e.g. DELETE ... RETURNING *). Extract handleReadonlyError() helper that returns [] for all/values and undefined for run/get. Also reduces complexity in the proxy handler.
1 parent b80b6d9 commit 300ceb8

File tree

2 files changed

+31
-2
lines changed

2 files changed

+31
-2
lines changed

src/lib/telemetry.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,19 @@ export function resetReadonlyWarning(): void {
630630
/** Methods on SQLite Statement that execute queries and should be traced */
631631
const TRACED_STATEMENT_METHODS = ["get", "run", "all", "values"] as const;
632632

633+
/**
634+
* Handle a readonly database error by warning the user and returning a
635+
* type-appropriate no-op value. Returns `undefined` for run/get (void / no-row)
636+
* and `[]` for all/values (empty result set).
637+
*/
638+
function handleReadonlyError(method: string | symbol): unknown {
639+
warnReadonlyDatabaseOnce();
640+
if (method === "all" || method === "values") {
641+
return [];
642+
}
643+
return;
644+
}
645+
633646
/**
634647
* Wrap a SQLite Statement to automatically trace query execution.
635648
*
@@ -689,8 +702,7 @@ function createTracedStatement<T>(stmt: T, sql: string): T {
689702
// Handle readonly database gracefully: warn once, skip the write.
690703
// The CLI still works — reads succeed, only caching/persistence is lost.
691704
if (isReadonlyError(error)) {
692-
warnReadonlyDatabaseOnce();
693-
return;
705+
return handleReadonlyError(prop);
694706
}
695707

696708
// Re-throw if repair didn't help or wasn't applicable

test/lib/telemetry.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,23 @@ describe("createTracedDatabase", () => {
808808
db.close();
809809
});
810810

811+
test("all() and values() return empty arrays on readonly write", () => {
812+
const db = new Database(dbPath);
813+
const tracedDb = createTracedDatabase(db);
814+
815+
const allResult = tracedDb
816+
.query("INSERT INTO test (id, name) VALUES (?, ?)")
817+
.all(2, "Bob");
818+
const valuesResult = tracedDb
819+
.query("INSERT INTO test (id, name) VALUES (?, ?)")
820+
.values(3, "Charlie");
821+
822+
expect(allResult).toEqual([]);
823+
expect(valuesResult).toEqual([]);
824+
825+
db.close();
826+
});
827+
811828
test("warning message includes helpful instructions", () => {
812829
const db = new Database(dbPath);
813830
const tracedDb = createTracedDatabase(db);

0 commit comments

Comments
 (0)