fix(rest): a crashing hook body answers the sanitised fault envelope, not a raw TypeError at 400 (#7543)

.changeset/hook-script-fault-envelope.md

@@ -0,0 +1,61 @@
+---
+"@objectstack/rest": patch
+"@objectstack/example-showcase": patch
+---
+
+fix(rest): a crashing hook body answers the sanitised fault envelope, not a raw `TypeError` at 400 (#7543)
+
+`POST /api/v1/data/showcase_task` with `{"title": 12345}` answered
+
+```
+400 { "error": "TypeError: not a function", "object": "showcase_task" }
+```
+
+— a JS runtime error as the client-facing message, in a body with no `code` at
+all. Two contract breaks in one response: an internal fault echoed verbatim to a
+caller, and an error body outside the ledgered envelope, so a client keying on
+`code` got nothing.
+
+**The seam.** `mapDataError` has two sandbox-unwrap branches, and they are the
+only ones in the file that emit `{ error, object }` with no `code` at 400. They
+exist for one shape: a hook or action body that runs
+`throw new Error('删除被阻断：仍有未结清的发票')` — an author writing a business
+rule whose message *is* the remedy, which is answered verbatim at 400 and
+deliberately without a `code`. A body that instead **crashes** arrives as a
+thrown error too, so it took the same branch and its `TypeError` went out as if
+it were that author's message.
+
+**The fix.** Both branches now separate a body that *reported* something from a
+body that *faulted*, by the thrown error's constructor name — the sandbox
+stringifies a throw as `<name>: <message>`, so a leading `TypeError:`,
+`ReferenceError:`, `RangeError:`, `SyntaxError:`, `URIError:`, `EvalError:`,
+`InternalError:` or `AggregateError:` is structural evidence of a crash rather
+than a keyword heuristic over prose. A crash answers the same sanitised
+`500 INTERNAL_ERROR` the mapper's terminal branch already gives — which is not
+new policy: that branch's own contract (#5489) names this exact case ("a plain
+handler bug (`TypeError: x is not a function`) … server faults that a caller
+cannot fix and a caller SHOULD retry"). The unwraps simply sat above it and
+intercepted the crash first.
+
+Both doors are guarded, not one. The `innerMessage` branch and the raw-message
+regex fallback produce byte-identical bodies, so classifying in only one would
+make the envelope depend on whether the `SandboxError` instance survived a
+rethrow.
+
+**Unchanged:** a deliberate refusal still reaches the caller verbatim at 400
+with no `code`. The fix changes *which* errors take that branch, not what it
+emits. A body that expresses a business rule as `throw new RangeError('…')` is
+now sanitised — an accepted cost, since that is not the documented authoring
+style and the fail-safe direction is the one that does not ship runtime faults to
+clients. The operator still gets the full text: 500 is outside
+`isExpectedDataStatus`, so `handleRouteError` logs `[REST] Unhandled error` with
+the whole error.
+
+**Showcase.** `NormalizeTaskTitleHook` guarded its trim with truthiness
+(`if (ctx.input.title)`), so the number `12345` passed the guard and had no
+`.trim`. It now checks `typeof … === 'string'`. That is the actual cause of the
+reported repro, and with it fixed the request **succeeds** rather than erroring:
+`record-validator` coerces a `text` value with `String(value)`, so a number in a
+text field breaks no declared contract. These hook bodies are read as
+documentation, so the type-safe shape is the one to show — a hook must not assume
+a field's runtime type just because its metadata declares one.

examples/app-showcase/src/data/hooks/index.ts

@@ -30,8 +30,15 @@ export const NormalizeTaskTitleHook = {
   object: 'showcase_task',
   events: ['beforeInsert', 'beforeUpdate'] as LifecycleEvent[],
   body: {
+    // [#7543] The guard is `typeof … === 'string'`, not truthiness. A JSON body
+    // may put a number in a `text` field — `{"title": 12345}` — which is truthy,
+    // has no `.trim`, and made this body throw `TypeError: not a function` on a
+    // write the platform otherwise ACCEPTS (`record-validator` coerces a `text`
+    // value with `String(value)`). These bodies are read as documentation, so
+    // the type-safe shape is the one to show: a hook must not assume a field's
+    // runtime type just because its metadata declares one.
     language: 'js' as const,
-    source: "if (ctx.input.title) ctx.input.title = ctx.input.title.trim();",
+    source: "if (typeof ctx.input.title === 'string') ctx.input.title = ctx.input.title.trim();",
   },
   priority: 50,
   onError: 'abort' as const,

packages/qa/dogfood/test/hook-error-format.dogfood.test.ts

@@ -12,8 +12,23 @@
 // and not a `code` field an older bundled @objectstack/client would prepend
 // as `[ObjectStack] CODE: …`.
 //
-// Non-default error names (`TypeError: …`) are deliberately KEPT: they mark
-// a genuine script bug rather than a thrown business rule.
+// [#7543] Non-default error names (`TypeError: …`) are the OPPOSITE case, and
+// this file used to assert they were KEPT on the wire "as useful context". They
+// are not context for the caller — they are a server-side fault, and shipping
+// one as the client-facing message of a 400 with no `code` was the defect #7543
+// reports (`POST /data/showcase_task {"title":12345}` →
+// `400 {"error":"TypeError: not a function"}`). A body that CRASHES now answers
+// the sanitised `500 INTERNAL_ERROR` that `mapDataError`'s terminal branch
+// already gives every other handler bug (#5489). The full text still reaches the
+// operator's log; only the wire is sanitised.
+//
+// The distinction this file pins end-to-end is therefore: a hook that REPORTS
+// (`throw new Error('业务规则')`) speaks to the caller verbatim at 400, and a
+// hook that FAULTS (`throw new TypeError('boom')`) does not speak to the caller
+// at all. Classification lives in `packages/rest/src/rest-server.ts`
+// (`isScriptFaultMessage`), unit-covered in
+// `packages/rest/src/rest-hook-script-fault-envelope.test.ts`; this file is the
+// only place the whole chain runs for real.
 
 import { describe, it, expect, beforeAll, afterAll } from 'vitest';
 import { bootStack, type VerifyStack } from '@objectstack/verify';
@@ -55,8 +70,11 @@ const hefStack = defineStack({
       },
     },
     {
-      // A non-default error name signals a script bug, not a business rule —
-      // the name must survive to the client as useful context.
+      // [#7543] A non-default error name signals a script bug, not a business
+      // rule — so it must NOT reach the client. This fixture is the reported
+      // defect in miniature: `showcase_normalize_task_title` crashed the same
+      // way on `{"title": 12345}` (a truthy number has no `.trim`), and the
+      // resulting `TypeError` went out as the 400's message.
       name: 'hef_buggy_guard',
       object: 'hef_base',
       events: ['beforeUpdate'],
@@ -107,12 +125,28 @@ describe('objectstack verify: sandboxed hook error message format (#hef)', () =>
     expect(r.status).toBe(200);
   });
 
-  it('non-default error names (TypeError) survive as script-bug context', async () => {
+  // [#7543] REVERSED — see the file header. This case previously asserted
+  // `400` + `body.error === 'TypeError: boom'`.
+  it('a hook body that CRASHES is sanitised, not echoed to the client', async () => {
     const r = await stack.apiAs(token, 'PATCH', `/data/hef_base/${baseId}`, { name: '改名' });
-    expect(r.status).toBe(400);
+    expect(r.status).toBe(500);
 
     const body = (await r.json()) as any;
-    expect(body.error).toBe('TypeError: boom');
-    expect(JSON.stringify(body)).not.toMatch(/threw:|hook '/);
+    // Nothing of the runtime fault survives: not the constructor name, not the
+    // thrown text, not the sandbox debug wrapper.
+    expect(JSON.stringify(body)).not.toMatch(/TypeError|boom|threw:|hook '/);
+    // …and it joins the ledgered envelope, which the old 400 did not: a client
+    // keying on `code` got nothing at all from this response.
+    expect(body.code).toBe('INTERNAL_ERROR');
+  });
+
+  it('ground truth: the crashing hook still aborted the write', async () => {
+    // The status changed; the transactional outcome must not. `onError` defaults
+    // to abort, and a sanitised envelope must not be mistaken for a soft failure
+    // that let the write through.
+    const r = await stack.apiAs(token, 'GET', `/data/hef_base/${baseId}`);
+    expect(r.status).toBe(200);
+    const body = (await r.json()) as any;
+    expect((body.record ?? body).name).toBe('华东制作基地');
   });
 });