|
| 1 | +--- |
| 2 | +"@objectstack/driver-sql": patch |
| 3 | +--- |
| 4 | + |
| 5 | +fix(driver-sql): a dialect error the driver cannot attribute leaves the read exits as an ADR-0112 backend-fault envelope instead of raw (#8931) |
| 6 | + |
| 7 | +<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable is |
| 8 | +added, renamed, retired or tombstoned — no metadata key, no spec surface, no |
| 9 | +declaration an author writes. The change is entirely in what a failing READ |
| 10 | +throws: an error that already declared no `status` and carried the compiled |
| 11 | +statement now declares `DATABASE_ERROR` / 500 and does not. There is no source |
| 12 | +file for a consumer to migrate and therefore no semantic-migration TODO to |
| 13 | +emit; the accept set is unchanged, since every condition below failed before |
| 14 | +this change and fails after it. --> |
| 15 | + |
| 16 | +`SqlDriver.find()` / `findOne()` / `count()` had one exit that answered with the |
| 17 | +**database's own error object**: a `code` from the backend's vocabulary |
| 18 | +(`42P01`, `SQLITE_ERROR`, `42601`, `22P02`, …), **no `status`** at all, and a |
| 19 | +message opening with the compiled statement. Two things travelled out of it that |
| 20 | +should not have — the statement's shape, and on one measured row the caller's |
| 21 | +own value. |
| 22 | + |
| 23 | +Ruled 2026-08-17 on #8931: the driver stops answering an unenveloped dialect |
| 24 | +error. Any dialect error the existing classification does not claim now leaves |
| 25 | +as a **generic backend-fault envelope**, `DATABASE_ERROR` / 500, asserting only |
| 26 | +*"the backend rejected this statement"*. |
| 27 | + |
| 28 | +**Not a filter verdict, and that is the ruling rather than a preference.** |
| 29 | +Measured live on PostgreSQL 16.13, a dotted WHERE key and a table that was never |
| 30 | +created raise the *same* SQLSTATE: |
| 31 | + |
| 32 | +``` |
| 33 | +dotted key 42P01 missing FROM-clause entry for table "title" |
| 34 | +table not created 42P01 relation "no_such_object" does not exist |
| 35 | +``` |
| 36 | + |
| 37 | +An `INVALID_FILTER` here would tell an operator whose schema sync had not run |
| 38 | +that their *filter* was wrong. The signal cannot support the claim, so the |
| 39 | +envelope does not make it — and the driver still never inspects the caller's key |
| 40 | +for a `.` (that verdict is #8371's, and it landed there). |
| 41 | + |
| 42 | +**Mechanism: a terminal catch-all, not a new recognizer.** No predicate learns |
| 43 | +`42P01`. `isUnresolvableColumnError` and `isMissingTableError` are untouched, so |
| 44 | +the #8790 refusal (`INVALID_FILTER` / 400 naming the column) still wins wherever |
| 45 | +it applies, and the #3821 projection / ORDER-BY recoveries still return rows. |
| 46 | + |
| 47 | +**What now takes the envelope**, measured on live PG 16.13 and better-sqlite3: |
| 48 | +a table that was never provisioned; a dotted WHERE key on Postgres; a |
| 49 | +comparand-shape syntax fault; a value the column type rejects; and connection, |
| 50 | +pool-acquisition, timeout or permission failures. |
| 51 | + |
| 52 | +**The disclosure this closes on a route nobody had named.** Postgres puts the |
| 53 | +caller's rejected VALUE in its own `22P02` diagnostic (`invalid input syntax for |
| 54 | +type integer: "…"`), *downstream* of everything knex parameterised — so no |
| 55 | +statement cut removes it. Withholding the dialect text whole is what closes it. |
| 56 | +(#8931's headline premise, a bound literal inlined on the *dotted* route, was |
| 57 | +measured false and pinned by #9108; this is the neighbouring row where a value |
| 58 | +really does travel.) |
| 59 | + |
| 60 | +**The original error is kept as a non-enumerable `cause`.** That is load-bearing, |
| 61 | +not tidiness: `isMissingTableError` follows `cause`, and thirteen read paths use |
| 62 | +it to tell "the table was never provisioned" — a benign emptiness — from a |
| 63 | +failure that must stay loud. Non-enumerable so the statement cannot ride back |
| 64 | +out through `JSON.stringify(err)` or a spread. |
| 65 | + |
| 66 | +**For callers.** At the REST boundary the wire answer for these conditions is |
| 67 | +materially unchanged — `mapDataError` already derived `500` + `DATABASE_ERROR` |
| 68 | +for them by sniffing the message; it is now *declared* by the producer that |
| 69 | +knows, per ADR-0112, and every non-REST consumer (an in-process ObjectQL caller, |
| 70 | +a plugin, an AI-authored action) gets the same declared answer instead of having |
| 71 | +to pattern-match a SQLSTATE that differs per backend. Two consequences worth |
| 72 | +naming: code that matched on the raw dialect `code` or message of a failing |
| 73 | +**read** must read `error.cause` instead; and a read against a **registered |
| 74 | +object whose table was never created** now answers `500 DATABASE_ERROR` where it |
| 75 | +previously answered `404 OBJECT_NOT_FOUND` with the body `Object 'x' is not |
| 76 | +registered` — a sentence that was false in exactly that state. |
0 commit comments