refactor(types): one isNativeErrorName reader, so three doors cannot disagree about what a crash is

.changeset/17681-native-error-name-one-reader.md

@@ -0,0 +1,61 @@
+---
+'@objectstack/types': minor
+'@objectstack/rest': patch
+'@objectstack/objectql': patch
+'@objectstack/runtime': patch
+---
+
+refactor(types): one `isNativeErrorName` reader, so three doors cannot disagree about what a crash is (#17681)
+
+The predicate that decides whether a sandboxed body's `throw` is a business
+REFUSAL (4xx, the author's words relayed) or a CRASH (5xx, the words withheld)
+had **three byte-identical copies** — measured, one distinct 74-character regex
+literal across three packages:
+
+| copy | package | its stated reason for being a copy |
+|:--|:--|:--|
+| `isScriptFaultMessage` | `@objectstack/rest` (`error-response.ts`, #7543) | the original |
+| `isScriptCrash` | `@objectstack/objectql` (`hook-withheld-readonly-fault.ts`) | this package must not depend on `@objectstack/rest` for a regex |
+| `sandboxRefusalMessage` | `@objectstack/runtime` (`sandbox/quickjs-runner.ts`, #17265) | rest declares one export subpath and re-exports nothing from `error-response` |
+
+⭐ **Every reason is a statement about reaching `@objectstack/rest`, and none of
+them survives moving the rule.** `@objectstack/types` now owns
+`isNativeErrorName` — the name list, the `^` anchor, and the deliberate absence
+of a bare `Error:`. All three packages already depend on it and it depends on
+none of them, so this fold **adds zero dependency edges** and cannot cycle.
+
+⚠️ The hazard was never style. One copy learning a new native error name and the
+others not means the same throw is a refusal at one door and a crash at the
+next — a crash message **leaked** at one boundary and **withheld** at another.
+#16013's argument for extracting exactly this class applies verbatim: the
+classification is the part nobody may get wrong, so one *tested* helper is worth
+more than N correct copies that must each stay correct forever.
+
+⛔ **No behaviour changes at any door, per case.** This is a pure refactor and
+the three WRAPPERS are deliberately NOT folded, because they are not the same
+shape and merging them would move a door's answer:
+
+- rest asks a trimmed message and answers a boolean;
+- objectql asks **two** slots — `err.name` **or** `err.innerMessage.trim()` —
+  because a code hook and a sandboxed body carry the native name in different
+  places;
+- runtime asks the trimmed inner message and answers the **message**, not a
+  boolean.
+
+What the three share is the predicate, so the predicate is what moved. Each call
+site keeps its own slot choice and its own trimming, and `isNativeErrorName`
+deliberately does **not** trim for its callers — a contract pinned in its test.
+
+**Shipped rather than `skip-changeset`**, measured on a real build: all four
+packages publish `files[]: ["dist", …]`, and the built `dist` of each carries
+the new call — `@objectstack/types` 4 files, `@objectstack/objectql` 4,
+`@objectstack/rest` 3, `@objectstack/runtime` 2 — with `looksLikeInternalErrorLeak`
+scoring 4 in `types/dist` as the lit control and a nonexistent symbol scoring 0.
+The retired copies are gone from the artifacts too: the regex literal scores
+**0** in `rest/dist`, `objectql/dist` and `runtime/dist`, and **2** in
+`types/dist` (the ESM and CJS bundles).
+
+`@objectstack/types` takes **minor**: a new export is a purely additive widening
+of a published surface, which is at least minor whatever the commit type says.
+The three consumers take `patch` — their artifacts change, their behaviour does
+not.

packages/objectql/src/hook-withheld-readonly-fault.ts

@@ -71,18 +71,7 @@
  * borrowing it would make the error lie about which refusal happened.
  */
 
-/**
- * The ECMA-262 native error constructors, plus SpiderMonkey's `InternalError`
- * which QuickJS also raises. Same structural rule — and same deliberate
- * omission of `Error:` — that `packages/rest`'s `isScriptFaultMessage` applies
- * one door down: a body's plain `Error` is the documented way to AUTHOR a
- * refusal, so it is never a crash and its words are never rewritten here.
- *
- * ⛔ Kept as its own copy rather than imported: `@objectstack/objectql` does not
- * depend on `@objectstack/rest`, and it must not start to for a regex.
- */
-const NATIVE_ERROR_NAME_RE =
-  /^(?:Type|Reference|Range|Syntax|URI|Eval|Internal|Aggregate)Error(?::|$)/;
+import { isNativeErrorName } from '@objectstack/types';
 
 /**
  * Did the hook CRASH, as opposed to deliberately refusing?
@@ -99,12 +88,23 @@ const NATIVE_ERROR_NAME_RE =
  * A hook that threw an authored `Error` — sandboxed or not — answers `false` on
  * both, which is what keeps this from overwriting a business message that
  * `mapDataError` would otherwise serve to the caller verbatim.
+ *
+ * ⭐ The NAME LIST behind both spellings is {@link isNativeErrorName}
+ * (`@objectstack/types`, #17681) — the one reader `packages/rest`'s
+ * `isScriptFaultMessage` and `packages/runtime`'s `sandboxRefusalMessage` also
+ * call, with the same deliberate omission of a bare `Error:`. This module used
+ * to keep its own copy because the rule lived in `@objectstack/rest` and this
+ * package must not depend on rest for a regex; the shared home needs no such
+ * edge — `@objectstack/types` was already a dependency. ⛔ Do not re-inline it.
+ *
+ * The two SLOTS above stay local: which slot carries the name is this engine's
+ * own fact, and no other door has to ask both.
  */
 function isScriptCrash(err: unknown): boolean {
   if (!err || typeof err !== 'object') return false;
   const e = err as { name?: unknown; innerMessage?: unknown };
-  if (typeof e.name === 'string' && NATIVE_ERROR_NAME_RE.test(e.name)) return true;
-  return typeof e.innerMessage === 'string' && NATIVE_ERROR_NAME_RE.test(e.innerMessage.trim());
+  if (typeof e.name === 'string' && isNativeErrorName(e.name)) return true;
+  return typeof e.innerMessage === 'string' && isNativeErrorName(e.innerMessage.trim());
 }
 
 /** Read a message off anything a hook may have thrown, without assuming a shape. */

packages/rest/src/error-response.ts

@@ -56,6 +56,7 @@ import {
     demotedDeclaredCode,
     declaredUserMessage,
     declaredRefusalMessage,
+    isNativeErrorName,
     INTERNAL_ERROR_MESSAGE,
 } from '@objectstack/types';
 import type { DroppedFieldsEvent } from '@objectstack/spec/data';
@@ -207,13 +208,18 @@ const UNCLASSIFIED_FAULT = (): { status: number; body: Record<string, unknown> }
  * the sandbox REFUSING is a fault, and so is the body FAULTING — only the
  * body's deliberate `throw` is an answer addressed to the caller.
  *
- * **Matched by constructor name, not by phrasing.** These eight are the ECMA-262
- * native error constructors (plus SpiderMonkey's `InternalError`, which QuickJS
- * also raises for stack exhaustion); the sandbox stringifies a thrown error as
- * `<name>: <message>`, so the name is structural evidence rather than a keyword
- * heuristic over prose. `Error:` is deliberately absent — a plain `Error` is the
- * documented way to author a refusal, and `userFacingMessage` strips that prefix
- * upstream anyway.
+ * **Matched by constructor name, not by phrasing**, and since #17681 by the ONE
+ * reader — {@link isNativeErrorName} in `@objectstack/types`, which owns the
+ * name list, the `^` anchor and the deliberate absence of a bare `Error:`. This
+ * file held the original copy; `@objectstack/objectql` and
+ * `@objectstack/runtime` each kept their own because the rule lived HERE and
+ * neither could reach it, and all three now read the one helper. ⛔ Do not
+ * re-inline the pattern: a name learned at one door and not the others is the
+ * same throw answered as a refusal at one boundary and a crash at the next.
+ *
+ * What stays local is the TRIM — `userFacingMessage` strips a leading `Error: `
+ * upstream, so what arrives here may still be padded, and the helper
+ * deliberately does not trim for its callers.
  *
  * **Deliberate, accepted cost:** a body that expresses a business rule as
  * `throw new RangeError('数量超出范围')` now gets the sanitised 500 instead of
@@ -226,11 +232,8 @@ const UNCLASSIFIED_FAULT = (): { status: number; body: Record<string, unknown> }
  * `sendThrownError`'s `logWithheldServerFault` (#5437) covers the routes that bypass
  * it — the same operator path {@link UNCLASSIFIED_FAULT} relies on.
  */
-const NATIVE_ERROR_NAME_RE =
-    /^(?:Type|Reference|Range|Syntax|URI|Eval|Internal|Aggregate)Error(?::|$)/;
-
 function isScriptFaultMessage(message: string): boolean {
-    return NATIVE_ERROR_NAME_RE.test(message.trim());
+    return isNativeErrorName(message.trim());
 }
 
 /**

packages/runtime/src/sandbox/quickjs-runner.ts

@@ -38,7 +38,7 @@ import {
   type QuickJSDeferredPromise,
   type QuickJSHandle,
 } from 'quickjs-emscripten';
-import { resolveSandboxTimeoutMs } from '@objectstack/types';
+import { isNativeErrorName, resolveSandboxTimeoutMs } from '@objectstack/types';
 import type { HookBody, ScriptBody, ExpressionBody, HookBodyCapability } from '@objectstack/spec/data';
 import type {
   ScriptContext,
@@ -1365,28 +1365,6 @@ function hostErrorToVm(vm: QuickJSContext, err: unknown): QuickJSHandle {
  */
 const SANDBOX_FAULT_PROP = '__objectstackSandboxFault';
 
-/**
- * [#17265] The ECMA-262 native error constructors, plus SpiderMonkey's
- * `InternalError` which QuickJS also raises — the third copy of one pattern,
- * and deliberately a copy.
- *
- * `packages/rest`'s `isScriptFaultMessage` (`error-response.ts`) is the
- * original and `packages/objectql`'s `isScriptCrash`
- * (`hook-withheld-readonly-fault.ts`) already keeps its own, for the reason
- * stated there: the importing package must not take a dependency on
- * `@objectstack/rest` for a regex. This package's reason is one step narrower —
- * `@objectstack/runtime` DOES depend on `@objectstack/rest`, but that package
- * declares exactly one export subpath (`"."`) and re-exports nothing from
- * `error-response`, so importing the predicate would mean WIDENING rest's
- * published surface for an internal read.
- *
- * ⛔ Same deliberate omission of a bare `Error:` as both siblings: a body's
- * plain `Error` is the documented way to AUTHOR a refusal, so it is never a
- * crash.
- */
-const NATIVE_ERROR_NAME_RE =
-  /^(?:Type|Reference|Range|Syntax|URI|Eval|Internal|Aggregate)Error(?::|$)/;
-
 /**
  * [#17265] The caller-addressed BUSINESS sentence a sandboxed body threw, or
  * `undefined` when this error is not a body's deliberate refusal.
@@ -1400,11 +1378,18 @@ const NATIVE_ERROR_NAME_RE =
  *    threw this deliberately", and by {@link SandboxError}'s contract the thing
  *    a capability denial, a timeout and a marshalling failure all lack. Its
  *    absence is what keeps every #4431 case marked as a fault;
- *  - NOT a native error name (#7543). A nested body that CRASHED arrives in the
- *    identical shape carrying `TypeError: …`, which is an internal fault and
- *    not a sentence addressed to anyone. Dropping this half would turn a nested
- *    crash into a 400 and move the `an unexpected FAULT is a 500` line that
- *    `domains/actions-fault-vs-rejection.test.ts` pins.
+ *  - NOT a native error name (#7543) — {@link isNativeErrorName}
+ *    (`@objectstack/types`, #17681), the ONE reader `packages/rest`'s
+ *    `isScriptFaultMessage` and `packages/objectql`'s `isScriptCrash` also
+ *    call. A nested body that CRASHED arrives in the identical shape carrying
+ *    `TypeError: …`, which is an internal fault and not a sentence addressed to
+ *    anyone. Dropping this half would turn a nested crash into a 400 and move
+ *    the `an unexpected FAULT is a 500` line that
+ *    `domains/actions-fault-vs-rejection.test.ts` pins. ⛔ Do not re-inline the
+ *    pattern: this file used to keep the THIRD copy of it, because the rule
+ *    lived in `@objectstack/rest` and that package publishes one subpath which
+ *    re-exports nothing from `error-response` — a reason about reaching rest,
+ *    which the shared home in a package all three already depend on removes.
  *
  * ⛔ A READ of the field the runner populated, never a pattern-strip of the
  * `<kind> '<name>' threw:` wrapper off `.message` — the sibling's rule, for the
@@ -1414,7 +1399,7 @@ const NATIVE_ERROR_NAME_RE =
 function sandboxRefusalMessage(error: unknown): string | undefined {
   const inner = (error as { innerMessage?: unknown } | null | undefined)?.innerMessage;
   if (typeof inner !== 'string' || !inner) return undefined;
-  if (NATIVE_ERROR_NAME_RE.test(inner.trim())) return undefined;
+  if (isNativeErrorName(inner.trim())) return undefined;
   return inner;
 }
 

packages/types/src/index.ts

@@ -7,6 +7,16 @@ export * from './degraded-boot.js';
 export * from './email-verified.js';
 export * from './env.js';
 export * from './error-leak.js';
+// [#17681] The SIBLING question, kept deliberately separate: `error-leak.js`
+// asks "is this message a driver dump?", this asks "did the JS RUNTIME raise
+// it?" — the crash-vs-refusal half of what a door does with a sandboxed body's
+// throw. It had three copies (rest's `isScriptFaultMessage`, objectql's
+// `isScriptCrash`, runtime's `sandboxRefusalMessage`), each a copy only because
+// the rule then lived in `@objectstack/rest`; every consumer already depends on
+// this package, so adopting the predicate adds no edge. ⛔ Never merge the two
+// predicates — a driver dump and a runtime crash are withheld for different
+// reasons and at different statuses.
+export * from './native-error-name.js';
 // Seek-based pagination for batch walks — the offset alternative that neither
 // skips rows when the walk mutates as it goes, nor costs O(n²/p) (#4363).
 export * from './keyset-walk.js';

packages/types/src/native-error-name.test.ts

@@ -0,0 +1,108 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#17681] The one native-error-name reader, folded out of three copies.
+ *
+ * The cases below are the union of what the three doors relied on, pinned once
+ * so a name learned here is learned everywhere:
+ *
+ *  - §1 every name in the list answers `true`, in BOTH slots the callers hold
+ *    it against — the bare `name` a code hook throws, and the flattened
+ *    `<name>: <message>` a sandbox puts in `innerMessage`;
+ *  - §2 the omission of a plain `Error` — the load-bearing half, because a
+ *    plain `Error` is the documented way to AUTHOR a refusal and a `true` here
+ *    would withhold an author's words at every door at once;
+ *  - §3 the two regex limbs, each with the case that only it refuses;
+ *  - §4 the no-trim contract, which is what makes the fold behaviour-identical
+ *    at three call sites that disagree about trimming.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { isNativeErrorName } from './native-error-name.js';
+
+/**
+ * The seven ECMA-262 native error constructors plus SpiderMonkey's
+ * `InternalError`, which QuickJS raises for stack exhaustion.
+ *
+ * ⛔ Spelled out rather than derived from the predicate's own regex: a test
+ * that reads its subject's pattern back asserts that the pattern equals itself
+ * and would follow a name being dropped straight into a green run.
+ */
+const NATIVE_NAMES = [
+    'TypeError',
+    'ReferenceError',
+    'RangeError',
+    'SyntaxError',
+    'URIError',
+    'EvalError',
+    'InternalError',
+    'AggregateError',
+] as const;
+
+describe('§1 every native error name, in both slots', () => {
+    it.each(NATIVE_NAMES)('%s — the bare `name` slot a CODE hook carries', (name) => {
+        expect(isNativeErrorName(name)).toBe(true);
+    });
+
+    it.each(NATIVE_NAMES)('%s — the flattened `<name>: <message>` a sandbox carries', (name) => {
+        expect(isNativeErrorName(`${name}: something went wrong`)).toBe(true);
+    });
+
+    it('the list is the whole list — a silent shrink is what this count catches', () => {
+        expect(NATIVE_NAMES.length).toBe(8);
+        expect(new Set(NATIVE_NAMES).size).toBe(8);
+    });
+});
+
+describe('§2 a plain `Error` is an AUTHORED refusal, never a crash', () => {
+    it.each([
+        ['the bare name', 'Error'],
+        ['the flattened form', 'Error: Opportunity is closed.'],
+        ['an authored business sentence', 'Opportunity is closed.'],
+        ['a Chinese business sentence', '数量超出范围'],
+    ])('%s stays a refusal', (_label, text) => {
+        expect(isNativeErrorName(text)).toBe(false);
+    });
+
+    it.each([
+        ['the sandbox wrapper itself', "hook 'normalize_title' threw: TypeError: not a function"],
+        ['a platform error class', 'SandboxError: capability denied'],
+        ['a validation error class', 'ValidationFailedError: 2 fields'],
+    ])('%s is not this predicate’s subject', (_label, text) => {
+        expect(isNativeErrorName(text)).toBe(false);
+    });
+});
+
+describe('§3 the two regex limbs, each with the case only it refuses', () => {
+    it('`^` — prose that merely QUOTES a native name mid-sentence is not a crash report', () => {
+        expect(isNativeErrorName('rejected with TypeError: check the template')).toBe(false);
+        expect(isNativeErrorName('produced a TypeError in your template')).toBe(false);
+    });
+
+    it('`(?::|$)` — a longer identifier that merely STARTS with a native name is not one', () => {
+        expect(isNativeErrorName('TypeErrorish')).toBe(false);
+        expect(isNativeErrorName('RangeErrorReport: out of bounds')).toBe(false);
+        expect(isNativeErrorName('TypeErrors are common')).toBe(false);
+    });
+});
+
+describe('§4 the no-trim contract — the callers own their own trimming', () => {
+    it('leading whitespace is NOT stripped here', () => {
+        expect(isNativeErrorName('  TypeError: not a function')).toBe(false);
+        expect(isNativeErrorName('\nTypeError')).toBe(false);
+    });
+
+    it('a caller that trims first gets the answer it had before the fold', () => {
+        expect(isNativeErrorName('  TypeError: not a function'.trim())).toBe(true);
+    });
+});
+
+describe('§5 absent input', () => {
+    it.each([
+        ['empty string', ''],
+        ['undefined', undefined],
+        ['null', null],
+    ])('%s answers false rather than throwing', (_label, text) => {
+        expect(isNativeErrorName(text as string | undefined | null)).toBe(false);
+    });
+});