fix(rest): a hook's onError abort is no longer logged as an unhandled error

.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.

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);
   });
 

packages/rest/src/error-response.ts

@@ -866,9 +866,10 @@ function isExpectedQueryRejection(body: Record<string, unknown> | 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<string, unknow
  *     `resolveThrownHttpError`, the one rule every door reads (`status` or
  *     `statusCode`, or the 400 a validation-shaped throw declares by shape).
  *     A status the DOOR supplied is not a declaration: the sandbox unwrap's
- *     default 400 fails here, so the un-coded business-rule 400s the predicate
- *     above stays narrow for are still logged, and so is a registered code that
- *     arrived with no status (half an envelope).
+ *     default 400 fails here, and so does a registered code that arrived with
+ *     no status (half an envelope). The un-coded business-rule 400 a sandboxed
+ *     body's refusal answers is quiet by its OWN read instead —
+ *     {@link isSandboxBodyRefusal} — not by this one.
  *  3. **The producer's own code is the wire `code`.** That holds exactly when
  *     the spelling is a member of `StandardErrorCode` ∪ the ADR-0112 ledger —
  *     anything else is demoted to `declaredCode` beside a status-derived `code`
@@ -957,6 +959,55 @@ function isDeclaredClientRefusal(
         && resolved.body?.code === thrown.declaredCode;
 }
 
+/**
+ * [#27508] Is this answer a sandboxed body's own REFUSAL — a hook aborting the
+ * write under `onError: 'abort'` (the default) with a business `throw` — that
+ * the route delivered to the caller as the un-coded 4xx it always answers?
+ *
+ * Triage ruled the log half on its own: "an undeclared hook `onError` abort is
+ * a refusal the route answered", so it is not logged as `[REST] Unhandled
+ * error`. The code half (whether such an abort should carry a ledgered
+ * ADR-0112 code) is a separate spec-lane decision, so this changes NOTHING on
+ * the wire: status, body and envelope are exactly what
+ * {@link resolveErrorResponse} already answered.
+ *
+ * The recognition is the marker every door already reads, ⛔ not a message
+ * pattern: {@link sandboxBusinessMessage} answers the unwrapped sentence only
+ * for a sandbox ORIGIN (`innerMessage` set — the runner's own faults leave it
+ * unset) that is not a CRASH (a native-error sentence is declined). The
+ * operator keeps a line for it: the BodyRunner logs every body refusal at
+ * `info` (`sandboxed hook refused`), so the error-level line removed here was
+ * the second line for one ordinary refusal.
+ *
+ * The three conditions, each keeping the exemption off a genuine fault:
+ *
+ *  1. **The answer is in the 4xx band.** A crash is answered
+ *     `UNCLASSIFIED_FAULT`'s 500 and never reaches here; a declared 5xx keeps
+ *     its own `refusal: true` rule.
+ *  2. **A sandboxed body's business sentence** ({@link sandboxBusinessMessage}).
+ *     A plain un-coded 4xx from any other producer — a handler that set
+ *     `status: 400` and nothing else — fails here and stays loud.
+ *  3. **The body declared no code** — `declaredCode` from
+ *     `resolveThrownHttpError`, the same read {@link isDeclaredClientRefusal}
+ *     makes. A body that threw a code is judged by that predicate like every
+ *     other producer, so a code the ledger does not know, or a registered code
+ *     on a status the producer did not declare, is still logged exactly as
+ *     before. ⛔ Read off the THROWN error, not the answer: a code the door
+ *     itself adds to an undeclared abort's answer is the door's, not the
+ *     body's, and does not turn this refusal back into a logged fault.
+ *
+ * The marker carries no body KIND, so a sandboxed action's code-less business
+ * refusal at a data door is quiet too: the same refusal, the same answer.
+ */
+function isSandboxBodyRefusal(
+    error: any,
+    resolved: { status: number; body: Record<string, unknown> },
+): 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<string, unknown> }): 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;
     }

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 });