authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-10-03 23:35:32-07:00
committergravatar for noreply@github.comGitHub <noreply@github.com> 2026-10-03 23:35:32-07:00
log0fe4fa40fe65ac9545621a65633f08d9fa29fd0d
tree3c0e1ce4fc90122af26ab2ef8c61c7fa268caa3a
parent40596072ebabf045414f54ca6ffec3911543d8ff
signature Signed by PGP key B5690EEEBB952194

fix(server): sqlite transactions wait for the write lock instead of failing (#15488)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

2 files changed, 114 insertions(+), 24 deletions(-)

packages/shared/src/nodeSqliteClient.test.ts+80
...@@ -61,8 +61,88 @@ layer("NodeSqliteClient", (it) => {...@@ -61,8 +61,88 @@ layer("NodeSqliteClient", (it) => {
61 assert.equal(error.reason.operation, "prepare");61 assert.equal(error.reason.operation, "prepare");
62 }),62 }),
63 );63 );
64
65 it.effect("classifies constraint failures by their SQLite result code", () =>
66 Effect.gen(function* () {
67 const sql = yield* SqlClient.SqlClient;
68 yield* sql`CREATE TABLE constrained(name TEXT NOT NULL UNIQUE)`;
69 yield* sql`INSERT INTO constrained VALUES ('taken')`;
70
71 const duplicate = yield* sql`INSERT INTO constrained VALUES ('taken')`.pipe(Effect.flip);
72 assert(duplicate.reason._tag === "UniqueViolation");
73 assert.equal(duplicate.reason.constraint, "constrained.name");
74
75 const missing = yield* sql`INSERT INTO constrained VALUES (NULL)`.pipe(Effect.flip);
76 assert.equal(missing.reason._tag, "ConstraintError");
77 }),
78 );
64});79});
6580
81const makeTempDatabase = Effect.gen(function* () {
82 const fs = yield* FileSystem.FileSystem;
83 const path = yield* Path.Path;
84 const directory = yield* fs.makeTempDirectoryScoped({ prefix: "t3-sqlite-transaction-" });
85 const filename = path.join(directory, "state.sqlite");
86 // node:sqlite connections fail a busy statement at once unless given a timeout.
87 const other = yield* Effect.acquireRelease(
88 Effect.sync(() => new NodeSqlite.DatabaseSync(filename)),
89 (database) => Effect.sync(() => database.close()),
90 );
91 yield* Effect.sync(() => {
92 other.exec(`
93 PRAGMA journal_mode = WAL;
94 CREATE TABLE counters(id INTEGER PRIMARY KEY, value INTEGER NOT NULL);
95 INSERT INTO counters VALUES (1, 0);
96 `);
97 });
98 return { filename, other };
99});
100
101it.effect("keeps another connection from committing between a transaction's read and write", () =>
102 Effect.gen(function* () {
103 const { filename, other } = yield* makeTempDatabase;
104 yield* Effect.gen(function* () {
105 const sql = yield* SqlClient.SqlClient;
106 yield* sql.withTransaction(
107 Effect.gen(function* () {
108 const [row] = yield* sql<{ readonly value: number }>`
109 SELECT value FROM counters WHERE id = 1
110 `;
111 // With a deferred BEGIN this commit lands and the write below fails
112 // with SQLITE_BUSY_SNAPSHOT, which no busy timeout can wait out.
113 yield* Effect.sync(() =>
114 assert.throws(
115 () => other.exec("UPDATE counters SET value = value + 1 WHERE id = 1"),
116 /database is locked/,
117 ),
118 );
119 yield* sql`UPDATE counters SET value = ${(row?.value ?? 0) + 10} WHERE id = 1`;
120 }),
121 );
122 yield* Effect.sync(() => other.exec("UPDATE counters SET value = value + 1 WHERE id = 1"));
123 assert.deepEqual(yield* sql`SELECT value FROM counters WHERE id = 1`.values, [[11]]);
124 }).pipe(Effect.provide(SqliteClient.layer({ filename })));
125 }).pipe(Effect.provide(NodeServices.layer)),
126);
127
128it.effect("reports a transaction blocked by another writer as a lock timeout", () =>
129 Effect.gen(function* () {
130 const { filename, other } = yield* makeTempDatabase;
131 yield* Effect.gen(function* () {
132 const sql = yield* SqlClient.SqlClient;
133 yield* sql`PRAGMA busy_timeout = 0`;
134 const read = sql.withTransaction(sql`SELECT value FROM counters WHERE id = 1`.values);
135
136 yield* Effect.sync(() => other.exec("BEGIN IMMEDIATE"));
137 const error = yield* read.pipe(Effect.flip);
138 assert.equal(error.reason._tag, "LockTimeoutError");
139
140 yield* Effect.sync(() => other.exec("COMMIT"));
141 assert.deepEqual(yield* read, [[0]]);
142 }).pipe(Effect.provide(SqliteClient.layer({ filename })));
143 }).pipe(Effect.provide(NodeServices.layer)),
144);
145
66it.effect("returns a typed failure when the database cannot be opened", () =>146it.effect("returns a typed failure when the database cannot be opened", () =>
67 Effect.gen(function* () {147 Effect.gen(function* () {
68 const error = yield* Effect.flip(148 const error = yield* Effect.flip(
packages/shared/src/nodeSqliteClient.ts+34-24
...@@ -13,6 +13,7 @@ import * as Exit from "effect/Exit";...@@ -13,6 +13,7 @@ import * as Exit from "effect/Exit";
13import * as Fiber from "effect/Fiber";13import * as Fiber from "effect/Fiber";
14import { identity } from "effect/Function";14import { identity } from "effect/Function";
15import * as Layer from "effect/Layer";15import * as Layer from "effect/Layer";
16import * as Predicate from "effect/Predicate";
16import * as Schema from "effect/Schema";17import * as Schema from "effect/Schema";
17import * as Scope from "effect/Scope";18import * as Scope from "effect/Scope";
18import * as Semaphore from "effect/Semaphore";19import * as Semaphore from "effect/Semaphore";
...@@ -82,6 +83,22 @@ const checkNodeSqliteCompat = () => {...@@ -82,6 +83,22 @@ const checkNodeSqliteCompat = () => {
82 return Effect.void;83 return Effect.void;
83};84};
8485
86/**
87 * `node:sqlite` reports the SQLite result code as `errcode`, while
88 * `classifySqliteError` reads `errno`. Copy it across so busy, locked and
89 * constraint failures get their own reasons instead of `UnknownError`.
90 */
91const classifyError = (cause: unknown, message: string, operation: string) => {
92 if (
93 Predicate.hasProperty(cause, "errcode") &&
94 typeof cause.errcode === "number" &&
95 !Predicate.hasProperty(cause, "errno")
96 ) {
97 Object.assign(cause, { errno: cause.errcode });
98 }
99 return classifySqliteError(cause, { message, operation });
100};
101
85const make = Effect.fn("makeWithDatabase")(function* (102const make = Effect.fn("makeWithDatabase")(function* (
86 options: SqliteClientConfig,103 options: SqliteClientConfig,
87): Effect.fn.Return<Client.SqlClient, SqlError, Scope.Scope | Reactivity.Reactivity> {104): Effect.fn.Return<Client.SqlClient, SqlError, Scope.Scope | Reactivity.Reactivity> {
...@@ -102,10 +119,7 @@ const make = Effect.fn("makeWithDatabase")(function* (...@@ -102,10 +119,7 @@ const make = Effect.fn("makeWithDatabase")(function* (
102 }),119 }),
103 catch: (cause) =>120 catch: (cause) =>
104 new SqlError({121 new SqlError({
105 reason: classifySqliteError(cause, {122 reason: classifyError(cause, "Failed to open database", "open"),
106 message: "Failed to open database",
107 operation: "open",
108 }),
109 }),123 }),
110 });124 });
111 yield* Scope.addFinalizer(125 yield* Scope.addFinalizer(
...@@ -114,10 +128,7 @@ const make = Effect.fn("makeWithDatabase")(function* (...@@ -114,10 +128,7 @@ const make = Effect.fn("makeWithDatabase")(function* (
114 try: () => db.close(),128 try: () => db.close(),
115 catch: (cause) =>129 catch: (cause) =>
116 new SqlError({130 new SqlError({
117 reason: classifySqliteError(cause, {131 reason: classifyError(cause, "Failed to close database", "close"),
118 message: "Failed to close database",
119 operation: "close",
120 }),
121 }),132 }),
122 }).pipe(Effect.orDie),133 }).pipe(Effect.orDie),
123 );134 );
...@@ -138,10 +149,7 @@ const make = Effect.fn("makeWithDatabase")(function* (...@@ -138,10 +149,7 @@ const make = Effect.fn("makeWithDatabase")(function* (
138 try: () => db.prepare(sql),149 try: () => db.prepare(sql),
139 catch: (cause) =>150 catch: (cause) =>
140 new SqlError({151 new SqlError({
141 reason: classifySqliteError(cause, {152 reason: classifyError(cause, "Failed to prepare statement", "prepare"),
142 message: "Failed to prepare statement",
143 operation: "prepare",
144 }),
145 }),153 }),
146 });154 });
147155
...@@ -168,10 +176,7 @@ const make = Effect.fn("makeWithDatabase")(function* (...@@ -168,10 +176,7 @@ const make = Effect.fn("makeWithDatabase")(function* (
168 } catch (cause) {176 } catch (cause) {
169 return Effect.fail(177 return Effect.fail(
170 new SqlError({178 new SqlError({
171 reason: classifySqliteError(cause, {179 reason: classifyError(cause, "Failed to execute statement", "execute"),
172 message: "Failed to execute statement",
173 operation: "execute",
174 }),
175 }),180 }),
176 );181 );
177 }182 }
...@@ -201,10 +206,7 @@ const make = Effect.fn("makeWithDatabase")(function* (...@@ -201,10 +206,7 @@ const make = Effect.fn("makeWithDatabase")(function* (
201 },206 },
202 catch: (cause) =>207 catch: (cause) =>
203 new SqlError({208 new SqlError({
204 reason: classifySqliteError(cause, {209 reason: classifyError(cause, "Failed to execute statement", "execute"),
205 message: "Failed to execute statement",
206 operation: "execute",
207 }),
208 }),210 }),
209 }),211 }),
210 (statement) =>212 (statement) =>
...@@ -216,10 +218,11 @@ const make = Effect.fn("makeWithDatabase")(function* (...@@ -216,10 +218,11 @@ const make = Effect.fn("makeWithDatabase")(function* (
216 },218 },
217 catch: (cause) =>219 catch: (cause) =>
218 new SqlError({220 new SqlError({
219 reason: classifySqliteError(cause, {221 reason: classifyError(
220 message: "Failed to reset statement result mode",222 cause,
221 operation: "resetResultMode",223 "Failed to reset statement result mode",
222 }),224 "resetResultMode",
225 ),
223 }),226 }),
224 }).pipe(Effect.orDie),227 }).pipe(Effect.orDie),
225 );228 );
...@@ -273,6 +276,13 @@ const make = Effect.fn("makeWithDatabase")(function* (...@@ -273,6 +276,13 @@ const make = Effect.fn("makeWithDatabase")(function* (
273 acquirer,276 acquirer,
274 compiler,277 compiler,
275 transactionAcquirer,278 transactionAcquirer,
279 // A deferred BEGIN only takes the write lock at the first write. If another
280 // process commits after this transaction's first read, that write fails at
281 // once with SQLITE_BUSY_SNAPSHOT, which busy_timeout cannot wait out. Taking
282 // the lock up front makes it wait instead, at the cost of serializing
283 // read-only transactions behind other processes' writers. Read-only
284 // connections cannot write, so they keep the deferred BEGIN.
285 beginTransaction: options.readonly === true ? "BEGIN" : "BEGIN IMMEDIATE",
276 spanAttributes: [286 spanAttributes: [
277 ...(options.spanAttributes ? Object.entries(options.spanAttributes) : []),287 ...(options.spanAttributes ? Object.entries(options.spanAttributes) : []),
278 [ATTR_DB_SYSTEM_NAME, "sqlite"],288 [ATTR_DB_SYSTEM_NAME, "sqlite"],