Skip to content

Commit a5acd49

Browse files
committed
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
1 parent 5485a32 commit a5acd49

6 files changed

Lines changed: 370 additions & 3 deletions

File tree

.changeset/driver-raw-statement-declared-fault.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,14 @@
33
"@objectstack/driver-turso": patch
44
---
55

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`:** no exported type, signature or option changes; this puts an envelope the driver already emits onto an existing refusal, on the one exit that still let the dialect's raw error object out undeclared.
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`.
77

88
**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.
99

10-
**What moves on the wire.** A driver fault on the raw path now reaches `POST /api/v1/analytics/dataset/query` as `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 heuristic happened to fire and the raw engine text when it did not. The status is unchanged; the code is now the producer's, exactly as the read exits' faults already answer.
10+
**What moves on the wire — three doors, each because a declared fault is relayed where an undeclared one was re-labelled.**
1111

12-
**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.
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` 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 one follow-up card (read `cause` there).

packages/rest/src/analytics-16019-driver-declared-fault.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@
5151
* `select ` limb — and the driver-log assertion goes RED (the driver no longer
5252
* logs; the route's `logError` becomes the only copy). The second block stays
5353
* GREEN throughout: it hands the door shapes that never touch the driver.
54+
*
55+
* The ORDERING pin in block 2 has its own leg: gate the door's ③a relay
56+
* behind `looksLikeInternalErrorLeak` being false (i.e. consult the heuristic
57+
* first) and only that case goes RED (`ANALYTICS_QUERY_FAILED` in place of the
58+
* producer's code); the neighbouring "phrase the heuristic does not know"
59+
* case stays GREEN, which is precisely why it could not stand in for this one.
5460
*/
5561

5662
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
@@ -270,6 +276,26 @@ describe('[#16019] at the door: a declaration wins over the heuristic, and the h
270276
expect(JSON.stringify(res.body)).not.toContain('translate');
271277
});
272278

279+
it("DECLARED, with a phrase the heuristic DOES know → the producer's code, not the fallback's (the ORDERING pin)", async () => {
280+
// The one shape that discriminates the order of the two arms: the message
281+
// trips `looksLikeInternalErrorLeak` AND the error declares. Declared-first
282+
// (③a before ③b, the door as written) answers the producer's code;
283+
// heuristic-first would answer `ANALYTICS_QUERY_FAILED` with the same
284+
// withheld text and this case alone would go red. The case above cannot
285+
// tell the two orders apart, because its message trips nothing.
286+
expect(looksLikeInternalErrorLeak(KNEX)).toBe(true);
287+
const declared = Object.assign(new Error(KNEX), { code: 'DATABASE_ERROR', status: 500 });
288+
expect(declaresServerFault(declared)).toBe(true);
289+
290+
const res = await post(buildRoute(async () => throwingAnalytics(declared)), { dataset, selection });
291+
292+
expect(res.statusCode).toBe(500);
293+
expect(res.body.code).toBe('DATABASE_ERROR');
294+
expect(res.body.code).not.toBe('ANALYTICS_QUERY_FAILED');
295+
expect(res.body.error).toBe(INTERNAL_ERROR_MESSAGE);
296+
expect(JSON.stringify(res.body)).not.toContain('translate');
297+
});
298+
273299
it("UNDECLARED, the bare shape → the fallback's coverage boundary, pinned as a live subject", async () => {
274300
// ⛔ Asserting the residual, not endorsing it — see the file header. A
275301
// producer that reaches this door with dialect text and no declaration is
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#16019] `POST /api/v1/packages/publish` and `DELETE /api/v1/packages/:id`
5+
* — the wire `code` a raw-exec driver fault answers moved, and this file pins
6+
* the flip at the door.
7+
*
8+
* ## The flip
9+
*
10+
* `PackageService.publish` / `delete` (`service-package/src/index.ts`) wrap
11+
* `objectql.execute(...)` in a catch whose branch ② re-throws any error that
12+
* `declaresHttpAnswer` — a numeric `status` or `statusCode` — and whose branch
13+
* ③ swallows everything else as a driver fault, returning `{ success: false }`
14+
* for the door's `sendError` to answer `500 PACKAGE_PUBLISH_FAILED` /
15+
* `500 PACKAGE_DELETE_FAILED`.
16+
*
17+
* Before #16019 a raw-exec driver fault carried no `status` (knex's error
18+
* object: `code: 'SQLITE_ERROR'`, message `STATEMENT - DIAGNOSTIC`) → branch
19+
* ③. Since #16019 `SqlDriver.execute()` declares it — `code: DATABASE_ERROR`,
20+
* `status: 500`, a composed message, the dialect error under a non-enumerable
21+
* `cause` — → branch ② re-throws it → this door's catch-all `sendThrownError`
22+
* → `500 DATABASE_ERROR`, the composed sentence as the message (it trips no
23+
* phrasing heuristic, so it is not replaced by `INTERNAL_ERROR_MESSAGE`; it
24+
* carries no dialect word to withhold). Same status band, no disclosure
25+
* either way; the ledgered `code` on two published doors moves.
26+
*
27+
* The catch's own half — that the declared fault propagates UNCHANGED and the
28+
* undeclared ancestor still takes branch ③ — is pinned where the catch lives,
29+
* in `service-package`'s `publish-driver-fault.test.ts` /
30+
* `delete-driver-fault.test.ts` (`[#16019]` blocks, identity-asserted). This
31+
* file takes the re-thrown object from there and pins what the DOOR answers,
32+
* with a `PackageService` double that throws it — the shape every
33+
* `packageService.publish throws` case in `package-door-5xx-message-sanitization.test.ts`
34+
* uses — so `@objectstack/service-package` is not imported into this package's
35+
* test layer (it is not in `rest`'s unaliased-import ledger).
36+
*
37+
* ⛔ Not a re-judgement of either catch: `declaresHttpAnswer`'s docblock
38+
* already says a declared 5xx is re-thrown too. The contract review of PR
39+
* #16650 required the consequence to be NAMED and PINNED, nothing else.
40+
*/
41+
42+
import { describe, it, expect, vi } from 'vitest';
43+
import { ApiErrorSchema, BaseResponseSchema, envelopeViolations } from '@objectstack/spec/api';
44+
import type { RouteHandler } from '@objectstack/spec/contracts';
45+
import { INTERNAL_ERROR_MESSAGE, looksLikeInternalErrorLeak } from '@objectstack/types';
46+
import { registerPackageRoutes } from './package-routes.js';
47+
48+
const PKGS = '/api/v1/packages';
49+
const MANIFEST = { id: 'com.acme.crm', version: '1.0.0' };
50+
51+
/** A caller holding every capability these routes gate on. */
52+
const CLEARS_THE_GATE = async () => ({
53+
userId: 'u_pkg',
54+
systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'],
55+
});
56+
57+
interface Captured {
58+
status: number;
59+
body: any;
60+
}
61+
62+
function mount(svc: Record<string, unknown>) {
63+
const routes = new Map<string, RouteHandler>();
64+
const server = {
65+
get: (p: string, h: RouteHandler) => { routes.set(`GET:${p}`, h); },
66+
post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); },
67+
put: (p: string, h: RouteHandler) => { routes.set(`PUT:${p}`, h); },
68+
delete: (p: string, h: RouteHandler) => { routes.set(`DELETE:${p}`, h); },
69+
patch: () => {},
70+
use: () => {},
71+
listen: async () => {},
72+
close: async () => {},
73+
} as any;
74+
registerPackageRoutes(server, () => svc as any, '/api/v1', {
75+
resolveExecutionContext: CLEARS_THE_GATE,
76+
} as any);
77+
return routes;
78+
}
79+
80+
async function drive(
81+
routes: Map<string, RouteHandler>,
82+
method: string,
83+
path: string,
84+
req: Record<string, any> = {},
85+
): Promise<Captured> {
86+
const handler = routes.get(`${method}:${path}`);
87+
if (!handler) throw new Error(`no handler for ${method} ${path}`);
88+
const captured: Captured = { status: 0, body: undefined };
89+
const res: any = {
90+
json(data: any) { captured.body = data; },
91+
send() {},
92+
status(code: number) { captured.status = code; return res; },
93+
header() { return res; },
94+
};
95+
await handler(
96+
{ params: {}, query: {}, body: undefined, headers: {}, method, path, ...req } as any,
97+
res,
98+
);
99+
return captured;
100+
}
101+
102+
/** The wire contract, imported rather than restated. */
103+
function expectDeclaredEnvelope(captured: Captured): any {
104+
expect(BaseResponseSchema.safeParse(captured.body).success).toBe(true);
105+
expect(envelopeViolations(captured.body)).toEqual([]);
106+
expect(captured.body?.success).toBe(false);
107+
const parsed = ApiErrorSchema.safeParse(captured.body?.error);
108+
expect(parsed.error?.issues ?? []).toEqual([]);
109+
expect(parsed.success).toBe(true);
110+
return captured.body.error;
111+
}
112+
113+
const DIALECT_LINE = 'insert into `sys_packages` (`id`, …) values (…) - no such table: sys_packages';
114+
const COMPOSED =
115+
'The database refused to run a raw statement. The driver could not attribute the failure ' +
116+
'to any part of the request, so no verdict about the statement is claimed here. The ' +
117+
"backend's own diagnostic and the statement were written to the server log for an " +
118+
'operator to read.';
119+
120+
/** What `SqlDriver.execute()` raises since #16019, and what the service's branch ② re-throws. */
121+
function rawStatementFault(): Error {
122+
const err = Object.assign(new Error(COMPOSED), { code: 'DATABASE_ERROR', status: 500 });
123+
Object.defineProperty(err, 'cause', {
124+
value: Object.assign(new Error(DIALECT_LINE), { code: 'SQLITE_ERROR' }),
125+
enumerable: false, writable: true, configurable: true,
126+
});
127+
return err;
128+
}
129+
130+
async function publishWith(svc: Record<string, unknown>): Promise<Captured> {
131+
return drive(mount(svc), 'POST', `${PKGS}/publish`, {
132+
body: { manifest: MANIFEST, metadata: { author: 'acme' } },
133+
});
134+
}
135+
136+
async function deleteWith(svc: Record<string, unknown>): Promise<Captured> {
137+
return drive(mount(svc), 'DELETE', `${PKGS}/:id`, { params: { id: 'com.acme.crm' } });
138+
}
139+
140+
describe('[#16019] a raw-exec driver fault under sys_packages answers the producer\'s code on both package doors', () => {
141+
// The control that makes the assertions below about the DECLARATION and not
142+
// about the heuristic: the composed sentence trips nothing.
143+
it('the composed sentence is not a phrase the door\'s withhold heuristic knows', () => {
144+
expect(looksLikeInternalErrorLeak(COMPOSED)).toBe(false);
145+
expect(looksLikeInternalErrorLeak(DIALECT_LINE)).toBe(true);
146+
});
147+
148+
it('POST /packages/publish — AFTER #16019: the re-thrown declared fault → 500 DATABASE_ERROR, composed sentence, no dialect word', async () => {
149+
const publish = vi.fn(async () => { throw rawStatementFault(); });
150+
const captured = await publishWith({ publish });
151+
152+
expect(publish).toHaveBeenCalledTimes(1);
153+
expect(captured.status).toBe(500);
154+
const error = expectDeclaredEnvelope(captured);
155+
expect(error.code).toBe('DATABASE_ERROR');
156+
expect(error.code).not.toBe('PACKAGE_PUBLISH_FAILED');
157+
expect(error.message).toBe(COMPOSED);
158+
expect(JSON.stringify(captured.body)).not.toMatch(/sys_packages|no such table|insert into/i);
159+
});
160+
161+
it('POST /packages/publish — BEFORE #16019: the swallowed driver fault → 500 PACKAGE_PUBLISH_FAILED (the control, still what an undeclared fault answers)', async () => {
162+
// Branch ③'s return shape, verbatim from the service.
163+
const publish = vi.fn(async () => ({ success: false, driverFault: { message: 'The package was not persisted.' } }));
164+
const captured = await publishWith({ publish });
165+
166+
expect(captured.status).toBe(500);
167+
const error = expectDeclaredEnvelope(captured);
168+
expect(error.code).toBe('PACKAGE_PUBLISH_FAILED');
169+
});
170+
171+
it('DELETE /packages/:id — AFTER #16019: the re-thrown declared fault → 500 DATABASE_ERROR, composed sentence, no dialect word', async () => {
172+
const del = vi.fn(async () => { throw rawStatementFault(); });
173+
const captured = await deleteWith({ delete: del });
174+
175+
expect(del).toHaveBeenCalledTimes(1);
176+
expect(captured.status).toBe(500);
177+
const error = expectDeclaredEnvelope(captured);
178+
expect(error.code).toBe('DATABASE_ERROR');
179+
expect(error.code).not.toBe('PACKAGE_DELETE_FAILED');
180+
expect(error.message).toBe(COMPOSED);
181+
expect(JSON.stringify(captured.body)).not.toMatch(/sys_packages|no such table|insert into/i);
182+
});
183+
184+
it('DELETE /packages/:id — BEFORE #16019: the swallowed driver fault → 500 PACKAGE_DELETE_FAILED (the control)', async () => {
185+
const del = vi.fn(async () => ({ success: false }));
186+
const captured = await deleteWith({ delete: del });
187+
188+
expect(captured.status).toBe(500);
189+
const error = expectDeclaredEnvelope(captured);
190+
expect(error.code).toBe('PACKAGE_DELETE_FAILED');
191+
});
192+
193+
it('the withhold is untouched: a DECLARED fault whose message DOES carry dialect text is still replaced at this door', async () => {
194+
// Beside the flip, the invariant #8086 pinned: `sendThrownError` withholds
195+
// a leaky 5xx message whatever the code — so a producer that declared but
196+
// let dialect text into its message would still not disclose it here.
197+
const leaky = Object.assign(new Error(DIALECT_LINE), { code: 'DATABASE_ERROR', status: 500 });
198+
const publish = vi.fn(async () => { throw leaky; });
199+
const captured = await publishWith({ publish });
200+
201+
expect(captured.status).toBe(500);
202+
const error = expectDeclaredEnvelope(captured);
203+
expect(error.code).toBe('DATABASE_ERROR');
204+
expect(error.message).toBe(INTERNAL_ERROR_MESSAGE);
205+
});
206+
});

0 commit comments

Comments
 (0)