Repository navigation
fix(driver-sql): a dialect error the driver cannot attribute leaves the read exits as an ADR-0112 backend-fault envelope (#8931) - #9273
Conversation
… exits (#8931) The read exits of driver-sql had one path that answered with the database's own error object: a backend code, no status, and a message opening with the compiled statement. Ruled 2026-08-17 on #8931 — any dialect error the existing classification does not claim now leaves as a generic backend-fault envelope, DATABASE_ERROR / 500, asserting only that the backend rejected the statement. Not a filter verdict: measured live on PG 16.13, a dotted WHERE key and a table that was never created raise the same 42P01. A terminal catch-all, not a new recognizer — isUnresolvableColumnError and isMissingTableError are untouched and the caller's key is still never inspected for a dot. The original error is kept as a non-enumerable cause, which is what keeps isMissingTableError truthful for the thirteen read paths that use it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y26DJEHSBhhAQ6wwfsHNza
📓 Docs Drift CheckThis PR changes 1 package(s): 5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not seeCoarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop):
|
Fixes #8931
Implements the maintainer ruling of 2026-08-17 (「同意 C」): the driver stops answering an unenveloped dialect error at all. Any dialect error the existing classification does not claim now leaves
driver-sql's read exits as a generic backend-fault envelope —DATABASE_ERROR/ 500 — asserting only "the backend rejected this statement".All measurements below are from a live PostgreSQL 16.13 provisioned in-container (same version as the card's evidence) plus better-sqlite3. No live MySQL was provisioned; its cells are declared un-run rather than skipped silently.
The three fences, and how each is honoured
1. No filter verdict. The envelope is
DATABASE_ERROR, neverINVALID_FILTER, and it says nothing about the filter. Measured, and this is why:Same SQLSTATE.
isMissingTableErrorreturns true for both (verified live, on the real predicate). AnINVALID_FILTERhere would tell an operator whose schema sync had not run that their filter was wrong.2. No new per-error-class predicate. No predicate learns
42P01.isUnresolvableColumnErrorandisMissingTableErrorare byte-untouched; the caller's key is still never inspected for a.(#8371 owns that verdict and it landed there). The envelope comes from the exit, not from recognising anything — the two call sites are theelseoffindRows' recovery ladder and the tail ofcount.3. No statement shape in the caller-visible message. No table names, no quoted references, no
$n. Asserted as a negative set plus a positive anchor on every dialect cell.What the catch-all now envelopes — the enumeration, measured
42P01, nostatusSQLITE_ERROR, nostatusDATABASE_ERROR/ 50042P01, nostatusDATABASE_ERROR/ 50042601, nostatusSQLITE_ERROR, nostatusDATABASE_ERROR/ 50022P02, nostatus, value inlinedDATABASE_ERROR/ 500statusDATABASE_ERROR/ 500Everything else on the read path is untouched: the #8790
INVALID_FILTERrefusal still wins where it applies, and the #3821 projection / ORDER-BY recoveries still return rows.⛔ Write paths are deliberately NOT in scope.
create/update/deletefaults are classified at the REST boundary from their message text — a unique violation becomes409 UNIQUE_VIOLATION, an unknown column400 INVALID_FIELD— so composing a message over them would turn precise 4xx answers into a generic 500. The boundary is drawn in the code comment rather than left to be discovered.A disclosure this closes that nobody had named
Postgres puts the caller's rejected value in its own
22P02diagnostic, downstream of everything knex parameterised:No statement cut removes it — a redaction that keeps the diagnostic tail keeps the value with it. This is the measured reason the envelope composes its message instead of deriving one from the dialect's, and it is why
redactStatementFromMessageis not called here (see "Deviations" below). #8931's headline premise — a bound literal inlined on the dotted route — was measured false and pinned by #9108; this is the neighbouring row where a value really does travel.The fork risk, and what prevents it
isMissingTableErroris how 13 non-test call sites acrossmetadata,objectqlandmetadata-protocoltell "the table was never provisioned" — a benign emptiness — from a failure that must stay loud:seedAutonumberreturns 0 rather than restarting a counter against a full table,resolveFileReferencesfails open silently, the delete-dependents probe skips a relation. Wrapping without the original attached would have flipped all of them to their loud branch, which is a change to what the platform accepts, and expressly outside this ruling.The wrapper keeps the dialect error as a non-enumerable
cause. That predicate already followscausefour levels deep, with its own pins (schema-sync-errors.test.ts, "follows an error wrapped ascause") — so the wrap is transparent to it, and the predicate itself is untouched. Verified live:isMissingTableError(wrapped)istrueon both pg and sqlite, for bothfindandcount. Non-enumerable so the statement cannot ride back out throughJSON.stringify(err)or a spread.Downstream consequences, measured at the boundary
Run through the real
mapDataErrorwith the exact recorded driver errors:500DATABASE_ERROR500DATABASE_ERROR22P02/42601500DATABASE_ERROR500DATABASE_ERROR404OBJECT_NOT_FOUND500DATABASE_ERRORThe card's own route comes out unchanged on the wire —
mapDataErroralready derived500+DATABASE_ERRORfor it by sniffing the message; what moves is that it is now declared by the producer that knows, per ADR-0112, so every non-REST consumer gets the same answer instead of pattern-matching a SQLSTATE that differs per backend.The last row is a real change and is flagged for review rather than buried. It is a consequence of the ruled mechanism (the two 42P01s are indistinguishable without the predicate the ruling forbids). Note the body it replaces said
Object 'x' is not registered, which was false in exactly that state — the object is registered, its table is not provisioned. No test pinned the end-to-end behaviour; the unknown-object heuristic suites feed synthetic strings straight tomapDataErrorand are unaffected.One consumer test pinned the old raw escape and is updated in this PR:
runtime's #7842 ambient-vs-bare transaction integration test assertedrejects.toThrow(/Timeout acquiring a connection/i)— knex's own words reaching the caller. The discrimination it exists for is not weakened, only re-homed: it now asserts the envelope and reads the same string offcause.Verification
All at
fc3d82286, tree clean, gate union re-run at that head.initdb+pg_ctl, port 54931,timezone=Asia/Shanghai, processTZ=America/New_York— the three-way zone skew the matrix's own non-vacuity guard demands), torn down afterwards.pnpm --filter @objectstack/driver-sql testwith live PG: 103 files passed | 1 skipped, 1998 tests passed | 34 skipped (the skips are the MySQL cells, declared un-run).pnpm --filter @objectstack/driver-sql typecheck: clean.driver-turso1003,driver-sqlite-wasm395,rest2011,runtime2464,dogfood780,service-analytics1722,service-automation967,service-datasource478,service-messaging242,cli1367,plugin-security1279,plugin-auth1261,plugin-sharing617,plugin-approvals496,plugin-reports70,trigger-record-change78 — all green.node scripts/pm/dispatch-gates.mjsagainst the actual changed paths, all PASS:check:changeset-gate-self-tests,check:cross-package-test-inputs,check:objectui-changeset,check:test-source-alias,check:type-source-resolution,check-adr-0087-registration,check-changeset-no-major,check-empty-changeset,check-cross-package-test-inputs,docs-audit/check-affected-docs, plus the convention-triggeredcheck:query-options-erasure,check:type-check-coverage,check:type-check-debt,check:engine-double-contract,check:where-matcher, and the implicatedcheck:nul-bytes,check:error-code-casing,check:error-status-conformance,check:driver-conformance,check:route-envelope,check:driver-memory-census,check:durability-log-level.Reverse verification — direction predicted before each run, all three as predicted:
findRows' catch-all tothrow errorcounthalf staying greencausecauseenumerableThe
causeablation was also run againstruntime's consumer, which resolves@objectstack/driver-sqlthroughexportstodist/with no vitest alias. The first attempt was a stale-dist false green; the leg was redone withpnpm --filter @objectstack/driver-sql buildandnode scripts/ablation-dist-preflight.mjs … --absentproving the mutation reached the artifact, after which it went red as predicted (expected 'undefined' to match /Timeout acquiring a connection/i). Source restored and dist rebuilt, with the preflight re-run in the present direction to prove the mutation is gone from the artifact.Deviations from the dispatch, stated rather than hidden
redactStatementFromMessageis not called. The ruling asked for it to be reused. Two measured reasons it cannot serve this envelope, both worth a reviewer's judgment:@objectstack/objectql.driver-sqldepends oncore/observability/spec/typesonly, and no driver in this repo depends on the engine. Importing it would invert the layering; moving it is a cross-package refactor this card does not authorise.-and keeps the dialect's diagnostic tail. On the card's own route that tail ismissing FROM-clause entry for table "title"— a physical table name in a quoted reference, which fence 3 forbids — and on the22P02route it isinvalid input syntax for type integer: "zz-not-a-number", i.e. it would ship the caller's value.⛔ No second redaction vocabulary is minted: the caller-visible message is composed, so there is nothing to redact, and the server-log line is the dialect's text verbatim — the same shape
unresolvableFilterColumnRefusalone method over already writes.No semantic-migration entry. ADR-0087 D3 entries are structured TODOs for metadata source files (
replacementis "the canonical replacement the author should move to"). Nothing authorable changes here and there is no source for a consumer to migrate, so there is no TODO to emit;check:adr-0087-registrationpasses and the changeset carries thenot-requiredmarker with that reasoning. Flagged in case a reviewer reads the #8790 precedent more widely.A retry hint is not expressible under the ruled shape. A pool-acquisition timeout is transient and would ideally answer
503 SERVICE_UNAVAILABLE; distinguishing it needs a per-error-class predicate, which fence 2 forbids. It answers500 DATABASE_ERRORlike every other unattributable fault. Recorded as a known limitation of the one-envelope shape, not worked around.Generated by Claude Code
Generated by Claude Code