diff --git a/.changeset/db-client-hardening.md b/.changeset/db-client-hardening.md new file mode 100644 index 00000000..958fc358 --- /dev/null +++ b/.changeset/db-client-hardening.md @@ -0,0 +1,6 @@ +--- +"@bunny.net/database-client": patch +"@bunny.net/cli": patch +--- + +Harden `@bunny.net/database-client`: `batch()` takes a `mode`, guards its ROLLBACK, rejects transaction statements, and reports the failing statement as `error.batchIndex`; invalid `timeout` values and malformed responses become `DatabaseError`, transport errors keep their `cause`, integer-valued doubles past 2^53 bind as REAL, and `db.sql` carries a row type. Migrations apply with `BEGIN IMMEDIATE`. diff --git a/bun.lock b/bun.lock index ab3b18af..24c8b62a 100644 --- a/bun.lock +++ b/bun.lock @@ -16,7 +16,7 @@ }, "packages/cli": { "name": "@bunny.net/cli", - "version": "0.15.1", + "version": "0.16.0", "bin": { "bunny": "./bin/bunny.cjs", }, @@ -49,27 +49,27 @@ }, "packages/cli-darwin-arm64": { "name": "@bunny.net/cli-darwin-arm64", - "version": "0.15.1", + "version": "0.16.0", }, "packages/cli-darwin-x64": { "name": "@bunny.net/cli-darwin-x64", - "version": "0.15.1", + "version": "0.16.0", }, "packages/cli-linux-arm64": { "name": "@bunny.net/cli-linux-arm64", - "version": "0.15.1", + "version": "0.16.0", }, "packages/cli-linux-x64": { "name": "@bunny.net/cli-linux-x64", - "version": "0.15.1", + "version": "0.16.0", }, "packages/cli-windows-x64": { "name": "@bunny.net/cli-windows-x64", - "version": "0.15.1", + "version": "0.16.0", }, "packages/config": { "name": "@bunny.net/config", - "version": "0.1.5", + "version": "0.1.6", "dependencies": { "@bunny.net/openapi-client": "workspace:*", "zod": "^4.3.6", @@ -77,7 +77,7 @@ }, "packages/database-adapter": { "name": "@bunny.net/database-adapter", - "version": "0.1.3", + "version": "0.1.4", "dependencies": { "@bunny.net/database-client": "workspace:*", "@bunny.net/database-openapi": "workspace:*", @@ -86,9 +86,9 @@ }, "packages/database-client": { "name": "@bunny.net/database-client", - "version": "0.0.1", + "version": "0.0.2", "devDependencies": { - "typescript": "^5", + "typescript": "^5.7", }, }, "packages/database-client/examples/bun": { @@ -138,7 +138,7 @@ }, "packages/database-shell": { "name": "@bunny.net/database-shell", - "version": "0.2.5", + "version": "0.2.6", "bin": { "bsql": "./bin/bsql.cjs", }, @@ -157,27 +157,27 @@ }, "packages/database-shell-darwin-arm64": { "name": "@bunny.net/database-shell-darwin-arm64", - "version": "0.2.5", + "version": "0.2.6", }, "packages/database-shell-darwin-x64": { "name": "@bunny.net/database-shell-darwin-x64", - "version": "0.2.5", + "version": "0.2.6", }, "packages/database-shell-linux-arm64": { "name": "@bunny.net/database-shell-linux-arm64", - "version": "0.2.5", + "version": "0.2.6", }, "packages/database-shell-linux-x64": { "name": "@bunny.net/database-shell-linux-x64", - "version": "0.2.5", + "version": "0.2.6", }, "packages/database-shell-windows-x64": { "name": "@bunny.net/database-shell-windows-x64", - "version": "0.2.5", + "version": "0.2.6", }, "packages/database-studio": { "name": "@bunny.net/database-studio", - "version": "0.2.1", + "version": "0.2.2", "dependencies": { "@bunny.net/database-adapter": "workspace:*", "@bunny.net/database-client": "workspace:*", diff --git a/packages/cli/src/commands/db/migrations/client.ts b/packages/cli/src/commands/db/migrations/client.ts index 9b88f1cd..0b11fe2d 100644 --- a/packages/cli/src/commands/db/migrations/client.ts +++ b/packages/cli/src/commands/db/migrations/client.ts @@ -23,7 +23,7 @@ export function connectForMigrations(opts: { statements.map(({ sql, args }) => db.prepare(sql).bind(...(args ?? [])), ), - { foreignKeys: false }, + { foreignKeys: false, mode: "immediate" }, ); }, }; diff --git a/packages/database-client/README.md b/packages/database-client/README.md index 11e9d3ce..8a3b4606 100644 --- a/packages/database-client/README.md +++ b/packages/database-client/README.md @@ -34,7 +34,7 @@ const db = connect({ }); ``` -Runnable examples for Edge Scripting, Bun, Node, and Hono live in [`packages/database-client/examples`](https://github.com/BunnyWay/cli/tree/main/packages/database-client/examples). +Runnable examples for Edge Scripting, Bun, Node, Hono, Next.js, Astro, and SvelteKit live in [`packages/database-client/examples`](https://github.com/BunnyWay/cli/tree/main/packages/database-client/examples). ## API @@ -42,14 +42,14 @@ Runnable examples for Edge Scripting, Bun, Node, and Hono live in [`packages/dat Returns a `Database`. Every option is optional. -| Option | Type | Default | Description | -| ----------- | ------------------------ | --------------------------- | ----------------------------------------------------- | -| `url` | `string` | `BUNNY_DATABASE_URL` | `libsql://`, `https://`, or `http://` connection URL. | -| `authToken` | `string` | `BUNNY_DATABASE_AUTH_TOKEN` | Sent as `Authorization: Bearer `. | -| `fetch` | `typeof fetch` | global `fetch` | Override for testing, tracing, or a custom agent. | -| `headers` | `Record` | none | Extra headers on every request. | -| `signal` | `AbortSignal` | none | Applied to every request. | -| `timeout` | `number` | none | Milliseconds before a single request is aborted. | +| Option | Type | Default | Description | +| ----------- | ------------------------ | --------------------------- | ------------------------------------------------------------------------------------- | +| `url` | `string` | `BUNNY_DATABASE_URL` | `libsql://`, `https://`, or `http://` connection URL. | +| `authToken` | `string` | `BUNNY_DATABASE_AUTH_TOKEN` | Sent as `Authorization: Bearer `. | +| `fetch` | `typeof fetch` | global `fetch` | Override for testing, tracing, or a custom agent. | +| `headers` | `Record` | none | Extra headers on every request. | +| `signal` | `AbortSignal` | none | Applied to every request. | +| `timeout` | `number` | none | Milliseconds before a single request is aborted. A positive integer up to 2147483647. | The client rewrites a `libsql://` URL to `https://`. It rejects credentials in the URL, both `user:pass@` and an `authToken` query parameter, so pass `authToken` instead. Dropping a token silently would leave you debugging an unexplained 401. @@ -82,6 +82,8 @@ Each `${...}` becomes a `?` placeholder and its value is bound, never spliced in Values follow the same rules as `bind()`, with one difference: an interpolated object throws instead of being read as named parameters, since inside a template it is far more likely to be a mistake. +Pass a row type the same way as `prepare()`, as ``db.sql`...` ``. + Only values can be parameterized, which is a SQLite limit rather than a client one. Build the statement with `prepare()` when a table or column name has to vary. ### `statement.bind(...values)` @@ -109,7 +111,7 @@ Names may carry the sigil or leave it off, so `{ id: 1 }` and `{ ":id": 1 }` bot Anything else throws instead of being quietly converted, because SQLite has nowhere to put it. `Date` gets its own message suggesting `.toISOString()` or `.getTime()`, since guessing which one you meant would change what ends up in the column. -Integer `number`s past 2^53 also throw: JavaScript has already lost the precision by the time the client sees the value, so storing it would quietly write the wrong number. Pass a `bigint` for values that large. Bigints must fit SQLite's signed 64-bit range. +Integer `number`s are sent as INTEGER while they fit exactly, up to 2^53. Past that every double is a whole number, so the client sends it as REAL, which stores the value as is. Pass a `bigint` when you need an exact integer that large. Bigints must fit SQLite's signed 64-bit range. ### Executing @@ -165,7 +167,11 @@ const [inserted, count] = await db.batch([ ]); ``` -You get one `Result` per statement you passed, in order. `batchRaw()` is the same but with positional rows. If any statement fails the transaction rolls back and `batch()` throws that statement's error. +You get one `Result` per statement you passed, in order. `batchRaw()` is the same but with positional rows. If any statement fails the transaction rolls back and `batch()` throws that statement's error, with `error.batchIndex` set to the position of the statement that failed. + +The batch is the transaction, so a statement of your own that starts with `BEGIN`, `COMMIT`, `END`, or `ROLLBACK` is rejected before anything is sent. Savepoints are fine. + +`{ mode: "immediate" }` opens the transaction with `BEGIN IMMEDIATE`, which takes the write lock up front. SQLite's default, `deferred`, takes it at the first write instead, so a batch that reads and then writes can fail with `SQLITE_BUSY` if another writer got in between. Use `immediate` for batches you know will write. `exclusive` is also accepted. `batch()` infers each result's row type from its statement, so a `prepare(...)` statement comes back as `Result` even next to untyped ones. See [Types](#types). @@ -174,15 +180,17 @@ You get one `Result` per statement you passed, in order. `batchRaw()` is the sam ```ts await db.batch( [ - db.prepare("ALTER TABLE users RENAME TO users_old"), - db.prepare("CREATE TABLE users (id INTEGER PRIMARY KEY, email TEXT NOT NULL)"), - db.prepare("INSERT INTO users SELECT id, email FROM users_old"), - db.prepare("DROP TABLE users_old"), + db.prepare("CREATE TABLE users_new (id INTEGER PRIMARY KEY, email TEXT NOT NULL)"), + db.prepare("INSERT INTO users_new SELECT id, email FROM users"), + db.prepare("DROP TABLE users"), + db.prepare("ALTER TABLE users_new RENAME TO users"), ], - { foreignKeys: false }, + { foreignKeys: false, mode: "immediate" }, ); ``` +Keep that order: build the replacement under a temporary name, drop the original, then rename. Renaming the original out of the way first looks equivalent but is not, because with foreign keys off SQLite rewrites every `REFERENCES users` in other tables to follow the rename, and they end up pointing at a table you are about to drop. + ### `db.exec(sql)` Runs a multi-statement script. It takes no parameters and returns no rows, so it is mostly for setting up a schema. @@ -212,19 +220,23 @@ try { } ``` -| Property | Description | -| --------- | ------------------------------------------------------------------------------------ | -| `message` | The server's message, or a description of the local validation failure. | -| `code` | SQLite code (`SQLITE_CONSTRAINT`), or a client code (`UNAUTHORIZED`, `URL_MISSING`). | -| `status` | HTTP status, when the failure came from the transport rather than from SQL. | +| Property | Description | +| ------------ | ------------------------------------------------------------------------------------ | +| `message` | The server's message, or a description of the local validation failure. | +| `code` | SQLite code (`SQLITE_CONSTRAINT`), or a client code (`UNAUTHORIZED`, `URL_MISSING`). | +| `status` | HTTP status, when the failure came from the transport rather than from SQL. | +| `batchIndex` | Position of the failing statement, when one of your statements in `batch()` failed. | Failures that happen before any SQL runs are wrapped too, so a single `catch (error) { if (error instanceof DatabaseError) ... }` covers the whole surface rather than letting a `TypeError` through: -| `code` | Cause | -| --------- | ------------------------------------------------------------ | -| `NETWORK` | DNS, TLS, connection refused, or a response that isn't JSON. | -| `TIMEOUT` | The `timeout` deadline elapsed. | -| `ABORTED` | The `signal` you passed was aborted. | +| `code` | Cause | +| ---------- | -------------------------------------------------------------------- | +| `NETWORK` | DNS, TLS, connection refused, or a response that isn't JSON. | +| `TIMEOUT` | The `timeout` deadline elapsed. | +| `ABORTED` | The `signal` you passed was aborted. | +| `PROTOCOL` | The server answered 200 with a body that is not a pipeline response. | + +For these three, `error.cause` holds the runtime's original error. ## Types diff --git a/packages/database-client/package.json b/packages/database-client/package.json index 67892c6a..48624522 100644 --- a/packages/database-client/package.json +++ b/packages/database-client/package.json @@ -38,7 +38,8 @@ "exports": { ".": { "types": "./dist/index.d.ts", - "import": "./dist/index.js" + "import": "./dist/index.js", + "default": "./dist/index.js" } }, "files": [ @@ -47,7 +48,7 @@ "LICENSE" ], "devDependencies": { - "typescript": "^5" + "typescript": "^5.7" }, "publishConfig": { "access": "public" diff --git a/packages/database-client/scripts/smoke.ts b/packages/database-client/scripts/smoke.ts index 127c97b0..fe47be02 100644 --- a/packages/database-client/scripts/smoke.ts +++ b/packages/database-client/scripts/smoke.ts @@ -60,6 +60,17 @@ check( big, ); +// 5b. a double past 2^53 is bound as REAL and comes back unchanged +const real = await db + .prepare("SELECT ? AS r, typeof(?) AS t") + .bind(1e21, 1e21) + .first(); +check( + "large double binds as REAL", + real?.r === 1e21 && real?.t === "real", + real, +); + // 6. blob round trip const blob = await db .prepare("SELECT ? AS b") @@ -117,6 +128,24 @@ try { }); } +// 9b. the failing statement's position and the server's row counts come back +try { + await db.batch( + [ + db.prepare("CREATE TABLE __probe5 (id INTEGER PRIMARY KEY)"), + db.prepare("INSERT INTO __probe5 (id) VALUES (1)"), + db.prepare("INSERT INTO __probe5 (id) VALUES (1)"), + ], + { mode: "immediate" }, + ); + check("batchIndex", false); +} catch (error) { + const e = error as DatabaseError; + check("batch error names the failing statement", e.batchIndex === 2, { + batchIndex: e.batchIndex, + }); +} + // 10. exec runs a multi-statement script await db.exec( "CREATE TABLE __probe3 (id INTEGER); INSERT INTO __probe3 VALUES (1),(2); DROP TABLE __probe3;", diff --git a/packages/database-client/src/client.test.ts b/packages/database-client/src/client.test.ts index 2c778178..07cf5183 100644 --- a/packages/database-client/src/client.test.ts +++ b/packages/database-client/src/client.test.ts @@ -469,6 +469,19 @@ describe("sql template", () => { expect(() => db.sql`SELECT ${{ a: 1 }}`).toThrow(/cannot bind value/); }); + test("carries a row type like prepare does", async () => { + interface Note { + id: number; + } + const fake = fakeFetch([okExecute(["id"], [[1]])]); + const db = connect({ url: URL_, fetch: fake.fetch }); + + const note = await db.sql`SELECT id FROM notes`.first(); + const id: number | undefined = note?.id; + + expect(id).toBe(1); + }); + test("executes like any other statement", async () => { const fake = fakeFetch([okExecute(["id"], [[7]])]); const db = connect({ url: URL_, fetch: fake.fetch }); @@ -513,7 +526,7 @@ describe("batch", () => { const steps = (fake.captures[0] as Capture).body.requests[0]?.batch?.steps ?? []; expect(steps.map((s) => s.stmt.sql)).toEqual([ - "BEGIN", + "BEGIN DEFERRED", "INSERT INTO t VALUES (1)", "INSERT INTO t VALUES (2)", "COMMIT", @@ -521,10 +534,46 @@ describe("batch", () => { ]); expect(steps[1]?.condition).toEqual({ type: "ok", step: 0 }); expect(steps[3]?.condition).toEqual({ type: "ok", step: 2 }); + // Guarded on BEGIN so a failed BEGIN never rolls back a transaction the batch did not open. expect(steps[4]?.condition).toEqual({ - type: "not", - cond: { type: "ok", step: 3 }, + type: "and", + conds: [ + { type: "ok", step: 0 }, + { type: "not", cond: { type: "ok", step: 3 } }, + ], + }); + }); + + test("mode picks the BEGIN variant", async () => { + const fake = fakeFetch([okBatch(1)]); + const db = connect({ url: URL_, fetch: fake.fetch }); + + await db.batch([db.prepare("INSERT INTO t VALUES (1)")], { + mode: "immediate", }); + + const steps = + (fake.captures[0] as Capture).body.requests[0]?.batch?.steps ?? []; + expect(steps[0]?.stmt.sql).toBe("BEGIN IMMEDIATE"); + }); + + test("rejects a caller statement that would break the transaction", async () => { + const fake = fakeFetch([]); + const db = connect({ url: URL_, fetch: fake.fetch }); + + for (const sql of [ + "COMMIT", + " begin immediate", + "-- note\nROLLBACK", + "/* c */ END", + ]) { + const error = (await db + .batch([db.prepare("SELECT 1"), db.prepare(sql)]) + .catch((e) => e)) as DatabaseError; + expect(error.code).toBe("ARGUMENT_INVALID"); + expect(error.batchIndex).toBe(1); + } + expect(fake.captures).toHaveLength(0); }); test("returns one result per caller statement, not per wire step", async () => { @@ -557,7 +606,7 @@ describe("batch", () => { (fake.captures[0] as Capture).body.requests[0]?.batch?.steps ?? []; expect(steps.map((s) => s.stmt.sql)).toEqual([ "PRAGMA foreign_keys=off", - "BEGIN", + "BEGIN DEFERRED", "ALTER TABLE parent RENAME TO parent_old", "DROP TABLE parent_old", "COMMIT", @@ -568,8 +617,11 @@ describe("batch", () => { expect(steps[3]?.condition).toEqual({ type: "ok", step: 2 }); expect(steps[4]?.condition).toEqual({ type: "ok", step: 3 }); expect(steps[5]?.condition).toEqual({ - type: "not", - cond: { type: "ok", step: 4 }, + type: "and", + conds: [ + { type: "ok", step: 1 }, + { type: "not", cond: { type: "ok", step: 4 } }, + ], }); }); @@ -652,6 +704,35 @@ describe("batch", () => { expect(error).toBeInstanceOf(DatabaseError); expect(error.code).toBe("SQLITE_CONSTRAINT"); + expect(error.batchIndex).toBe(0); + }); + + test("a failed BEGIN or COMMIT carries no statement index", async () => { + const fake = fakeFetch([ + { + type: "ok", + response: { + type: "batch", + result: { + step_results: [null, null, null, null], + step_errors: [ + { message: "database is locked", code: "SQLITE_BUSY" }, + null, + null, + null, + ], + }, + }, + }, + ]); + const db = connect({ url: URL_, fetch: fake.fetch }); + + const error = (await db + .batch([db.prepare("INSERT INTO t VALUES (1)")]) + .catch((e) => e)) as DatabaseError; + + expect(error.code).toBe("SQLITE_BUSY"); + expect(error.batchIndex).toBeUndefined(); }); }); @@ -740,6 +821,70 @@ describe("errors", () => { expect(error.message).toContain(ENV_DATABASE_AUTH_TOKEN); }); + test("a 200 without a results array is a protocol error, not a TypeError", async () => { + const error = await failure(responds(JSON.stringify({ ok: true }))); + + expect(error).toBeInstanceOf(DatabaseError); + expect(error.code).toBe("PROTOCOL"); + }); + + test("the positional constructor still works for callers of 0.0.x", () => { + const error = new DatabaseError("boom", "SQLITE_BUSY", 503); + + expect(error.code).toBe("SQLITE_BUSY"); + expect(error.status).toBe(503); + expect(error.cause).toBeUndefined(); + + const statusOnly = new DatabaseError("boom", undefined, 502); + expect(statusOnly.code).toBeUndefined(); + expect(statusOnly.status).toBe(502); + }); + + test("a comment-heavy statement is scanned without blowing up", async () => { + const fake = fakeFetch([]); + const db = connect({ url: URL_, fetch: fake.fetch }); + const sql = `${" /*".repeat(5000)} COMMIT`; + + const error = (await db + .batch([db.prepare(sql)]) + .catch((e) => e)) as DatabaseError; + + // Unterminated comment: no keyword found, so it is sent as-is rather than rejected. + expect(error.code).not.toBe("ARGUMENT_INVALID"); + }); + + test("transport errors keep the underlying error as cause", async () => { + const thrown = new TypeError("fetch failed"); + const error = await failure((async () => { + throw thrown; + }) as unknown as typeof fetch); + + expect(error.cause).toBe(thrown); + }); + + test("an error body that is JSON but not an object falls back to the status", async () => { + const error = await failure(responds("null", { status: 500 })); + + expect(error.message).toBe("database request failed with HTTP 500"); + expect(error.status).toBe(500); + }); + + test("a timeout that is not a positive number is rejected up front", () => { + // Node throws on fractions and silently clamps anything past 2^31-1 to 1ms. + for (const timeout of [ + 0, + -1, + 1.5, + 2 ** 31, + Number.NaN, + Number.POSITIVE_INFINITY, + ]) { + expect(() => connect({ url: URL_, timeout })).toThrow( + /timeout must be a positive integer/, + ); + } + }); + test("a non-JSON error body still yields a usable message", async () => { const error = await failure(responds("upstream is down", { status: 502 })); @@ -807,6 +952,16 @@ describe("connect", () => { ); }); + test("an empty url falls back to the environment like a missing one", async () => { + const fake = fakeFetch([okExecute(["a"], [])]); + await withEnv({ [ENV_DATABASE_URL]: URL_ }, async () => { + await connect({ url: "", fetch: fake.fetch }).exec("SELECT 1"); + expect((fake.captures[0] as Capture).url).toBe( + "https://db.lite.bunnydb.net/v2/pipeline", + ); + }); + }); + test("names the environment variable when there is no url at all", async () => { await withEnv({ [ENV_DATABASE_URL]: undefined }, () => { expect(() => connect()).toThrow(ENV_DATABASE_URL); diff --git a/packages/database-client/src/client.ts b/packages/database-client/src/client.ts index 8c37087b..deb4dd04 100644 --- a/packages/database-client/src/client.ts +++ b/packages/database-client/src/client.ts @@ -2,6 +2,7 @@ import { ENV_DATABASE_AUTH_TOKEN, ENV_DATABASE_URL, readEnv } from "./env.ts"; import { DatabaseError } from "./errors.ts"; import { createTransport, + decodeInteger, decodeValue, encodeValue, type SqlValue, @@ -26,19 +27,46 @@ export interface Result { } /** Same as `Result`, with rows as positional arrays so duplicate column names survive. */ -export interface RawResult extends Omit, "rows"> { - rows: SqlValue[][]; -} +export type RawResult = Result; export interface Config extends TransportConfig { - /** Abort signal applied to every request unless a per-call signal is given. */ + /** Abort signal applied to every request this connection makes. */ signal?: AbortSignal; } +/** How the batch transaction takes its lock. `immediate` reserves the write lock up front so a batch that reads then writes cannot lose to another writer. */ +export type BatchMode = "deferred" | "immediate" | "exclusive"; + /** Options for `batch()` and `batchRaw()`. */ export interface BatchOptions { /** Enforce foreign key constraints. Pass `false` for schema changes that rebuild tables. */ foreignKeys?: boolean; + /** Transaction locking mode. Defaults to `deferred`, SQLite's own default. */ + mode?: BatchMode; +} + +const TRANSACTION_KEYWORDS = new Set(["BEGIN", "COMMIT", "END", "ROLLBACK"]); + +/** First keyword of a statement, skipping leading whitespace, semicolons, and comments. A plain scan, since a regex here is quadratic on nested comment openers. */ +function firstKeyword(sql: string): string | undefined { + let i = 0; + while (i < sql.length) { + const ch = sql[i] as string; + if (ch === ";" || /\s/.test(ch)) { + i++; + } else if (sql.startsWith("--", i)) { + const end = sql.indexOf("\n", i); + if (end === -1) return undefined; + i = end + 1; + } else if (sql.startsWith("/*", i)) { + const end = sql.indexOf("*/", i + 2); + if (end === -1) return undefined; + i = end + 2; + } else { + break; + } + } + return /^[A-Za-z]+/.exec(sql.slice(i))?.[0].toUpperCase(); } interface StatementInternals { @@ -64,28 +92,32 @@ function toRawResult(wire: WireStmtResult): RawResult { lastInsertRowid: wire.last_insert_rowid === null ? null - : (decodeValue({ type: "integer", value: wire.last_insert_rowid }) as - | number - | bigint), + : decodeInteger(wire.last_insert_rowid), }; } +/** Zip one positional row with its column names. Null prototype so a column named __proto__ (or constructor, toString, ...) is a plain own property. */ +function toRow(columns: string[], values: SqlValue[]): Row { + const row: Row = Object.create(null); + columns.forEach((name, i) => { + row[name] = values[i] as SqlValue; + }); + return row; +} + function toResult(wire: WireStmtResult): Result { const raw = toRawResult(wire); - const rows = raw.rows.map((row) => { - // Null prototype so a column named __proto__ (or constructor, toString, ...) is a plain own property. - const out: Row = Object.create(null); - for (let i = 0; i < raw.columns.length; i++) - out[raw.columns[i] as string] = row[i] as SqlValue; - return out as T; - }); - return { ...raw, rows } as Result; + return { + ...raw, + rows: raw.rows.map((values) => toRow(raw.columns, values) as T), + }; } /** A SQL statement plus its bound arguments. Immutable and reusable. `T` is the row shape its executors return. */ export class Statement { readonly #internals: StatementInternals; + /** Not for direct use: statements come from `Database.prepare()` and `Database.sql`. */ constructor(internals: StatementInternals) { this.#internals = internals; } @@ -96,24 +128,19 @@ export class Statement { if (named && values.length > 1) { throw new DatabaseError( "cannot mix positional and named parameters; pass a list of values or a single object", - "ARGUMENT_INVALID", + { code: "ARGUMENT_INVALID" }, ); } - if (named) { - return new Statement({ - ...this.#internals, - args: [], - // The server resolves a bare name against :name, @name, and $name, so the sigil is optional. - namedArgs: Object.entries(named).map(([name, value]) => ({ - name, - value: encodeValue(value), - })), - }); - } return new Statement({ ...this.#internals, - args: values.map(encodeValue), - namedArgs: [], + args: named ? [] : values.map(encodeValue), + // The server resolves a bare name against :name, @name, and $name, so the sigil is optional. + namedArgs: named + ? Object.entries(named).map(([name, value]) => ({ + name, + value: encodeValue(value), + })) + : [], }); } @@ -133,7 +160,7 @@ export class Statement { if (!Object.hasOwn(row, column)) { throw new DatabaseError( `column "${column}" is not in the result; got ${result.columns.join(", ")}`, - "COLUMN_NOT_FOUND", + { code: "COLUMN_NOT_FOUND" }, ); } return row[column] as SqlValue; @@ -185,32 +212,34 @@ export class Database { readonly #signal?: AbortSignal; constructor(config: Config) { - if (!config.url) throw new DatabaseError("url is required", "URL_INVALID"); + if (!config.url) + throw new DatabaseError("url is required", { code: "URL_INVALID" }); this.#transport = createTransport(config); this.#signal = config.signal; } - /** Create a statement from SQL. Bind arguments with `.bind()`. Pass `T` to type every row it returns. */ - prepare(sql: string): Statement { + #statement(sql: string, args: WireValue[]): Statement { return new Statement({ sql, - args: [], + args, namedArgs: [], transport: this.#transport, signal: this.#signal, }); } - /** Build a statement from a template literal, binding every interpolated value positionally. */ - sql(strings: TemplateStringsArray, ...values: unknown[]): Statement { + /** Create a statement from SQL. Bind arguments with `.bind()`. Pass `T` to type every row it returns. */ + prepare(sql: string): Statement { + return this.#statement(sql, []); + } + + /** Build a statement from a template literal, binding every interpolated value positionally. Pass `T` to type its rows. */ + sql( + strings: TemplateStringsArray, + ...values: unknown[] + ): Statement { // Binding here rather than through bind() keeps an interpolated object a rejected value instead of named parameters. - return new Statement({ - sql: strings.join("?"), - args: values.map(encodeValue), - namedArgs: [], - transport: this.#transport, - signal: this.#signal, - }); + return this.#statement(strings.join("?"), values.map(encodeValue)); } /** Run every statement in one transaction. All succeed or none are applied. */ @@ -237,32 +266,46 @@ export class Database { ): Promise { if (statements.length === 0) return []; + // A caller's own BEGIN or COMMIT would break the transaction the batch wraps around it. + statements.forEach((statement, index) => { + const keyword = firstKeyword(statement.wire.sql); + if (keyword && TRANSACTION_KEYWORDS.has(keyword)) { + throw new DatabaseError( + `statement ${index} starts with ${keyword}; batch() already runs its statements in one transaction`, + { code: "ARGUMENT_INVALID", batchIndex: index }, + ); + } + }); + const control = (sql: string, condition?: unknown) => ({ stmt: { sql, args: [], named_args: [], want_rows: false }, ...(condition ? { condition } : {}), }); + const ok = (step: number) => ({ type: "ok", step }); // SQLite ignores `PRAGMA foreign_keys` inside a transaction, so the pragmas // bracket BEGIN/COMMIT instead of sitting within them. const unchecked = options.foreignKeys === false; const prelude = unchecked ? [control("PRAGMA foreign_keys=off")] : []; const begin = prelude.length; + const first = begin + 1; const last = begin + statements.length; + const commit = last + 1; - // BEGIN is already DEFERRED in SQLite, so naming the mode would change nothing. const steps = [ ...prelude, - control("BEGIN"), + control(`BEGIN ${(options.mode ?? "deferred").toUpperCase()}`), ...statements.map((statement, index) => ({ stmt: statement.wire, - condition: { type: "ok", step: begin + index }, + condition: ok(begin + index), })), - control("COMMIT", { type: "ok", step: last }), + control("COMMIT", ok(last)), + // Only roll back a transaction this batch opened, and only when it did not commit. control("ROLLBACK", { - type: "not", - cond: { type: "ok", step: last + 1 }, + type: "and", + conds: [ok(begin), { type: "not", cond: ok(commit) }], }), - // Matches @libsql/client, which hardcodes `on` too; harmless here because the stream closes in this same request. + // Hardcoded `on` is harmless here because the stream closes in this same request. ...(unchecked ? [control("PRAGMA foreign_keys=on")] : []), ]; @@ -272,11 +315,18 @@ export class Database { ); const batch = unwrap(results[0]); - const failure = batch.step_errors.find((error) => error !== null); - if (failure) throw DatabaseError.fromWire(failure); + // Steps fail in order (the chain stops at the first error), so the first error is the cause. + const failed = batch.step_errors.findIndex((error) => error != null); + if (failed !== -1) { + const index = failed - first; + throw DatabaseError.fromWire( + batch.step_errors[failed] ?? undefined, + index >= 0 && index < statements.length ? index : undefined, + ); + } return statements.map((_, index) => { - const step = batch.step_results[begin + 1 + index]; + const step = batch.step_results[first + index]; if (!step) throw new DatabaseError("batch step returned no result"); return step; }); @@ -300,11 +350,11 @@ export class Database { * wherever the CLI or Edge Scripting has already put them in the environment. */ export function connect(config: Partial = {}): Database { - const url = config.url ?? readEnv(ENV_DATABASE_URL); + const url = config.url || readEnv(ENV_DATABASE_URL); if (!url) { throw new DatabaseError( `no database URL: pass { url } or set ${ENV_DATABASE_URL}`, - "URL_MISSING", + { code: "URL_MISSING" }, ); } return new Database({ diff --git a/packages/database-client/src/errors.ts b/packages/database-client/src/errors.ts index bb89cf71..48df7fb7 100644 --- a/packages/database-client/src/errors.ts +++ b/packages/database-client/src/errors.ts @@ -1,64 +1,100 @@ import { ENV_DATABASE_AUTH_TOKEN } from "./env.ts"; +export interface DatabaseErrorOptions { + code?: string; + status?: number; + batchIndex?: number; + cause?: unknown; +} + +/** Fetch rejections that mean the request never completed, keyed by the error name each runtime uses. */ +const TRANSPORT_FAILURES: Record = { + TimeoutError: { message: "database request timed out", code: "TIMEOUT" }, + AbortError: { message: "database request was aborted", code: "ABORTED" }, +}; + export class DatabaseError extends Error { override readonly name = "DatabaseError"; - /** SQLite/hrana error code (e.g. `SQLITE_CONSTRAINT_UNIQUE`). */ + /** SQLite/hrana error code (e.g. `SQLITE_CONSTRAINT`), or a client code such as `TIMEOUT`. */ readonly code?: string; /** HTTP status when the failure came from the transport rather than SQL. */ readonly status?: number; - constructor(message: string, code?: string, status?: number) { - super(message); - this.code = code; - this.status = status; + /** Zero-based position of the statement that failed inside `batch()`, when one of the caller's statements did. */ + readonly batchIndex?: number; + + /** The runtime's original error for `NETWORK`, `TIMEOUT`, and `ABORTED` failures. Declared here so it is visible without the ES2022 lib. */ + declare readonly cause?: unknown; + + constructor(message: string, options?: DatabaseErrorOptions); + /** @deprecated Pass `{ code, status }` instead. Kept so callers of 0.0.x keep compiling. */ + constructor(message: string, code?: string, status?: number); + constructor( + message: string, + codeOrOptions?: string | DatabaseErrorOptions, + status?: number, + ) { + const options = + typeof codeOrOptions === "object" + ? codeOrOptions + : { code: codeOrOptions, status }; + super( + message, + options.cause === undefined ? undefined : { cause: options.cause }, + ); + this.code = options.code; + this.status = options.status; + this.batchIndex = options.batchIndex; } static fromWire( error: { message: string; code?: string | null } | undefined, + batchIndex?: number, ): DatabaseError { - return new DatabaseError( - error?.message ?? "unknown database error", - error?.code ?? undefined, - ); + return new DatabaseError(error?.message ?? "unknown database error", { + code: error?.code ?? undefined, + batchIndex, + }); } /** Classify a failure that happened before any SQL ran: an aborted, timed out, or unreachable request. */ - static fromTransport(error: unknown): DatabaseError { - if (error instanceof DatabaseError) return error; - const { name, message } = (error ?? {}) as { + static fromTransport(cause: unknown): DatabaseError { + if (cause instanceof DatabaseError) return cause; + const { name, message } = (cause ?? {}) as { name?: string; message?: string; }; - if (name === "TimeoutError") { - return new DatabaseError("database request timed out", "TIMEOUT"); - } - if (name === "AbortError") { - return new DatabaseError("database request was aborted", "ABORTED"); - } + const known = name === undefined ? undefined : TRANSPORT_FAILURES[name]; + if (known) + return new DatabaseError(known.message, { code: known.code, cause }); return new DatabaseError( `could not reach the database${message ? `: ${message}` : ""}`, - "NETWORK", + { code: "NETWORK", cause }, ); } static fromHttp(status: number, body: string): DatabaseError { let message = `database request failed with HTTP ${status}`; try { - const parsed = JSON.parse(body) as { error?: string; message?: string }; - if (parsed.error || parsed.message) - message = String(parsed.error ?? parsed.message); + const parsed = JSON.parse(body) as unknown; + if (parsed && typeof parsed === "object") { + const { error, message: text } = parsed as { + error?: unknown; + message?: unknown; + }; + if (error || text) message = String(error ?? text); + } } catch { if (body.trim()) message = body.trim().slice(0, 300); } if (status === 401 || status === 403) { return new DatabaseError( `${message} (check the auth token, or set ${ENV_DATABASE_AUTH_TOKEN})`, - "UNAUTHORIZED", - status, + { code: "UNAUTHORIZED", status }, ); } - return new DatabaseError(message, undefined, status); + return new DatabaseError(message, { status }); } } diff --git a/packages/database-client/src/index.ts b/packages/database-client/src/index.ts index b3903014..910bfa0c 100644 --- a/packages/database-client/src/index.ts +++ b/packages/database-client/src/index.ts @@ -1,4 +1,5 @@ export { + type BatchMode, type BatchOptions, type BatchResults, type Config, @@ -10,5 +11,5 @@ export { Statement, } from "./client.ts"; export { ENV_DATABASE_AUTH_TOKEN, ENV_DATABASE_URL } from "./env.ts"; -export { DatabaseError } from "./errors.ts"; +export { DatabaseError, type DatabaseErrorOptions } from "./errors.ts"; export type { SqlValue } from "./protocol.ts"; diff --git a/packages/database-client/src/protocol.test.ts b/packages/database-client/src/protocol.test.ts index 1c1bafa3..512ba6f3 100644 --- a/packages/database-client/src/protocol.test.ts +++ b/packages/database-client/src/protocol.test.ts @@ -98,13 +98,12 @@ describe("encodeValue", () => { expect(() => encodeValue(undefined)).toThrow(/cannot bind undefined/); }); - test("rejects integer numbers past 2^53 instead of silently rounding them", () => { - expect(() => encodeValue(Number.MAX_SAFE_INTEGER + 2)).toThrow( - /pass a bigint/, - ); - expect(() => encodeValue(Number.MIN_SAFE_INTEGER - 2)).toThrow( - /pass a bigint/, - ); + test("sends integer-valued doubles past 2^53 as floats rather than rejecting them", () => { + expect(encodeValue(1e21)).toEqual({ type: "float", value: 1e21 }); + expect(encodeValue(Number.MIN_SAFE_INTEGER - 2)).toEqual({ + type: "float", + value: Number.MIN_SAFE_INTEGER - 2, + }); expect(encodeValue(Number.MAX_SAFE_INTEGER)).toEqual({ type: "integer", value: "9007199254740991", diff --git a/packages/database-client/src/protocol.ts b/packages/database-client/src/protocol.ts index 9eeadaa7..7c17d98f 100644 --- a/packages/database-client/src/protocol.ts +++ b/packages/database-client/src/protocol.ts @@ -84,43 +84,36 @@ function base64ToBytes(base64: string): Uint8Array { return bytes; } +const invalid = (message: string) => + new DatabaseError(message, { code: "ARGUMENT_INVALID" }); + +// Node's timers take a signed 32-bit integer; a larger value silently fires after 1ms and a fraction throws. +const MAX_TIMEOUT = 2_147_483_647; + export function encodeValue(value: unknown): WireValue { if (value === null) return { type: "null" }; if (value === undefined) { - throw new DatabaseError( - "cannot bind undefined; pass null to store SQL NULL", - "ARGUMENT_INVALID", - ); + throw invalid("cannot bind undefined; pass null to store SQL NULL"); } if (typeof value === "boolean") { return { type: "integer", value: value ? "1" : "0" }; } if (typeof value === "bigint") { if (value > INT64_MAX || value < INT64_MIN) { - throw new DatabaseError( + throw invalid( `cannot bind bigint ${value}; outside SQLite's 64-bit integer range`, - "ARGUMENT_INVALID", ); } return { type: "integer", value: value.toString() }; } if (typeof value === "number") { if (!Number.isFinite(value)) { - throw new DatabaseError( - `cannot bind non-finite number: ${value}`, - "ARGUMENT_INVALID", - ); + throw invalid(`cannot bind non-finite number: ${value}`); } - if (Number.isInteger(value)) { - if (!Number.isSafeInteger(value)) { - throw new DatabaseError( - `cannot bind unsafe integer ${value}; numbers past 2^53 have already lost precision, pass a bigint instead`, - "ARGUMENT_INVALID", - ); - } - return { type: "integer", value: value.toString() }; - } - return { type: "float", value }; + // Past 2^53 every double is integral, so send it as the REAL it is rather than guessing at a lost integer. + return Number.isSafeInteger(value) + ? { type: "integer", value: value.toString() } + : { type: "float", value }; } if (typeof value === "string") return { type: "text", value }; if (value instanceof Uint8Array) @@ -129,17 +122,21 @@ export function encodeValue(value: unknown): WireValue { return { type: "blob", base64: bytesToBase64(new Uint8Array(value)) }; } if (value instanceof Date) { - throw new DatabaseError( + throw invalid( "cannot bind a Date; pass date.toISOString() or date.getTime() instead", - "ARGUMENT_INVALID", ); } - throw new DatabaseError( + throw invalid( `cannot bind value of type ${typeof value}; expected null, boolean, number, bigint, string, or Uint8Array`, - "ARGUMENT_INVALID", ); } +/** Widen to bigint only where a number would lose precision. */ +export function decodeInteger(value: string): number | bigint { + const big = BigInt(value); + return big > MAX_SAFE || big < MIN_SAFE ? big : Number(big); +} + export function decodeValue(value: WireValue): SqlValue { switch (value.type) { case "null": @@ -150,10 +147,8 @@ export function decodeValue(value: WireValue): SqlValue { return value.value; case "blob": return base64ToBytes(value.base64); - case "integer": { - const big = BigInt(value.value); - return big > MAX_SAFE || big < MIN_SAFE ? big : Number(big); - } + case "integer": + return decodeInteger(value.value); default: throw new DatabaseError( `unsupported value type from server: ${(value as { type: string }).type}`, @@ -169,12 +164,14 @@ const SCHEME_MAP: Record = { http: "http", }; +const invalidUrl = (message: string) => + new DatabaseError(message, { code: "URL_INVALID" }); + export function normalizeUrl(url: string): string { const match = /^([a-zA-Z][a-zA-Z0-9+.-]*):\/\//.exec(url); if (!match) { - throw new DatabaseError( + throw invalidUrl( `invalid database URL "${url}"; expected a libsql:// or https:// URL`, - "URL_INVALID", ); } const scheme = (match[1] as string).toLowerCase(); @@ -182,7 +179,7 @@ export function normalizeUrl(url: string): string { if (!mapped) { throw new DatabaseError( `unsupported URL scheme "${scheme}:"; expected libsql:, https:, or http:`, - "URL_SCHEME_NOT_SUPPORTED", + { code: "URL_SCHEME_NOT_SUPPORTED" }, ); } @@ -190,18 +187,16 @@ export function normalizeUrl(url: string): string { try { parsed = new URL(`${mapped}://${url.slice(match[0].length)}`); } catch { - throw new DatabaseError(`invalid database URL "${url}"`, "URL_INVALID"); + throw invalidUrl(`invalid database URL "${url}"`); } if (parsed.username || parsed.password) { - throw new DatabaseError( + throw invalidUrl( "database URL must not contain credentials; pass authToken instead", - "URL_INVALID", ); } if (parsed.searchParams.has("authToken")) { - throw new DatabaseError( + throw invalidUrl( "database URL must not carry an authToken query parameter; pass authToken instead", - "URL_INVALID", ); } if (scheme === "libsql" && parsed.searchParams.get("tls") === "0") { @@ -242,18 +237,29 @@ function requestSignal( /** Build a stateless transport: every call is one self-contained POST to /v2/pipeline. */ export function createTransport(config: TransportConfig): Transport { + const { timeout } = config; + if ( + timeout !== undefined && + !(Number.isInteger(timeout) && timeout > 0 && timeout <= MAX_TIMEOUT) + ) { + throw invalid( + `timeout must be a positive integer of milliseconds no larger than ${MAX_TIMEOUT}, got ${timeout}`, + ); + } // v2 is the widest-supported pipeline path and every request we send is v2-capable. const endpoint = `${normalizeUrl(config.url)}/v2/pipeline`; const doFetch = config.fetch ?? fetch; + // Lowercased so a caller's User-Agent replaces the default instead of sending both. const headers: Record = { "content-type": "application/json", "user-agent": USER_AGENT, + ...Object.fromEntries( + Object.entries(config.headers ?? {}).map(([name, value]) => [ + name.toLowerCase(), + value, + ]), + ), }; - - for (const [name, value] of Object.entries(config.headers ?? {})) { - headers[name.toLowerCase()] = value; - } - if (config.authToken) headers.authorization = `Bearer ${config.authToken}`; return { @@ -267,7 +273,7 @@ export function createTransport(config: TransportConfig): Transport { baton: null, requests: [...requests, { type: "close" }], }), - signal: requestSignal(signal, config.timeout), + signal: requestSignal(signal, timeout), }); } catch (error) { throw DatabaseError.fromTransport(error); @@ -278,12 +284,18 @@ export function createTransport(config: TransportConfig): Transport { throw DatabaseError.fromHttp(response.status, body); } - let payload: WireResponse; + let payload: Partial | null; try { - payload = (await response.json()) as WireResponse; + payload = (await response.json()) as Partial | null; } catch (error) { throw DatabaseError.fromTransport(error); } + if (!Array.isArray(payload?.results)) { + throw new DatabaseError( + "database returned a response without results", + { code: "PROTOCOL" }, + ); + } return payload.results; }, }; diff --git a/packages/database-client/tsconfig.build.json b/packages/database-client/tsconfig.build.json index 5ccb64e1..aeae4827 100644 --- a/packages/database-client/tsconfig.build.json +++ b/packages/database-client/tsconfig.build.json @@ -3,6 +3,7 @@ "compilerOptions": { "noEmit": false, "declaration": true, + "stripInternal": true, "rewriteRelativeImportExtensions": true, "outDir": "dist", "rootDir": "src", diff --git a/skills/bunny-cli/references/database-client.md b/skills/bunny-cli/references/database-client.md index 9ab0b928..403145c1 100644 --- a/skills/bunny-cli/references/database-client.md +++ b/skills/bunny-cli/references/database-client.md @@ -85,7 +85,8 @@ These throw rather than converting quietly: - `undefined` — so a mistyped property (`bind(user.nmae)`) surfaces instead of writing NULL. Pass `null` for NULL. - `Date` — the error suggests `.toISOString()` or `.getTime()`, since guessing would change what lands in the column. -- Integer `number`s past 2^53 — JavaScript already lost the precision. Pass a `bigint` (within SQLite's signed 64-bit range). + +Integer `number`s are sent as INTEGER up to 2^53 and as REAL past it, where every double is already a whole number. Pass a `bigint` for an exact integer that large (within SQLite's signed 64-bit range). ### Executing @@ -121,7 +122,7 @@ const [inserted, count] = await db.batch([ ]); ``` -If any statement fails, the transaction rolls back and `batch()` throws that statement's error. `batchRaw()` is the same with positional rows. +If any statement fails, the transaction rolls back and `batch()` throws that statement's error with `error.batchIndex` set. `batchRaw()` is the same with positional rows. Statements starting with `BEGIN`, `COMMIT`, `END`, or `ROLLBACK` are rejected: the batch is the transaction. Pass `{ mode: "immediate" }` for write batches so the lock is taken up front instead of at the first write. `{ foreignKeys: false }` brackets the transaction with `PRAGMA foreign_keys=off` and `=on`. Schema changes need it: SQLite's table rebuild procedure and several `ALTER TABLE` forms require enforcement genuinely off, not just deferred to commit. `bunny db migrations apply` runs this way. @@ -160,7 +161,7 @@ try { | `code` | SQLite code (`SQLITE_CONSTRAINT`), or a client code (`UNAUTHORIZED`, `URL_MISSING`) | | `status` | HTTP status, when the failure came from the transport rather than from SQL | -Client codes for pre-SQL failures: `NETWORK` (DNS, TLS, connection refused, or a non-JSON response), `TIMEOUT` (the `timeout` deadline elapsed), `ABORTED` (the `signal` was aborted). +Client codes for pre-SQL failures: `NETWORK` (DNS, TLS, connection refused, or a non-JSON response), `TIMEOUT` (the `timeout` deadline elapsed), `ABORTED` (the `signal` was aborted), `PROTOCOL` (a 200 whose body is not a pipeline response). `error.cause` holds the runtime's original error. A `DatabaseError` carries the server's message and SQLite code, so returning one straight to a caller can leak schema details. Log it and return something generic.