Skip to content

fix(driver-sql): a dialect error the driver cannot attribute leaves the read exits as an ADR-0112 backend-fault envelope (#8931) - #9273

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-8931-backend-fault-envelope
Aug 17, 2026
Merged

os-project-manager merged 1 commit into
mainfrom
claude/issue-8931-backend-fault-envelope

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

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, never INVALID_FILTER, and it says nothing about the filter. Measured, and this is why:

dotted WHERE key    42P01  missing FROM-clause entry for table "title"   routine=errorMissingRTE
table not created   42P01  relation "no_such_object" does not exist      routine=parserOpenTable

Same SQLSTATE. isMissingTableError returns true for both (verified live, on the real predicate). An INVALID_FILTER here 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. isUnresolvableColumnError and isMissingTableError are 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 the else of findRows' recovery ladder and the tail of count.

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

condition pg before sqlite before after
table never provisioned 42P01, no status SQLITE_ERROR, no status DATABASE_ERROR / 500
a dotted WHERE key 42P01, no status (refused since #8790) DATABASE_ERROR / 500
comparand-shape syntax fault 42601, no status SQLITE_ERROR, no status DATABASE_ERROR / 500
value rejected by column type 22P02, no status, value inlined (coerced, no error) DATABASE_ERROR / 500
connection / pool / timeout / ACL the dialect's own, no status same DATABASE_ERROR / 500

Everything else on the read path is untouched: the #8790 INVALID_FILTER refusal still wins where it applies, and the #3821 projection / ORDER-BY recoveries still return rows.

⛔ Write paths are deliberately NOT in scope. create/update/delete faults are classified at the REST boundary from their message text — a unique violation becomes 409 UNIQUE_VIOLATION, an unknown column 400 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 22P02 diagnostic, downstream of everything knex parameterised:

select * from "t" where "rank" = $1 - invalid input syntax for type integer: "zz-not-a-number"

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 redactStatementFromMessage is 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

isMissingTableError is how 13 non-test call sites across metadata, objectql and metadata-protocol tell "the table was never provisioned" — a benign emptiness — from a failure that must stay loud: seedAutonumber returns 0 rather than restarting a counter against a full table, resolveFileReferences fails 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 follows cause four levels deep, with its own pins (schema-sync-errors.test.ts, "follows an error wrapped as cause") — so the wrap is transparent to it, and the predicate itself is untouched. Verified live: isMissingTableError(wrapped) is true on both pg and sqlite, for both find and count. Non-enumerable so the statement cannot ride back out through JSON.stringify(err) or a spread.

Downstream consequences, measured at the boundary

Run through the real mapDataError with the exact recorded driver errors:

route before after
pg dotted key (this card) 500 DATABASE_ERROR 500 DATABASE_ERROR
pg 22P02 / 42601 500 DATABASE_ERROR 500 DATABASE_ERROR
read on a registered object whose table was never created 404 OBJECT_NOT_FOUND 500 DATABASE_ERROR

The card's own route comes out unchanged on the wire — mapDataError already derived 500 + DATABASE_ERROR for 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 to mapDataError and 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 asserted rejects.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 off cause.

Verification

All at fc3d82286, tree clean, gate union re-run at that head.

  • Live PG 16.13 provisioned in-container (initdb + pg_ctl, port 54931, timezone=Asia/Shanghai, process TZ=America/New_York — the three-way zone skew the matrix's own non-vacuity guard demands), torn down afterwards.
  • pnpm --filter @objectstack/driver-sql test with 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.
  • Downstream sweep (dependents of driver-sql — the prefix direction, i.e. packages that consume it): driver-turso 1003, driver-sqlite-wasm 395, rest 2011, runtime 2464, dogfood 780, service-analytics 1722, service-automation 967, service-datasource 478, service-messaging 242, cli 1367, plugin-security 1279, plugin-auth 1261, plugin-sharing 617, plugin-approvals 496, plugin-reports 70, trigger-record-change 78 — all green.
  • Gates, re-derived with node scripts/pm/dispatch-gates.mjs against 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-triggered check:query-options-erasure, check:type-check-coverage, check:type-check-debt, check:engine-double-contract, check:where-matcher, and the implicated check: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:

ablation predicted observed
revert findRows' catch-all to throw error broad red, count half staying green 19 failed / 53 passed, including "find() and count() answer an unclassified fault the SAME way"
drop the cause narrow red on the two cause pins only 5 failed / 67 passed
make the cause enumerable red on the serialisation pins only 2 failed / 70 passed

The cause ablation was also run against runtime's consumer, which resolves @objectstack/driver-sql through exports to dist/ with no vitest alias. The first attempt was a stale-dist false green; the leg was redone with pnpm --filter @objectstack/driver-sql build and node scripts/ablation-dist-preflight.mjs … --absent proving 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

redactStatementFromMessage is not called. The ruling asked for it to be reused. Two measured reasons it cannot serve this envelope, both worth a reviewer's judgment:

  1. Layering. It lives in @objectstack/objectql. driver-sql depends on core/observability/spec/types only, 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.
  2. It would not satisfy the disclosure clause anyway. It cuts at the last - and keeps the dialect's diagnostic tail. On the card's own route that tail is missing FROM-clause entry for table "title" — a physical table name in a quoted reference, which fence 3 forbids — and on the 22P02 route it is invalid 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 unresolvableFilterColumnRefusal one method over already writes.

No semantic-migration entry. ADR-0087 D3 entries are structured TODOs for metadata source files (replacement is "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-registration passes and the changeset carries the not-required marker 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 answers 500 DATABASE_ERROR like 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

… 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
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/driver-sql, touching 6 documentable anchor(s).

5 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))
  • content/docs/plugins/packages.mdx (via SqlDriver (symbol))
  • content/docs/protocol/kernel/index.mdx (via SqlDriver (symbol))
  • content/docs/protocol/kernel/lifecycle.mdx (via SqlDriver (symbol))
  • content/docs/protocol/objectql/query-syntax.mdx (via SqlDriver (symbol))

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

  • content/docs/releases/v17.mdx (via SqlDriver (symbol))

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

Coarse fallback — 9 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 origin/main → packageMentionDocs.

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 origin/main → pass the list as args.docs.

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/l tests tooling

Projects

None yet

1 participant