diff --git a/.changeset/27508-hook-abort-not-unhandled.md b/.changeset/27508-hook-abort-not-unhandled.md new file mode 100644 index 00000000000..efc9a41fabd --- /dev/null +++ b/.changeset/27508-hook-abort-not-unhandled.md @@ -0,0 +1,12 @@ +--- +'@objectstack/rest': patch +--- + +fix(rest): a sandboxed hook's code-less `onError: 'abort'` refusal is no longer logged as `[REST] Unhandled error` + +Clause-②: no +Metadata-protocol: none + +A hook that aborts its write with a business `throw` (`onError: 'abort'`, the default) is a refusal the route already answered: the data door replies with the hook's own sentence at 400. The server still printed `[REST] Unhandled error: SandboxError` and a stack for every such abort, beside the BodyRunner's own `sandboxed hook refused` line, so operators reading the error log could not tell an intended abort from a crash. + +The route log now recognises that refusal by the sandbox marker every door already reads (the body's unwrapped sentence, which a crash never has) on a 4xx answer from a body that threw no code, and stays quiet for it. Nothing changes on the wire: status, body and envelope are byte-identical. A sandbox crash, a plain un-coded 4xx from any other producer, and a sandboxed body that threw a code (judged as a coded refusal, as before) are still logged. diff --git a/packages/qa/dogfood/test/object-hook-lifecycle-scratch.dogfood.test.ts b/packages/qa/dogfood/test/object-hook-lifecycle-scratch.dogfood.test.ts index 5906b32cd04..f221c7c33a3 100644 --- a/packages/qa/dogfood/test/object-hook-lifecycle-scratch.dogfood.test.ts +++ b/packages/qa/dogfood/test/object-hook-lifecycle-scratch.dogfood.test.ts @@ -119,9 +119,20 @@ describe('records-forms.object-hook-lifecycle — the scratch hook variants over it('acceptance[6], negative[0]: a throwing beforeInsert under abort refuses the POST and lands no row', async () => { expect(await stack.rows(ABORT, { name: 'ohl-abort' })).toHaveLength(0); - const res = await call('POST', `/data/${ABORT}`, { name: 'ohl-abort' }); + const log = captureLog(); + let res: Answer; + try { + res = await call('POST', `/data/${ABORT}`, { name: 'ohl-abort' }); + } finally { + log.restore(); + } expect(res.status, res.text).toBe(400); expect(res.text).toContain(ABORT_MSG); + // [#27508] The abort is a refusal the route answered: the wire answer is + // the hook's own sentence, code-less, exactly as before, and the server + // does not log it as an unhandled error. + expect(res.json).toEqual({ error: ABORT_MSG, object: ABORT }); + expect(log.matching('[REST] Unhandled error')).toHaveLength(0); expect(await stack.rows(ABORT, { name: 'ohl-abort' })).toHaveLength(0); }); diff --git a/packages/rest/src/error-response.ts b/packages/rest/src/error-response.ts index 77862fd5460..10b6edec5c0 100644 --- a/packages/rest/src/error-response.ts +++ b/packages/rest/src/error-response.ts @@ -866,9 +866,10 @@ function isExpectedQueryRejection(body: Record | undefined): bo * 500, which this predicate cannot treat as expected at all * (`isExpectedDataStatus` names 502/503 and nothing else in the 5xx band), so * the handler bug is loud STRUCTURALLY rather than by this sentence. The - * narrowness still matters for what remains in the un-coded 4xx band — the - * sandbox unwraps' business-rule 400s — and for the next author tempted to - * simplify the predicate down to a status range. + * narrowness still matters for what remains in the un-coded 4xx band — every + * producer's un-coded 4xx beside the sandbox unwraps' business-rule 400s, which + * are quiet only by their own marker read ({@link isSandboxBodyRefusal}) — and + * for the next author tempted to simplify the predicate down to a status range. * * [#4886] Every route catch now decides through this one function. Before, the * metadata family logged unconditionally — the designer's `?state=draft` probe @@ -926,9 +927,10 @@ export function isExpectedRouteError(status: number, body: Record }, +): boolean { + if (resolved.status < 400 || resolved.status >= 500) return false; + return sandboxBusinessMessage(error) !== undefined + && resolveThrownHttpError(error, resolved.status).declaredCode === undefined; +} + /** * Log "[REST] Unhandled error" only when `resolved` is a genuine fault. For * catch blocks that must emit their own response shape (the CRUD handlers that @@ -985,11 +1036,16 @@ function isDeclaredClientRefusal( * band is already the caller's — `ApiErrorSchema.refusal`'s own table says the * field is "redundant on a 4xx". Two declarations, one per band, each read off * the thrown error; neither is a list of codes or of statuses. + * + * [#27508] And a sandboxed body's un-coded business refusal — a hook's + * `onError: 'abort'` — is a refusal the route answered: + * {@link isSandboxBodyRefusal}. Log only; the wire answer is untouched. */ export function logUnexpectedRouteError(error: any, resolved: { status: number; body: Record }): void { if (!isExpectedRouteError(resolved.status, resolved.body) && declaredRefusalMessage(error) === undefined - && !isDeclaredClientRefusal(error, resolved)) { + && !isDeclaredClientRefusal(error, resolved) + && !isSandboxBodyRefusal(error, resolved)) { logError('[REST] Unhandled error:', error); return; } diff --git a/packages/rest/src/rest-declared-client-refusal-log.test.ts b/packages/rest/src/rest-declared-client-refusal-log.test.ts index f49d7942476..5940807eab9 100644 --- a/packages/rest/src/rest-declared-client-refusal-log.test.ts +++ b/packages/rest/src/rest-declared-client-refusal-log.test.ts @@ -214,16 +214,6 @@ describe('[#23932] controls that stay LOUD — the verdict is not "any 4xx"', () } }); - it('an un-coded business-rule 400 (a sandboxed hook refusal) logs', async () => { - const hookRefusal = Object.assign(new Error("hook 'guard' threw: Error: Opportunity is closed."), { - innerMessage: 'Opportunity is closed.', - }); - for (const res of await expectLoudAtBothDoors(hookRefusal)) { - expect(res.statusCode).toBe(400); - expect(res.body.code).toBeUndefined(); - } - }); - it('a declared 400 with no code (half an envelope) logs', async () => { const uncoded = Object.assign(new Error('bad input'), { status: 400 }); for (const res of await expectLoudAtBothDoors(uncoded)) { @@ -270,6 +260,60 @@ describe('[#23932] controls that stay LOUD — the verdict is not "any 4xx"', () }); }); +describe('[#27508] a hook `onError: abort` is a refusal the route answered — quiet, wire unchanged', () => { + /** + * The card's reproduction: a sandboxed `beforeInsert` hook declared + * `onError: 'abort'` throws `boom abort`. The runner raises a `SandboxError` + * (`.message` the debug wrapper, `.innerMessage` the hook's own sentence) + * and the abort policy rethrows it unchanged, so this is the exact value + * both doors catch. + */ + const hookAbort = () => Object.assign(new Error("hook 'qa_abort' threw: Error: boom abort"), { + name: 'SandboxError', + innerMessage: 'boom abort', + }); + + it('POST /data/:object answers the same code-less 400 and prints no "[REST] Unhandled error"', async () => { + const res = await postRecord(serverRejecting(hookAbort())); + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ error: 'boom abort', object: 'qa_media' }); + expect(unhandledLogs()).toHaveLength(0); + }); + + it('the handleRouteError door agrees: same answer it always gave, no unhandled line', async () => { + const res = await putMeta(serverRejecting(hookAbort())); + expect(res.statusCode).toBe(400); + expect(res.body).toEqual({ error: 'boom abort' }); + expect(unhandledLogs()).toHaveLength(0); + }); + + it('CONTROL — a genuine unexpected error at the same door is still logged, by identity', async () => { + const bug = new TypeError("Cannot read properties of undefined (reading 'name')"); + const res = await postRecord(serverRejecting(bug)); + expect(res.statusCode).toBe(500); + expect(unhandledLogs()).toHaveLength(1); + expect(unhandledLogs()[0][1]).toBe(bug); + }); + + it('CONTROL — an un-coded 400 from a producer that is NOT a sandboxed body is still logged', async () => { + const uncoded = Object.assign(new Error('bad input'), { status: 400 }); + const res = await postRecord(serverRejecting(uncoded)); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBeUndefined(); + expect(unhandledLogs()).toHaveLength(1); + }); + + it('CONTROL — a sandboxed body that threw an UNREGISTERED code is judged as a coded refusal and still logged', async () => { + const coded = Object.assign(new Error("hook 'qa_abort' threw: Error: nope"), { + name: 'SandboxError', innerMessage: 'nope', code: 'QA_NOT_A_REGISTERED_CODE', status: 400, + }); + const res = await postRecord(serverRejecting(coded)); + expect(res.statusCode).toBe(400); + expect(res.body.declaredCode).toBe('QA_NOT_A_REGISTERED_CODE'); + expect(unhandledLogs()).toHaveLength(1); + }); +}); + describe('[#23932] a control that stays QUIET — the existing expected outcomes', () => { it('VALIDATION_FAILED 400 and a structured 404 still print nothing', async () => { const validation = Object.assign(new Error('bad'), { code: 'VALIDATION_FAILED', status: 400 });