docs(driver-sql): uncompilableAggregateFunctionError's 501 now survives to the wire (#7407) - #7480
Conversation
…es to the wire (#7407) The docblock's closing "measured consequence" paragraph recorded that `mapDataError`'s generic status passthrough on the `/data` routes was 4xx-ONLY, so this producer's declared `501` / `NOT_IMPLEMENTED` fell to `UNCLASSIFIED_FAULT`'s `500 INTERNAL_ERROR`. True when written (#5907), and #5582 was the right pointer for it. PR #7402 closed #5582: the passthrough now spans 400-599 (`packages/rest/src/rest-server.ts:846`), and the >=500 arm keeps the status and — via `declaresServerFault` — the producer's `code`. So the paragraph asserted the opposite of shipped behaviour, in the direction that costs the most: a reader measuring a real `501` on the wire had to work out which of the two was stale. Replaced with the post-#7402 reading, keeping the paragraph's purpose (don't rediscover this as a bug) pointed the new way, plus the #5582 / #7402 history so the flip is legible. Records the half that did not change: the 5xx band still drops the PROSE unconditionally, so the body is the generic `Internal server error` and the driver's carefully worded message reaches only in-process callers and the operator log — which is what the paragraph's closing ADR-0112 sentence was already about. Both halves verified against the merged code and pinned end-to-end by `packages/rest/src/rest-5xx-status-passthrough.test.ts` ("an aggregate the backend cannot compile answers 501 NOT_IMPLEMENTED on the wire"). The ADR-0112 class-2 argument and the catalog-choice rationale above it are untouched. Comment-only: no behaviour, no tests, no changeset (`skip-changeset`). Fixes #7407
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 8 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:
|
Fixes #7407
Comment-only. One paragraph of one docblock in
packages/drivers/driver-sql/src/sql-driver.ts. No behaviour, no tests, no changeset —skip-changesetroute (per the card; not creating an empty changeset).What was stale
uncompilableAggregateFunctionError's closing "measured consequence" paragraph said:True when written (#5907). PR #7402 closed #5582 and inverted it.
Verification (read, not taken on trust)
Against merged
main@36237a1:packages/rest/src/rest-server.ts:846—if (typeof error?.status === 'number' && error.status >= 400 && error.status < 600). The passthrough is 400-599, not 4xx-only.packages/rest/src/rest-server.ts:872-880— the>= 500arm returns{ status: error.status, body: { error: INTERNAL_ERROR_MESSAGE, ...(declaresServerFault(error) ? { code: error.code } : {}) } }.packages/types/src/error-leak.ts:113-117—declaresServerFaultisstatus >= 500 && typeof code === 'string' && code.length > 0. The driver throwsstatus: 501+code: 'NOT_IMPLEMENTED', so both halves hold.:846intercepts it: every earlier branch keys onDELETE_RESTRICTED/CONCURRENT_UPDATE/ERR_DATASOURCE_UNAVAILABLE/VALIDATION_FAILED/ the capability + access gates /PERMISSION_DENIED/innerMessage/OBJECT_NOT_FOUND/INVALID_FIELD.packages/rest/src/rest-5xx-status-passthrough.test.ts:406-415— "an aggregate the backend cannot compile answers 501 NOT_IMPLEMENTED on the wire", assertingres.statusCode === 501andres.body === { error: INTERNAL_ERROR_MESSAGE, code: 'NOT_IMPLEMENTED' }.So: the status and the ADR-0112 code now reach the caller. The prose does not — the 5xx arm drops the message unconditionally and substitutes
'Internal server error'. The card and the dispatch both described only the first half; the new paragraph records both, because the message-drop is the next thing a reader measuring this endpoint will notice, and it is what the paragraph's existing closing ADR-0112 sentence ("what reaches every in-process caller and the operator log") was always about.The edit
Before:
After:
The ADR-0112 class-2 argument, the #6188/ADR-0049 history, the "class is now EMPTY, kept deliberately" warning, and the catalog-choice rationale are all untouched. The issue pointers (#5582, plus #7402) are kept, now as the history that explains why the paragraph once said the opposite — so a reader measuring a real 501 learns it is correct-by-design instead of having to work out which of the two statements is stale.
Companion grep (positive control, re-run)
One hit, this file. No
driver-tursocompanion edit needed — matches triage's finding.Comment-only self-check
Emits 17 lines, every one of them
* …docblock content.git diff --stat: 1 file, +10 −7.Gates run locally
eslint packages/drivers/driver-sql/src/sql-driver.ts --no-inline-configpnpm --filter @objectstack/driver-sql typecheckturbo run build --filter=@objectstack/driver-sql^...first, since a fresh worktree has no built workspacedist/type declarationsPer the card, the rest of the gate farm is CI's; it is
in_progressat the time of writing.🤖 Generated with Claude Code
https://claude.ai/code/session_01YcMB8dGrg4WFmpH8JuSRmo
Generated by Claude Code