Skip to content

Commit 001a83b

Browse files
os-muskclaude
andauthored
fix(driver-sql, driver-turso): SqlDriver.execute() declares a backend refusal as DATABASE_ERROR/500 instead of leaking the dialect's error object (#16650)
* wip(#16019): declare raw-statement faults on SqlDriver.execute and the Turso remote transport Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * wip(#16019): pins at the driver exits, the types predicate and the analytics door; changeset Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * fix(driver-sql): keep tracker ids out of the raw-statement fault's log prose check:doc-authoring's cross-package prose-id leg counts issue ids inside string prose; the new warn line carried two. The docblock keeps its provenance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * wip(#16019): review round — pin the package-door code flip, the door ordering, annotate the degrade docblocks, name three doors in the changeset Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * docs(#16019): delta review — the publish door is /packages/publish, the degrade notes exclude turso remote, the card is cited by number Text only: the changeset named the dispatcher's install route as the moving door and cited the follow-up card anonymously; the two degrade docblocks claimed the declared answer for driver-turso's remote transport, whose typed exits are undeclared and pre-date this card; the catch comment below them still said a declared 5xx is served through the ANALYTICS_QUERY_FAILED path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg * test(driver-sql): the prefix-unique measurement reads MySQL's rejection off the declared envelope's cause The live-MySQL pin in sql-driver-keyed-text-mysql.test.ts asserted toThrow(/Duplicate entry/i) on execute()'s rejection — the dialect text off error.message, which the raw-path envelope now composes. It was the one red in the Temporal Conformance job on every head of this branch (step 12, ~100 s in), reproduced locally against MySQL 8.0.46 + PG 16.13 with the job's zone settings. The measurement is unchanged: the second value is rejected as a duplicate (isUniqueViolationError follows cause; the cause's own line still reads Duplicate entry) and one row remains. 167 files / 3599 tests green against both live servers after the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c8e5ac6 commit 001a83b

13 files changed

Lines changed: 1198 additions & 11 deletions
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
"@objectstack/driver-turso": patch
4+
---
5+
6+
`SqlDriver.execute()` — the raw-SQL path the analytics compilers run on — now declares a backend refusal the way the typed read exits (`find` / `count` / `aggregate`) have since #8931: `code: DATABASE_ERROR`, `status: 500`, a composed message that carries none of the dialect's words, and the dialect error whole under a non-enumerable `cause`. `TursoDriver` in remote mode — the one transport that hands the engine's text back with no statement in front of it — declares through the same terminal, so both transports leave the driver with one envelope. **Graded `patch`** on AGENTS.md's changeset rule ("A bug fix in a released package takes a `patch` changeset"; breaking is what removes or renames something an author can write — a spec key, an export, a config field — and nothing here does: `execute()` stays `Promise` of `any`, and `code` / `status` were untyped before) and on the precedent of the identical change on the typed read exits, #8931 via PR #9273, which shipped `@objectstack/driver-sql: patch`.
7+
8+
**The defect this closes (#16019, folding in the envelope half of #16028).** `no such function: translate` — what SQLite answers when a compiler emits a function the dialect lacks — left `execute()` as knex's own error: `code: 'SQLITE_ERROR'`, no `status`, message `<statement> - no such function: translate`. Undeclared, it fell to the HTTP doors' phrasing heuristic (`looksLikeInternalErrorLeak`), which recognises `no such column:` and not `no such function:`, so whether the caller saw the engine's text depended on which limb the message happened to match: through knex it was withheld by accident (the statement prefix starts with `select`), through the Turso remote transport it was withheld by a different accident (`SQLITE_ERROR:` in front), and a bare `Error('no such function: translate')` reached the body verbatim. Maintainer ruling 2026-09-06 (decision batch #57, option 3): the substring list is not grown; the driver declares its own fault and the doors classify on the declaration. The heuristic stays as the last-resort fallback for an error that arrives with no declaration.
9+
10+
**What moves on the wire — three doors, each because a declared fault is relayed where an undeclared one was re-labelled.**
11+
12+
- `POST /api/v1/analytics/dataset/query`: a driver fault on the raw path answers `500 {"code":"DATABASE_ERROR","error":"Internal server error"}` — the declared-fault relay, the same answer the `/data` door and `/analytics/query` already give a declared 5xx — where it was `500 {"code":"ANALYTICS_QUERY_FAILED","error":"Internal server error"}` when the phrasing heuristic happened to fire and the raw engine text when it did not. Status unchanged; the code is now the producer's, exactly as the typed read exits' faults have answered at this door since PR #9273.
13+
- The same door, a dataset over a backing table that is NOT present, on the native-SQL strategy (the strategy every deployment whose data engine exposes `execute()` runs): `500 DATABASE_ERROR` where it was `200 {"rows":[],"fields":[],"totals":[]}` plus a `warn`. `queryDataset`'s missing-source degrade sits behind its declared-envelope re-throw (#5717 defence B: a declared envelope is re-thrown untouched, whatever it says), so a driver-raised missing table no longer reaches it — the answer the ObjectQL-aggregate strategy has given since #9273, now on both strategies. The degrade still applies to an undeclared producer (an embedder's own `executeRawSql`, the framework's not-registered signals).
14+
- `POST /api/v1/packages/publish` and `DELETE /api/v1/packages/:id`: a raw-exec driver fault under `sys_packages` answers `500 {"code":"DATABASE_ERROR"}` with the composed sentence as its message — `PackageService.publish` / `delete` re-throw a throw that declares an HTTP answer (`declaresHttpAnswer`, whose docblock already says a declared 5xx is re-thrown too) and the door's `sendThrownError` relays it — where it was `500 PACKAGE_PUBLISH_FAILED` / `500 PACKAGE_DELETE_FAILED` from the swallowing branch. Same status band, no dialect text on the wire either way; the ledgered `code` on those two doors moves.
15+
16+
**What a consumer of `execute()` sees.** `error.message` is the composed sentence; `error.code` is `DATABASE_ERROR` where it was the backend's errno; `error.status` is `500` where it was absent. The backend's error object — its errno, its diagnostic, and on the dialects that inline them the bound literals — is on `error.cause` (non-enumerable, so it does not serialise), and the driver writes it, with the statement, to its warn log before composing. Cause-following predicates are unaffected: `isMissingTableError(err, readObject)` still classifies a missing table raised on this path. An error that already declares a `status` is passed through untouched, never double-wrapped. A caller that read the dialect's text off `error.message` (a migration preflight recording it as its `detail`, say) now reads the composed sentence there and finds the dialect text on `cause` and in the log; the in-repo sites of that class are tracked as #16657 (read `cause` there).
Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16019] `SqlDriver.execute()` — the raw-SQL path every analytics compiler
5+
* runs on — declares its own fault.
6+
*
7+
* ## The gap, measured on this tree
8+
*
9+
* The typed read exits terminate in `backendStatementFault`, so a dialect
10+
* refusal on `find` / `count` / `aggregate` leaves the driver as a declared
11+
* `DATABASE_ERROR`/500. The raw path had no terminal: `execute()` awaited
12+
* `knex.raw()` bare, and knex's executor hands back the dialect's own error
13+
* object — `code: 'SQLITE_ERROR'`, no `status`, and the message
14+
* `<statement> - <diagnostic>`. Measured on knex 3.3.0 + better-sqlite3:
15+
*
16+
* ```
17+
* select translate('ABC', 'ABC', 'abc') as x - no such function: translate
18+
* ```
19+
*
20+
* The prefix is unconditional (`compileSqlOnError: false` only changes how the
21+
* statement is formatted; it never drops it), so through knex the HTTP doors'
22+
* phrasing heuristic withheld this text by ACCIDENT — `startsWith('select ')`
23+
* — while `no such function:` itself matched nothing, and a transport that
24+
* hands the engine's text back without a statement (`driver-turso` remote
25+
* mode, pinned in its own package) reached the same door bare. Neither was a
26+
* declaration.
27+
*
28+
* ## Maintainer ruling 2026-09-06 (decision batch #57, option 3)
29+
*
30+
* The substring list in `looksLikeInternalErrorLeak` is not grown. The driver
31+
* declares its fault and the doors classify on the declaration. These pins
32+
* are that declaration at the layer that produces it, in every direction that
33+
* matters: the composed envelope carries `code` + `status` and none of the
34+
* dialect's words; the dialect error survives whole under a NON-ENUMERABLE
35+
* `cause`, so cause-following classification (`isMissingTableError`) keeps
36+
* working; and an error that already declares a status passes through, never
37+
* double-wrapped.
38+
*
39+
* ## Reverse verification, direction predicted BEFORE running
40+
*
41+
* Restore the bare `await builder` in `execute()` (delete its `try`/`catch`)
42+
* and the envelope cases go RED on `code` / `status` (`SQLITE_ERROR` and
43+
* `undefined` in their place) and on the message assertions (the statement and
44+
* `no such function` are then IN the message). The positive control and the
45+
* pass-through gate stay GREEN — that leg never hands the gate a knex error.
46+
* Recorded in the PR, both legs.
47+
*/
48+
49+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
50+
import { SqlDriver } from './index.js';
51+
import { declaresServerFault, isMissingTableError, looksLikeInternalErrorLeak } from '@objectstack/types';
52+
53+
/** The shape `declaresServerFault` and both HTTP doors read off a thrown driver error. */
54+
interface WireBearingError extends Error {
55+
code?: string;
56+
status?: number;
57+
cause?: unknown;
58+
}
59+
60+
/** The statement SQLite refuses — `translate()` is the #16028 fault verbatim. */
61+
const TRANSLATE_SQL = "select translate('ABC', 'ABC', 'abc') as x";
62+
63+
async function faultOf(run: () => Promise<unknown>): Promise<WireBearingError> {
64+
try {
65+
await run();
66+
} catch (e) {
67+
return e as WireBearingError;
68+
}
69+
throw new Error('expected the driver to refuse this statement, but it resolved');
70+
}
71+
72+
/**
73+
* A driver whose log sink is captured, so the server-side copy of the dialect
74+
* text can be asserted, and whose protected terminal is exposed for the
75+
* pass-through pin.
76+
*/
77+
class LoggedSqlDriver extends SqlDriver {
78+
readonly warned: string[] = [];
79+
80+
constructor() {
81+
super({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true });
82+
this.logger = { warn: (msg: string) => { this.warned.push(msg); } };
83+
}
84+
85+
terminal(command: string, error: unknown): Error {
86+
return this.rawStatementFault(command, error);
87+
}
88+
}
89+
90+
describe('[#16019] SqlDriver.execute() declares a backend refusal as DATABASE_ERROR/500', () => {
91+
let driver: LoggedSqlDriver;
92+
93+
beforeEach(() => {
94+
driver = new LoggedSqlDriver();
95+
});
96+
97+
afterEach(async () => {
98+
await driver.disconnect();
99+
});
100+
101+
it("the statement SQLite refuses leaves execute() as a declared fault carrying none of the dialect's words", async () => {
102+
const err = await faultOf(() => driver.execute(TRANSLATE_SQL));
103+
104+
expect(err.code).toBe('DATABASE_ERROR');
105+
expect(err.status).toBe(500);
106+
expect(declaresServerFault(err)).toBe(true);
107+
expect(err.message).toMatch(/refused to run a raw statement/);
108+
// Not the statement, not the diagnostic, not the function name we emitted.
109+
expect(err.message).not.toMatch(/translate/i);
110+
expect(err.message).not.toMatch(/no such function/i);
111+
expect(err.message).not.toMatch(/select/i);
112+
});
113+
114+
it('the dialect error travels whole under a NON-ENUMERABLE cause — the knex shape, statement prefixed', async () => {
115+
const err = await faultOf(() => driver.execute(TRANSLATE_SQL));
116+
const cause = err.cause as WireBearingError;
117+
118+
expect(cause).toBeInstanceOf(Error);
119+
// knex 3.3.0's executor: `<formatted statement> - <engine diagnostic>`, unconditionally.
120+
expect(cause.message).toBe(`${TRANSLATE_SQL} - no such function: translate`);
121+
expect(cause.code).toBe('SQLITE_ERROR');
122+
// Readable by code, invisible to serialisation — the same carrier discipline
123+
// `backendStatementFaultError` applies one terminal over.
124+
expect(Object.getOwnPropertyDescriptor(err, 'cause')?.enumerable).toBe(false);
125+
expect(Object.keys(err)).not.toContain('cause');
126+
expect(JSON.stringify(err)).not.toMatch(/translate/);
127+
});
128+
129+
it('the declaration, not the phrasing heuristic, is what withholds it', async () => {
130+
const err = await faultOf(() => driver.execute(TRANSLATE_SQL));
131+
132+
// The heuristic never covered the engine's phrase, and the composed message
133+
// gives it nothing to recognise either: the doors withhold on the declaration.
134+
expect(looksLikeInternalErrorLeak('no such function: translate')).toBe(false);
135+
expect(looksLikeInternalErrorLeak(err.message)).toBe(false);
136+
expect(declaresServerFault(err)).toBe(true);
137+
// Control: the sibling limb the heuristic DOES cover, so the `false` above is
138+
// a reading about the phrase and not about a broken probe.
139+
expect(looksLikeInternalErrorLeak('no such column: bogus_dim')).toBe(true);
140+
});
141+
142+
it('writes the statement and the dialect message to the server log — after this change, the only copy', async () => {
143+
await faultOf(() => driver.execute(TRANSLATE_SQL));
144+
145+
const line = driver.warned.find((m) => m.includes('DATABASE_ERROR'));
146+
expect(line).toBeDefined();
147+
expect(line).toContain('(SQLITE_ERROR)');
148+
expect(line).toContain(TRANSLATE_SQL);
149+
expect(line).toContain('no such function: translate');
150+
});
151+
152+
it('a missing table on the raw path stays classifiable through `cause` (isMissingTableError)', async () => {
153+
const err = await faultOf(() => driver.execute('select 1 from nope_16019'));
154+
155+
expect(err.code).toBe('DATABASE_ERROR');
156+
expect(err.status).toBe(500);
157+
expect(err.message).not.toMatch(/nope_16019/);
158+
expect(isMissingTableError(err, 'nope_16019')).toBe(true);
159+
// Control: the same envelope is not a missing-table verdict about some
160+
// OTHER relation — no `DRIVER_TARGETED_TABLE` is declared on this path, so
161+
// the comparison is the caller's name against the phrase, as before.
162+
expect(isMissingTableError(err, 'other_16019')).toBe(false);
163+
});
164+
165+
it('an error that already declares a status passes through untouched — never double-wrapped', () => {
166+
const declared = Object.assign(new Error('the Query Protocol has no such function'), {
167+
code: 'INVALID_QUERY',
168+
status: 400,
169+
});
170+
171+
expect(driver.terminal('select 1', declared)).toBe(declared);
172+
expect(driver.warned).toHaveLength(0);
173+
});
174+
175+
it('POSITIVE CONTROL: a statement the engine runs still resolves with its rows, and logs nothing', async () => {
176+
const rows: unknown = await driver.execute('select 1 as x');
177+
const first = Array.isArray(rows) ? rows[0] : (rows as { rows?: unknown[] })?.rows?.[0];
178+
179+
expect(first).toEqual({ x: 1 });
180+
expect(driver.warned).toHaveLength(0);
181+
});
182+
});

‎packages/drivers/driver-sql/src/sql-driver-keyed-text-mysql.test.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
import { describe, it, expect, afterEach } from 'vitest';
4343
import { SqlDriver } from '../src/index.js';
44+
import { isUniqueViolationError } from '@objectstack/types';
4445
import { MYSQL_CELL, dialectCell, declareDialectCell } from './live-dialect-matrix.testkit.js';
4546

4647
/**
@@ -314,12 +315,23 @@ declareDialectCell(MYSQL_CELL, 'keyed text columns (#11374)', (cell) => {
314315
first,
315316
] as any);
316317

317-
await expect(
318-
driver.execute(`insert into os11374_prefix (id, token) values (?, ?)`, [
319-
'r2',
320-
second,
321-
] as any),
322-
).rejects.toThrow(/Duplicate entry/i);
318+
// [#16019] `execute()` declares its fault: `DATABASE_ERROR` / 500 with a
319+
// composed message, the dialect error whole under a non-enumerable
320+
// `cause`. The measurement is unchanged — MySQL rejects the second value
321+
// as a duplicate — and it is read where the dialect's text now lives:
322+
// the cause-following `isUniqueViolationError`, and the cause's own
323+
// `Duplicate entry` line. Reading `error.message` for it was what this
324+
// pin did before, and it is the one raw-exec consumer in this package
325+
// that did; the declared envelope is what every door reads.
326+
const rejection: unknown = await driver
327+
.execute(`insert into os11374_prefix (id, token) values (?, ?)`, ['r2', second] as any)
328+
.then(() => undefined, (e: unknown) => e);
329+
expect(rejection).toBeInstanceOf(Error);
330+
expect((rejection as { code?: unknown }).code).toBe('DATABASE_ERROR');
331+
expect((rejection as { status?: unknown }).status).toBe(500);
332+
expect(isUniqueViolationError(rejection)).toBe(true);
333+
const dialect = (rejection as { cause?: unknown }).cause as { message?: unknown } | undefined;
334+
expect(String(dialect?.message)).toMatch(/Duplicate entry/i);
323335

324336
// One row, from two distinct tokens: the second was lost to a constraint
325337
// the object never declared.

0 commit comments

Comments
 (0)