Uh oh!
There was an error while loading. Please reload this page.
fix(metadata-protocol): gate a batch row's errors[].message on a declared client refusal - #8569
Conversation
…ch-row-error-message
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#8502
toRowApiErrorinterpolated whatever it caught into a bulk write's per-rowerrors[].message, so a driver fault underdeleteManyDataanswered{ "code": "INTERNAL_ERROR", "message": "SQLITE_ERROR: no such table: leave_request" }on response data riding a 200 — where no HTTP boundary's 5xx withhold can reach it. Fourth sink in the family, after #8136's overlay delete, #8333'sfailed[].errorand #8442's seederrors[].message. Thecodehalf was already correct (#8441 gates it on catalog membership) and is untouched.The disclosure is row data, not schema shape
The card quotes a tidy
no such table. Driven against a realSqlDriveron better-sqlite3, the raw text is the whole failing statement:A delete leaks the
WHEREclause and its bound record id; a create on the same surface leaks the fullINSERTwith every submitted value (dup@example.comin the pin). And it multiplies:reconcileStoppedBatchandbuildRolledBackBatchResponsecopy the causal row's message onto everyNOT_ATTEMPTED/ROLLED_BACKsibling, so one leaked sentence is repeated across the batch. Every assertion here is therefore taken over the whole payload, not the failing row.The measurement that decided the card
The complete set of catches was derived from the TypeScript AST, not a source grep:
toRowApiErroris module-private with exactly three call sites —runBatchDataLoop,runUpdateManyLoop,runDeleteManyLoop— and no other reference of any kind. Those three serve five entry points (batchData+ its atomic arm, and both arms ofupdateManyData/deleteManyData).Every producer reaching them was then driven on the real stack — a real
ObjectQLover a realSqlDriver, through all three loops, with the error captured at the engine seam:codestatusstatusCoderowRequiredIdErrorrecordNotFoundErrorValidationErrorErrorSqliteError(unique / FK / no-table)Neither sibling's answer transfers whole.#8333's
status-only test blanks rows 3 and 4. #8442's disjunct (4xxstatusor theVALIDATION_FAILEDshape) reaches row 3 and still blanks row 4 — becauseplugin-approvalsbinds a globalbeforeUpdatehook whoselockedErrorspells its refusalstatusCode, a spelling this sink receives and the seed channel never did.So the rule asks the one question that covers all three declarations: would the boundary serving this throw call it a client refusal? That question already has exactly one implementation —
resolveThrownHttpErrorfrom@objectstack/types, the same resolver/api/v1/dataanswers with — and it is imported, never re-spelled. Reading only one status spelling is how that door answered 500 to a deliberate409 RECORD_LOCKEDuntil #7525; a fourth local spelling here would rebuild that divergence one layer down, so a batch row and the single-recordPATCHof the same object would disagree about the same error.declaresClientRefusalis deliberately not widened — #8333 measured its own population and that limb is left exactly as it landed.What it costs, stated plainly
A hook that refuses by throwing an undeclared
Errorno longer has its sentence echoed on the row. That is the measured cost of a positive list, not an oversight: at this sink an undeclared hook throw is indistinguishable from an undeclared driver throw, which is the whole hole. The remedy is at the producer and now has three accepted spellings — a 4xxstatus, a 4xxstatusCode, orvalidationFailure(message, fields)— so declaring is cheaper than working around it. Same reasoning #8333 applied when it fixed P9's undeclaredZodErrorat the producer rather than loosening the collector. The case is pinned rather than regretted.Also closed: the old
String(err)fallback for an empty message, which rendersError: SQLITE_ERROR: …— the same second leak path #8333 found at its P13.Verification
Positive control, real not synthesized —
packages/objectql/src/batch-row-authoring-feedback.test.tsruns the realvalidateRecordover a genuinely malformed record through the real loops: the author still readsReason must be ≤ 8 characters (got 27)on bothbatchDataandupdateManyData, and the same file measures that the refusal carries nostatusand nostatusCode— so it is quotable only because the rule reads more thanstatus.Real driver —
packages/runtime/src/batch-row-driver-text-real-driver.integration.test.tsasserts the raw text really does carrydelete from/'p1'/insert into/dup@example.com, that it is not validation-shaped and resolves to 500, and that none of it appears anywhere in the payload.Doubles are audited — metadata-protocol cannot import objectql or driver-sql (objectql depends on it), so the in-package pins use doubles whose exact own-property sets were measured from the real classes. Section 5 asserts those key lists in order and runs the production recogniser over each; it caught a real drift in my own
ValidationErrordouble on the first run (nameassigned beforecode).Both ablations, predicted before running:
protocol.tsThe two directions test different properties, and (a) alone would not have been enough: the two no-
statuspopulations are green in both arms of (a) — pre-fix quoted everything, so it quoted them too, for the wrong reason. Only (b) separates "quotes what it should" from "quotes everything".Fixture triage — 11 existing pins asserted an undeclared bare
Error's message verbatim; 10 in metadata-protocol and one inpackages/objectqlthat a package-scoped sweep would have missed (found by the downstream sweep). Nine were re-spelled with the propagation claims re-anchored to the causal row's own message so the two cannot drift. One needed replacing rather than re-spelling:upsert-existence's "a real update failure surfaces ITSELF" discriminated by message, and after the fix both the update's and the fallback insert's text are withheld — so.not.toContain('duplicate key')would have passed even if the fallback had run. Its claim is now carried structurally (the update was attempted, no insert followed), which is stronger than the string ever was.Suites — metadata-protocol 85 files / 1261 tests, objectql 201 / 3553, rest 114 / 1881, runtime 155 / 2369, all green on the merged tree. Downstream consumer sweep in the prefix direction (
--filter '...@objectstack/metadata-protocol'= the 44 packages that depend on it): all green.Gates — the union re-derived with
dispatch-gates.mjsagainst the actual changed paths and re-run after the final commit and after mergingorigin/main:check:nul-bytes,check:error-code-casing,check:engine-double-contract,check:cross-package-test-inputs,check:durability-log-level,check:filter-alias-parity,check:changeset-gate-self-tests,check:objectui-changeset,check:query-options-erasure,check-adr-0087-registration,check-changeset-no-major,check-empty-changeset,check-engine-split-ratio— all pass.check:type-check-debt(the ratchet,--self-testand--re-measure, per #8545) passes with no ledger raised: it caught two of my own defects mid-run — 2 untyped engine-option sites and a dead helper — both fixed at the source.check:objectui-pin-freshis red and pre-existing:.objectui-shais untouched by this branch and it fires identically onmain. Control-byte self-scan over all changed files: clean.Generated by Claude Code