Skip to content

fix(driver-sql, driver-turso): SqlDriver.execute() declares a backend refusal as DATABASE_ERROR/500 instead of leaking the dialect's error object - #16650

Merged
os-musk merged 9 commits into
mainfrom
claude/issue-16019-driver-declared-fault
Sep 7, 2026
Merged

fix(driver-sql, driver-turso): SqlDriver.execute() declares a backend refusal as DATABASE_ERROR/500 instead of leaking the dialect's error object#16650
os-musk merged 9 commits into
mainfrom
claude/issue-16019-driver-declared-fault

Conversation

@os-musk

@os-musk os-musk commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16019

Clause-②: yes

What this lands

SqlDriver.execute() — the raw-SQL path every analytics compiler runs on — now terminates a backend refusal the way the typed read exits (find / count / aggregate) have since #8931: a composed DATABASE_ERROR / 500 envelope with none of the dialect's words, the dialect error whole under a non-enumerable cause, and the statement plus dialect message written to the driver's warn log. TursoDriver in remote mode — the one in-repo transport that hands the engine's text back with no statement in front of it — declares through the same protected terminal, so both transports leave the driver with one shape. looksLikeInternalErrorLeak is untouched in code; its docblock now records the ruling that froze it as the last-resort fallback.

Maintainer ruling, decision batch #57, 2026-09-06 (issue comment 5559821104), reply verbatim: 「同意」 — option 3. ⛔ Option 1 (widening the heuristic) was not taken: no row was added, and no such function: is pinned as UNCOVERED by the list. This also folds in the envelope half of #16028 (its compilation half already landed in PR #16204; the envelope half is exactly this declaration).

CI round — Temporal Conformance (live PG + MySQL) was red on every head of this branch, and it was this PR's. Measured, not picked: the job's step 12 (Run driver-sql suite against both live servers) concluded failure on 709d51df1f, a5acd49733 and 3fdd44c69c alike (each ~100 s into the step), and success on main at my merge base cf74a11285 — so it was neither the text round's delta (0 executable lines) nor the merge; it was the driver change on a cell that only runs with a live MySQL. Reproduced locally on 3fdd44c69c with PostgreSQL 16.13 (Asia/Shanghai) and MySQL 8.0.46 (+08:00) provisioned on this box, TZ=America/New_York, OS_EXPECT_LIVE_DIALECT_MATRIX=1, the job's own command (pnpm --filter @objectstack/driver-sql test): Test Files 1 failed | 166 passed (167), Tests 1 failed | 3598 passed | 1 skipped (3600), duration 98.79 s — one red, src/sql-driver-keyed-text-mysql.test.ts:322 MEASUREMENT: a prefix-unique index rejects two DIFFERENT values sharing the prefix: expected [Function] to throw error matching /Duplicate entry/i but got 'The database refused to run a raw sta…'. That pin read MySQL's rejection off execute()'s error.message — the one raw-exec consumer in this package that did, and exactly the class the changeset declares (the dialect text now lives on cause). Fixed in 61b85c009b by re-aiming the pin, not the driver: the measurement is unchanged (the second value is rejected as a duplicate, one row remains) and is read where the text lives — the declared DATABASE_ERROR/500 envelope, the cause-following isUniqueViolationError, and the cause's own Duplicate entry line. Re-run the same way: Test Files 167 passed (167), Tests 3599 passed | 1 skipped (3600), exit 0; the job's live-MySQL migration leg (pnpm --filter @objectstack/metadata-protocol exec vitest run live-mysql): 3 passed (3), 19 passed (19), exit 0; the touched non-SQL package under the job's skewed zone (service-analytics, TZ=America/New_York): 96 passed (96), 2163 passed (2163), exit 0; driver-sql typecheck exit 0. The servers were stopped by their recorded PIDs and their data dirs removed.

Text round (delta review, issue comment 5574921392 — PASS WITH FINDINGS, three text corrections). In 3fdd44c69c (after merging origin/main cf74a11285): the changeset names the moving publish door as POST /api/v1/packages/publish (the bare POST /packages is the dispatcher's install route, #3610) and cites #16657 by number; the two degrade docblocks no longer claim the declared answer for driver-turso's REMOTE transport, whose typed exits are undeclared and pre-date this card (RemoteTransport.aggregate swallows a missing table into [] on the ObjectQL-aggregate strategy — pre-existing, not filed from here), and the pre-existing catch comment below them no longer says a declared 5xx is served through the ANALYTICS_QUERY_FAILED path; #16657 drops the three swallow-only sites (overlay-index.ts, sys-setting-identity-index.ts, protocol.ts). Text only — no ablation is owed; the pins that read the edited files were re-run (below).

Review round (contract review, issue comment 5573043610 — verdict FAIL on disclosure, substance CORRECT). Addressed in a5acd49733 and below, item by item: ① the two package doors are named and their code flip pinned at the catch and at the door; ② the analytics 200 → 500 move is in the changeset; ③ the two degrade docblocks now say what happens; ④ an ORDERING pin whose only red is a reversed arm order, proven by ablation; ⑤ the operator-detail class is filed as #16657; ⑥ semver re-cited. Nothing the review called CORRECT or CORRECTLY DEFERRED was touched.

The measurement the round opened on — why the read path never reached the backendStatementFaultError wrapper

Re-derived by symbol on 8341ed21ce (the dispatch's tree) and again on the merged head:

  • The wrapper's only caller is SqlDriver.backendStatementFault, the terminal of the TYPED read exits — findRows (the find half), count, and aggregateBackendFault (aggregate): three throw this.backendStatementFault(...) sites and one return.
  • The analytics native-SQL strategy uses none of them. NativeSQLStrategy.execute calls ctx.executeRawSql; the plugin's auto-bridge (service-analytics/src/plugin.ts) calls engine.execute(knexSql, { args, object }); ObjectQL.execute only selects the driver and calls driver.execute(rawCommand, params, options); and SqlDriver.execute awaited this.knex.raw(...) with no try at all. The raw path had no terminal, so the dialect's error object left the driver as knex handed it back: code: 'SQLITE_ERROR', no status, message shaped STATEMENT - DIAGNOSTIC.
  • The ObjectQLStrategy path goes engine.aggregatedriver.aggregateaggregateBackendFault → already declared. That is why the raw path was the one exit left.

Which paths produce the bare shape — measured, not reasoned

producer error reaching the door status withheld on origin/main by
SqlDriver.execute via knex 3.3.0 + better-sqlite3 select translate('ABC', 'ABC', 'abc') as x - no such function: translate, code: 'SQLITE_ERROR' none the heuristic's startsWith('select ') limb — by accident
same, with knex compileSqlOnError: false identical prefix (only the statement's formatting changes; query-executioner.js prefixes unconditionally) none the same accident — ⛔ this falsifies the premise that a knex option yields the bare shape
TursoDriver remote → RemoteTransport.execute@libsql/client on file::memory: SQLITE_ERROR: no such function: translate (LibsqlError, code: 'SQLITE_ERROR') — no statement none the heuristic's sqlite_ limb — by a different accident
the stub the remote suites use (better-sqlite3 wearing the libsql interface) no such function: translate — the card's own shape none nothing
an embedder-supplied executeRawSql (AnalyticsPluginOptions.executeRawSql, documented with a pg pool example) whatever that client raises, bare none nothing — ⛔ outside every driver in this repo; see the report's open questions

Both driver-raised shapes now land on the declared path (DATABASE_ERROR / 500) and are driven in this PR's tests. The last row is the residual the ruling leaves to producers; it is pinned as the fallback's coverage boundary, not fixed.

Scope

  • packages/drivers/driver-sql/src/sql-driver.ts — module-scope rawStatementFaultError(cause) beside backendStatementFaultError; protected SqlDriver.rawStatementFault(command, error) (the same "is it already ours" gate over the DECLARED status, warn-logs the statement and dialect text, composes); execute() wraps its await builder. ⛔ formatOutput, readPresentationKind, presentReadValue untouched — the diff's three hunks are at the composer (after backendStatementFaultError) and at the terminal plus execute() (the "Raw Execution" section), and git diff origin/main...HEAD on the file has 0 lines naming those three symbols. Serial and independent of PR feat(driver-sql): the read door presents datetime values and audit stamps as canonical ISO-Z text on every dialect (#13973, B1 narrow) #16619 as the dispatch ruled.
  • packages/drivers/driver-turso/src/turso-driver.ts — the remote branch of execute() calls the base terminal. PR TursoDriverConfig.timeout has no seam on the WebSocket transport (wss:// / ws://) in @libsql/client 0.17.4 — documented as unbounded, not delivered #16378 has since merged; its hunks and this one (:1407) were disjoint and merged without conflict.
  • packages/types/src/error-leak.ts — documentation only: the module note no longer invites growth, and the ruling is recorded on DIALECT_LEAK_PHRASINGS.
  • packages/services/service-analytics/src/analytics-service.ts — documentation only (review item ③): the isMissingSourceError docblock and the queryDataset degrade comment now state that the degrade is reached by a BARE error only, that a driver-raised fault declares and is re-thrown before the question is asked ([finding] queryDataset 里还有第二个 message 嗅探器 isMissingSourceError,命中即静默返回空结果 —— dataset-compiler 的一条拒收措辞已经命中它,只因抛点在 try 之外才没出事 #5717 defence B), and which producers still reach it. No code moved; the three degrade pins (dataset-degradation-envelope, missing-column-phrase-hard-failure, dataset-cross-datasource-registration) run green: 3 files, 35 tests.
  • packages/drivers/driver-sql/src/sql-driver-keyed-text-mysql.test.ts (CI round) — the live-MySQL prefix-unique measurement reads the rejection off the declared envelope's cause instead of error.message; the measurement itself is unchanged.
  • Tests: packages/drivers/driver-sql/src/sql-driver-16019-raw-statement-fault-envelope.test.ts (a real better-sqlite3 driver), packages/drivers/driver-turso/src/turso-driver-16019-remote-raw-statement-fault-envelope.test.ts (remote mode over the libsql stub), packages/rest/src/analytics-16019-driver-declared-fault.test.ts (a real AnalyticsService and a real SqlDriver behind the plugin's own bridge, a dataset dimension whose expression calls translate(), through POST /analytics/dataset/query; the door-ordering block: declared wins, the ORDERING pin, the fallback stays, the bare undeclared residual pinned as the boundary), packages/rest/src/package-door-16019-raw-statement-fault-code.test.ts (review item ①, the door half), [#16019] blocks in packages/services/service-package/src/publish-driver-fault.test.ts and delete-driver-fault.test.ts (review item ①, the catch half), packages/types/src/error-leak.test.ts (the frozen-list block).
  • .changeset/driver-raw-statement-declared-fault.md@objectstack/driver-sql patch, @objectstack/driver-turso patch; its wire section now names all three doors.

What moves on the wire, named — three doors

Semver

patch, on AGENTS.md's changeset rule (Post-Task Checklist item 3: "A bug fix in a released package takes a patch changeset — never none"; breaking is defined there as removing or renaming 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, PR #9273 (cbf4b40b05), which shipped "@objectstack/driver-sql": patch. check-changeset-no-major.mjs --base origin/main exit 0. (Review item ⑥: the "lane's row" I cited before is a dispatch-seat convention, not a repo document; withdrawn.)

Verification — on a5acd49733 (merge of origin/main 4fe00b80e7); driver ablation on 7569f2a7a5, ordering ablation on a5acd49733

Every exit captured after a redirect; where a run was locked, the wrapper's own VERDICT line was read.

Reverse verification

Driver terminal — both legs on 7569f2a7a5, predictions written before running. Predicted: driver-sql pin 5 red / 2 green; door pin 2 red / 4 green (block 1 cases 1 and 2 — code returning ANALYTICS_QUERY_FAILED, and the absent driver log line — with its INTERNAL_ERROR_MESSAGE half green by the re-scope's accident; block 2 green throughout); turso 4 green. Leg A, mutate: execute() restored to the bare const result = await builder; (composer and terminal left in place). On-disk proof: injected spelling count 1, removed throw this.rawStatementFault(command, error) count 0, git diff --stat 1 file, +1 −7. Rebuilt driver-sql (exit 0); ablation-dist-preflight.mjs @objectstack/driver-sql 'this.rawStatementFault(command, error)' --absent → "✓ dist/: marker absent from all 6 built files". Observed: driver-sql Tests 5 failed | 2 passed (7), rest Tests 2 failed | 4 passed (6) (first red expected 'ANALYTICS_QUERY_FAILED' to be 'DATABASE_ERROR', second red the absent driver log line), turso 4 passed (4) — no unpredicted direction. Leg B, restore: git checkout HEAD -- ABS_PATH; git rev-parse HEAD:packages/drivers/driver-sql/src/sql-driver.ts = c31eb97dc6e18aac52d0f115f5c4c6efe7cb002b = git hash-object of the file on disk (neither empty); porcelain empty; whole-tree git diff HEAD --stat empty. Rebuilt (exit 0); preflight without --absent → "✓ dist/: marker present in 2 built files". Re-run: driver-sql 7, rest 6, turso 4 passed. Resolver: rest and turso consume @objectstack/driver-sql through exportsdist/ (both pairs in KNOWN_UNALIASED_TEST_IMPORTS), hence rebuild + preflight per leg.

Ordering pin (review item ④) — both legs on a5acd49733, prediction written before running: exactly 1 red (the ORDERING pin, ANALYTICS_QUERY_FAILED in place of the producer's code), 6 green — including the neighbouring "phrase the heuristic does not know" case, which is precisely why it could not stand in for this one. Leg A: rest-server.ts's ③a relay gated behind the heuristic being false (const declaredFault = looksLikeInternalErrorLeak(msg) ? undefined : declaredServerFaultAnswer(error)), i.e. heuristic consulted first; on-disk proof: marker count 1, original line count 0, git diff --stat +1 −1; rest resolves ./rest-server from source, so no build is involved. Observed: Tests 1 failed | 6 passed (7), the one red expected 'ANALYTICS_QUERY_FAILED' to be 'DATABASE_ERROR' on the ORDERING pin — as predicted. Leg B: git checkout HEAD -- ABS_PATH; HEAD blob 53ec32739e6b5812fd3b36241e4e742f6e62d522 = disk blob; whole-tree porcelain empty; marker count 0; re-run 7 passed (7).

The package-door pins (item ①) are behaviour pins on catches this PR does not edit: their "AFTER" cases hand each catch the shape the driver now raises and assert identity on the re-throw and 500 DATABASE_ERROR at the door; their "BEFORE" controls hand the undeclared ancestor and assert the swallowing branch and PACKAGE_*_FAILED. They pass on origin/main as well, by construction — the flip is a property of the fault's shape, which this PR changes at the driver, and the driver ablation above is what makes those doors' AFTER cases the driver's to answer.

The live-MySQL pin (CI round) discriminates on its own two runs: the pre-fix assertion (toThrow(/Duplicate entry/i) on error.message) was the one red against the declared envelope, and the re-aimed assertion is green — while the driver ablation above is what turns its cause read red (no envelope, no cause).

验收备注 (acceptance notes)

  • filed as Raw-exec consumers that surface error.message as an operator-facing detail now read the composed DATABASE_ERROR sentence — read cause there (follow-up to #16019) #16657 (review item ⑤): raw-exec consumers that surface error.message as an operator-facing detail now read the composed sentence — runtime-index-preflight (:298, :337), seed-tenancy-backfill (:1184, :1259, :1305, :1388, :1414, :1447), partial-index-probe (:37, :359, whose comments now say the opposite of what happens), cli db clean (:115), and the four migration result records in packages/metadata/src/migrations/* (three swallow-only sites the card first listed — overlay-index.ts, sys-setting-identity-index.ts, protocol.ts — were dropped after the delta review: they store nothing). Remedy on the card: read cause there; correct the two comments.
  • noted, not filed: RemoteTransport.find (driver-turso) keeps its own message sniff (no such column → projection retry) — pre-existing, on this.client.execute directly, not on the changed path; untouched (the review: CORRECTLY DEFERRED).
  • security label deliberately NOT applied — triage ruled twice that the leaked text is a function name we emitted, not user data or physical names.
  • Reported, not implemented: whether service-package should keep PACKAGE_PUBLISH_FAILED / PACKAGE_DELETE_FAILED for a declared driver fault (a design change beyond the ruling), and the analytics degrade question (open question 3 of the report), for the PM.

…e Turso remote transport

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…alytics door; changeset

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ADLdAs2pVcH17h9tZKWMBg
…g 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
@github-actions github-actions Bot added the size/l label Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/driver-sql, @objectstack/driver-turso, @objectstack/service-analytics, @objectstack/types, touching 5 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/types/src/error-leak.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/data-modeling/drivers.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/data-modeling/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/permissions/tenant-audit-census.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/plugins/packages.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/index.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/kernel/lifecycle.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/protocol/objectql/query-syntax.mdx (via SqlDriver (symbol, a top-level class))

3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx (via queryDataset (symbol, a method of class AnalyticsService))
  • content/docs/releases/v17.mdx (via SqlDriver (symbol, a top-level class))
  • content/docs/releases/v9.mdx (via queryDataset (symbol, a method of class AnalyticsService))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/types/src/error-leak.ts) — pages documenting those are invisible to this run
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 17 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json c8e5ac645f65dbaed0e44d8e40644f28a156a022packageMentionDocs.

Which tree this was computed on

This run read content/docs from ac95e4049b068a1c309336e5d3d8060bceac9472 — the merge of head 61b85c009b27083e808ba77dc326343198e22c1f into base c8e5ac645f65dbaed0e44d8e40644f28a156a022, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ac95e4049b068a1c309336e5d3d8060bceac9472 && git checkout ac95e4049b068a1c309336e5d3d8060bceac9472
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c8e5ac645f65dbaed0e44d8e40644f28a156a022 61b85c009b27083e808ba77dc326343198e22c1f && git checkout -B drift-repro c8e5ac645f65dbaed0e44d8e40644f28a156a022 && git merge --no-ff 61b85c009b27083e808ba77dc326343198e22c1f

node scripts/docs-audit/affected-docs.mjs --json c8e5ac645f65dbaed0e44d8e40644f28a156a022

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs c8e5ac645f65dbaed0e44d8e40644f28a156a022 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 7, 2026
…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
…he 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
…on 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
@os-musk
os-musk marked this pull request as ready for review September 7, 2026 20:46
@os-musk
os-musk enabled auto-merge September 7, 2026 20:46
@os-musk
os-musk added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 001a83b Sep 7, 2026
41 checks passed
@os-musk
os-musk deleted the claude/issue-16019-driver-declared-fault branch September 7, 2026 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

2 participants