diff --git a/.changeset/27672-hook-aborted-thrown-doors.md b/.changeset/27672-hook-aborted-thrown-doors.md new file mode 100644 index 00000000000..07338ac129f --- /dev/null +++ b/.changeset/27672-hook-aborted-thrown-doors.md @@ -0,0 +1,17 @@ +--- +'@objectstack/types': patch +'@objectstack/rest': patch +'@objectstack/runtime': patch +--- + +fix(types): a hook that aborts with free text answers `HOOK_ABORTED` on package publish and the runtime `/actions` doors too, not `VALIDATION_ERROR` + +Clause-②: no +Metadata-protocol: none + +- **Before.** Since #27597 a sandboxed body that aborts with free text (`throw new Error('…')`, no `code`) answers `/api/v1/data` with `HOOK_ABORTED`. The doors that take their code from `resolveThrownHttpError` (`@objectstack/types`) instead of from the `/data` classification still answered the same abort with the code its `400` derives, `VALIDATION_ERROR`, so a client branching on the code had to know which door the abort came through. +- **Now.** `resolveThrownHttpError` answers `UNDECLARED_HOOK_REFUSAL_CODE` (`HOOK_ABORTED`) for a sandboxed body's refusal that declared no code, at a 4xx status. It is the one constant `/data` already reads, now declared beside the resolver. The value moves, from `VALIDATION_ERROR` to `HOOK_ABORTED`, on these doors: + - `POST /api/v1/packages/publish` (`@objectstack/rest`); + - the runtime `/actions` doors: `POST /api/v1/actions/:object/:action`, `POST /api/v1/actions/:object/:action/:recordId`, `POST /api/v1/actions/global/:action` and `POST /api/v1/actions//:action`, for an action body's own free-text throw, a hook abort relayed through an action body, and an action body's unreadable rejection (`throw ''`, `throw Object.create(null)`) (`@objectstack/runtime`). +- **Unchanged.** A code the body declared is kept: a registered code is still its own wire `code`, and an unregistered one (a `refusals` catalogue entry) still rides `declaredCode` beside the status-derived `code`. `declaredCode` stays absent for an undeclared abort. A producer that is not a sandboxed body still derives its code from the status, a crashed body is still the sanitised `500 INTERNAL_ERROR`, and a sandboxed refusal resolved at a 5xx keeps the fault's code. `/api/v1/data` answers exactly as before. +- **Moved, not added.** No error code is added; `HOOK_ABORTED` is the one #27597 registered. `sandboxBusinessMessage` moved to its own module so the resolver can read it without an import cycle; `@objectstack/types` exports the same names as before. diff --git a/docs/qa/platform-checklist/areas/api-backend.json b/docs/qa/platform-checklist/areas/api-backend.json index 12deeacfee6..dc81fac40b8 100644 --- a/docs/qa/platform-checklist/areas/api-backend.json +++ b/docs/qa/platform-checklist/areas/api-backend.json @@ -3307,7 +3307,7 @@ "ref": "packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts + packages/rest/src/rest-hook-script-fault-envelope.test.ts + packages/runtime/src/dispatcher-unique-violation-wire-code.test.ts + packages/services/service-storage/src/storage-routes.server-fault-withheld.test.ts (acceptance[0], unit and engine) + packages/qa/dogfood/test/error-sentences-live.dogfood.test.ts (acceptance[1]–[3] over HTTP on the scratch app packages/qa/dogfood/test/fixtures/error-sentences-fixture.ts: the hook's sentence at 400 with no code on the /data doors, 409 UNIQUE_VIOLATION with details exactly developerMessage + object at the action door, and a storage route's 500 INTERNAL withholding the caught text)" }, "source": [ - "packages/types/src/data-error-classification.ts#sandboxBusinessMessage", + "packages/types/src/sandbox-business-message.ts#sandboxBusinessMessage", "packages/types/src/data-error-classification.ts#isSandboxOwnFault", "packages/types/src/response-envelope.ts#sendServerFault", "packages/runtime/src/http-dispatcher.ts", diff --git a/packages/rest/src/package-door-undeclared-hook-abort-code.test.ts b/packages/rest/src/package-door-undeclared-hook-abort-code.test.ts new file mode 100644 index 00000000000..cfaa3948f3f --- /dev/null +++ b/packages/rest/src/package-door-undeclared-hook-abort-code.test.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27672] `POST /api/v1/packages/publish` answers a sandboxed hook's + * undeclared abort with `HOOK_ABORTED`, the code `/api/v1/data` has answered + * since #27597, instead of the `VALIDATION_ERROR` its 400 derives. + * + * This door does not answer from the `/data` classification: `sendThrownError` + * (`package-routes.ts`) takes the status from it and the CODE from + * `resolveThrownHttpError` (`@objectstack/types`). The resolver now answers + * `UNDECLARED_HOOK_REFUSAL_CODE` for a sandboxed refusal that declared no code, + * so this door reads the one rule rather than a copy of it. §2 is the control: + * a code the body declared is kept, and a plain 400 with no code still derives + * `VALIDATION_ERROR`. + */ + +import { describe, it, expect } from 'vitest'; +import { ApiErrorSchema } from '@objectstack/spec/api'; +import type { RouteHandler } from '@objectstack/spec/contracts'; +import { UNDECLARED_HOOK_REFUSAL_CODE } from '@objectstack/types'; +import { registerPackageRoutes } from './package-routes.js'; + +const PUBLISH = 'POST:/api/v1/packages/publish'; +const BODY = { manifest: { id: 'com.acme.crm', version: '1.0.0' }, metadata: {} }; + +/** The shape `runtime/src/sandbox/quickjs-runner.ts` throws for a body's own `throw`. */ +function sandboxRefusal(businessMessage: string, extra: Record = {}) { + const err: any = new Error(`hook 'guard' threw: Error: ${businessMessage}`); + err.name = 'SandboxError'; + err.innerMessage = businessMessage; + return Object.assign(err, extra); +} + +async function publishRejectingWith(error: unknown): Promise<{ status: number; body: any }> { + const routes = new Map(); + const server = { + get: () => {}, + post: (p: string, h: RouteHandler) => { routes.set(`POST:${p}`, h); }, + put: () => {}, delete: () => {}, patch: () => {}, use: () => {}, + listen: async () => {}, close: async () => {}, + } as any; + const svc = { publish: async () => { throw error; } }; + registerPackageRoutes(server, () => svc as any, '/api/v1', { + resolveExecutionContext: async () => ({ + userId: 'u_pkg', + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], + }), + } as any); + const captured = { status: 0, body: undefined as any }; + const res: any = { + json(data: any) { captured.body = data; }, + send() {}, + status(code: number) { captured.status = code; return res; }, + header() { return res; }, + }; + await routes.get(PUBLISH)!( + { params: {}, query: {}, headers: {}, method: 'POST', path: '/api/v1/packages/publish', body: BODY } as any, + res, + ); + return captured; +} + +describe('[#27672] §1 an undeclared hook abort answers HOOK_ABORTED at package publish', () => { + it('400, the hook\'s own sentence, and the code /data answers', async () => { + const { status, body } = await publishRejectingWith(sandboxRefusal('Publishing is frozen.')); + expect(status).toBe(400); + expect(body.error.code).toBe('HOOK_ABORTED'); + expect(body.error.code).toBe(UNDECLARED_HOOK_REFUSAL_CODE); + expect(body.error.message).toBe('Publishing is frozen.'); + // The body declared no code, so none is reported as declared. + expect(body.error).not.toHaveProperty('declaredCode'); + expect(ApiErrorSchema.safeParse(body.error).success).toBe(true); + }); + + it('a 4xx status the body declared without a code keeps the status', async () => { + const { status, body } = await publishRejectingWith(sandboxRefusal('Frozen.', { status: 422 })); + expect(status).toBe(422); + expect(body.error.code).toBe('HOOK_ABORTED'); + }); +}); + +describe('[#27672] §2 CONTROLS — a declared code is kept; nothing is invented elsewhere', () => { + it('a registered code the body declared is its own code', async () => { + const { status, body } = await publishRejectingWith(sandboxRefusal('Locked.', { code: 'RECORD_LOCKED', status: 409 })); + expect(status).toBe(409); + expect(body.error.code).toBe('RECORD_LOCKED'); + }); + + it('an unregistered code the body declared rides declaredCode beside the derived code', async () => { + const { body } = await publishRejectingWith(sandboxRefusal('Over the limit.', { code: 'CREDIT_LIMIT', status: 400 })); + expect(body.error.code).toBe('VALIDATION_ERROR'); + expect(body.error.declaredCode).toBe('CREDIT_LIMIT'); + }); + + it('a non-sandbox producer that declared a 400 and no code still derives VALIDATION_ERROR', async () => { + const { status, body } = await publishRejectingWith(Object.assign(new Error('Manifest is invalid.'), { status: 400 })); + expect(status).toBe(400); + expect(body.error.code).toBe('VALIDATION_ERROR'); + }); +}); diff --git a/packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts b/packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts index 02356a27cb3..c0ffa77a342 100644 --- a/packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts +++ b/packages/rest/src/rest-write-route-hook-refusal-sentence.ledger.test.ts @@ -277,12 +277,6 @@ const IMPORT_BODY = { format: 'json', rows: [{ name: 'x' }] }; const importRow = (res: any) => res.body?.results?.[0]?.error; const jobRow = async (_res: any, b: Booted) => (await terminalJobPatch(b)).results?.items?.[0]?.error; -/** - * [#22719] An undeclared refusal re-dressed in the nested envelope: the code - * the classification leaves out is the catalog's own floor for `400`. - */ -const REFUSED_400 = { status: 400, code: 'VALIDATION_ERROR' } as const; - /** * [#27597] The same refusal where the family re-dresses the `/data` door's * classification whole: a sandboxed hook's undeclared abort answers the @@ -373,8 +367,11 @@ const ANSWERS_THE_SENTENCE: Record = { 'POST /api/v1/approvals/requests/:id/comment': { called: 'approvals.comment', body: {}, answers: HOOK_ABORTED_400 }, // Package publish: the classification decides the status the door falls // back to, the shared read the sentence. Was `500 INTERNAL_ERROR`. + // [#27672] FLIPPED on purpose from `400 VALIDATION_ERROR`: the door's code + // is `resolveThrownHttpError`'s, which now answers an undeclared hook abort + // `HOOK_ABORTED`, the code `/data` answers. 'POST /api/v1/packages/publish': { - called: 'service:package.publish', answers: REFUSED_400, + called: 'service:package.publish', answers: HOOK_ABORTED_400, body: { manifest: { id: 'p1', version: '1.0.0' }, metadata: {} }, }, // External datasources: the shared read only, so status and code are this diff --git a/packages/runtime/src/domains/actions-hook-refusal-sentence.ledger.test.ts b/packages/runtime/src/domains/actions-hook-refusal-sentence.ledger.test.ts index 11a0f7142fe..fb167b1b7cf 100644 --- a/packages/runtime/src/domains/actions-hook-refusal-sentence.ledger.test.ts +++ b/packages/runtime/src/domains/actions-hook-refusal-sentence.ledger.test.ts @@ -136,7 +136,9 @@ describe('[#22588] the /actions ledger rows, each driven with a real nested hook expect(executeAction).toHaveBeenCalled(); expect(res.response.status).toBe(400); expect(res.response.body.success).toBe(false); - expect(res.response.body.error.code).toBe('VALIDATION_ERROR'); + // [#27672] FLIPPED on purpose from `VALIDATION_ERROR`: an undeclared + // abort answers `HOOK_ABORTED` here, the code `/data` answers. + expect(res.response.body.error.code).toBe('HOOK_ABORTED'); // The wording IS the defect here, so the sentence is asserted whole. expect(res.response.body.error.message).toBe(REFUSAL); expect(JSON.stringify(res.response.body)).not.toMatch(/threw:|hook '/); diff --git a/packages/runtime/src/domains/actions-undeclared-hook-abort-code.test.ts b/packages/runtime/src/domains/actions-undeclared-hook-abort-code.test.ts new file mode 100644 index 00000000000..011754117ba --- /dev/null +++ b/packages/runtime/src/domains/actions-undeclared-hook-abort-code.test.ts @@ -0,0 +1,130 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27672] The runtime `/actions` door answers a sandboxed body's undeclared + * abort with `HOOK_ABORTED`, the code `/api/v1/data` has answered since + * #27597, instead of the `VALIDATION_ERROR` its 400 derives. + * + * The door's catch hands a rejection to `errorFromThrown` + * (`http-dispatcher.ts`), whose code is `resolveThrownHttpError`'s + * (`@objectstack/types`). The resolver now answers + * `UNDECLARED_HOOK_REFUSAL_CODE` for a sandboxed refusal that declared no code, + * so this door reads the one rule rather than a copy of it. §2 is the control: + * a code the body declared is kept, a registered handler's plain rejection + * still derives `VALIDATION_ERROR`, and a crash is still the sanitised 500. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { UNDECLARED_HOOK_REFUSAL_CODE } from '@objectstack/types'; + +import { HttpDispatcher } from '../http-dispatcher.js'; +import { SandboxError } from '../sandbox/quickjs-runner.js'; + +const scriptAction = { + name: 'submit_signoff', + objectName: 'crm_invoice', + type: 'script', + body: { language: 'js', source: 'return 1;', capabilities: ['api.write'] }, +}; + +/** A dispatcher whose action handler rejects with `thrown`. */ +function makeDispatcher(thrown: unknown) { + const objectDef = { name: 'crm_invoice', actions: [scriptAction] }; + const ql: any = { + executeAction: vi.fn(async () => { throw thrown; }), + getSchema: (n: string) => (n === objectDef.name ? objectDef : undefined), + registry: { getObject: (n: string) => (n === objectDef.name ? objectDef : undefined), getItem: () => undefined }, + find: vi.fn(async () => [{ id: 'inv_1', status: 'draft' }]), + insert: vi.fn(), update: vi.fn(), delete: vi.fn(), + }; + const metadata: any = { + load: vi.fn(async () => null), + listObjects: vi.fn(async () => [objectDef]), + getObject: vi.fn(async () => objectDef), + }; + const kernel: any = { + context: { + getService: (n: string) => + n === 'objectql' || n === 'data' ? ql : n === 'metadata' ? metadata : null, + }, + }; + return new HttpDispatcher(kernel); +} + +async function invoke(thrown: unknown) { + const res: any = await makeDispatcher(thrown).handleActions( + '/crm_invoice/submit_signoff/inv_1', + 'POST', + {}, + { request: {}, environmentId: 'platform', executionContext: { userId: 'u1', systemPermissions: [] } } as any, + ); + return res.response; +} + +describe('[#27672] §1 an undeclared abort answers HOOK_ABORTED at /actions', () => { + it('the action body\'s own free-text throw', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const response = await invoke( + new SandboxError("action 'submit_signoff' threw: Contact has no phone", 'Contact has no phone'), + ); + expect(response.status).toBe(400); + expect(response.body.error.code).toBe('HOOK_ABORTED'); + expect(response.body.error.code).toBe(UNDECLARED_HOOK_REFUSAL_CODE); + expect(response.body.error.message).toBe('Contact has no phone'); + expect(response.body.error).not.toHaveProperty('declaredCode'); + }); + + it('a hook abort relayed through the action body (the nested refusal\'s shape)', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const response = await invoke(new SandboxError( + "action 'submit_signoff' threw: hook 'guard' threw: Error: A finished invoice cannot be signed.", + 'A finished invoice cannot be signed.', + )); + expect(response.status).toBe(400); + expect(response.body.error.code).toBe('HOOK_ABORTED'); + expect(response.body.error.message).toBe('A finished invoice cannot be signed.'); + }); + + it('a 4xx status the body declared without a code keeps the status', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const response = await invoke(new SandboxError("action 'submit_signoff' threw: Closed.", 'Closed.', { status: 422 })); + expect(response.status).toBe(422); + expect(response.body.error.code).toBe('HOOK_ABORTED'); + }); +}); + +describe('[#27672] §2 CONTROLS — a declared code is kept; nothing is invented elsewhere', () => { + it('a registered code the body declared is its own code', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const response = await invoke(new SandboxError( + "action 'submit_signoff' threw: Locked.", 'Locked.', { code: 'RECORD_LOCKED', status: 409 }, + )); + expect(response.status).toBe(409); + expect(response.body.error.code).toBe('RECORD_LOCKED'); + }); + + it('an unregistered code the body declared rides declaredCode beside the derived code', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const response = await invoke(new SandboxError( + "action 'submit_signoff' threw: Over the limit.", 'Over the limit.', { code: 'CREDIT_LIMIT', status: 400 }, + )); + expect(response.status).toBe(400); + expect(response.body.error.code).toBe('VALIDATION_ERROR'); + expect(response.body.error.declaredCode).toBe('CREDIT_LIMIT'); + }); + + it('a registered handler\'s plain rejection (no sandbox) still derives VALIDATION_ERROR', async () => { + const response = await invoke(new Error('Lead is already converted')); + expect(response.status).toBe(400); + expect(response.body.error.code).toBe('VALIDATION_ERROR'); + }); + + it('a body that CRASHED is still the sanitised 500', async () => { + vi.spyOn(console, 'error').mockImplementation(() => {}); + const response = await invoke(new SandboxError( + "action 'submit_signoff' threw: TypeError: x is not a function", 'TypeError: x is not a function', + )); + expect(response.status).toBe(500); + expect(response.body.error.code).toBe('INTERNAL_ERROR'); + }); +}); diff --git a/packages/runtime/src/sandbox-plain-object-throw-client-sentence.test.ts b/packages/runtime/src/sandbox-plain-object-throw-client-sentence.test.ts index 270225430f0..e02e4985f9f 100644 --- a/packages/runtime/src/sandbox-plain-object-throw-client-sentence.test.ts +++ b/packages/runtime/src/sandbox-plain-object-throw-client-sentence.test.ts @@ -201,7 +201,10 @@ describe('[#23718] the action door answers a body\'s thrown plain object with it it('CONTROL — a thrown Error answers as before', async () => { const response = await postAction('throw_error'); expect(response.status, JSON.stringify(response.body)).toBe(400); - expect(response.body?.error?.code).toBe('VALIDATION_ERROR'); + // [#27672] FLIPPED on purpose from `VALIDATION_ERROR`: a thrown Error + // with no code is an undeclared abort, answered `HOOK_ABORTED` as `/data` + // answers it. The sentence is this control's subject and is unchanged. + expect(response.body?.error?.code).toBe('HOOK_ABORTED'); expect(response.body?.error?.message).toBe('x'); }); }); diff --git a/packages/runtime/src/sandbox-unreadable-rejection-door.test.ts b/packages/runtime/src/sandbox-unreadable-rejection-door.test.ts index 5872a0155b2..ddfcf7c0f00 100644 --- a/packages/runtime/src/sandbox-unreadable-rejection-door.test.ts +++ b/packages/runtime/src/sandbox-unreadable-rejection-door.test.ts @@ -128,14 +128,18 @@ describe('[#23723] the action door answers an unreadable rejection at once, as a it("throw '': 400, the fixed sentence", async () => { const response = await postAction('throw_empty_string'); expect(response.status, JSON.stringify(response.body)).toBe(400); - expect(response.body?.error?.code).toBe('VALIDATION_ERROR'); + // [#27672] FLIPPED on purpose from `VALIDATION_ERROR`: the body declared + // no code, so this is an undeclared abort, answered `HOOK_ABORTED`. + expect(response.body?.error?.code).toBe('HOOK_ABORTED'); expect(response.body?.error?.message).toBe(UNREADABLE); }); it('throw Object.create(null): 400, the fixed sentence', async () => { const response = await postAction('throw_null_proto'); expect(response.status, JSON.stringify(response.body)).toBe(400); - expect(response.body?.error?.code).toBe('VALIDATION_ERROR'); + // [#27672] FLIPPED on purpose from `VALIDATION_ERROR`: the body declared + // no code, so this is an undeclared abort, answered `HOOK_ABORTED`. + expect(response.body?.error?.code).toBe('HOOK_ABORTED'); expect(response.body?.error?.message).toBe(UNREADABLE); }); }); diff --git a/packages/runtime/src/sandbox/nested-hook-refusal-is-a-rejection.test.ts b/packages/runtime/src/sandbox/nested-hook-refusal-is-a-rejection.test.ts index ff89d8bccd7..e7438fdb865 100644 --- a/packages/runtime/src/sandbox/nested-hook-refusal-is-a-rejection.test.ts +++ b/packages/runtime/src/sandbox/nested-hook-refusal-is-a-rejection.test.ts @@ -162,14 +162,17 @@ describe('[#17265] a nested sandboxed hook refusal keeps its business message', expect(err.innerMessage).not.toContain('SandboxError:'); }); - it('the wire answer matches /data: 400 VALIDATION_ERROR with the sentence', async () => { + it('the wire answer matches /data: 400 HOOK_ABORTED with the sentence', async () => { // /data answers this refusal `400` with `error.innerMessage` verbatim // (`error-response.ts`'s sandbox unwrap door, `declared ?? 400`). The // action route must not answer a second thing for one refusal. const response = await wireAnswer(await escapeOf(nestedHookRefusal)); expect(response.status).toBe(400); - expect(response.body.error.code).toBe('VALIDATION_ERROR'); + // [#27672] FLIPPED on purpose from `VALIDATION_ERROR`, which /data + // stopped answering in #27597: the undeclared abort is `HOOK_ABORTED` + // at both doors now, so this pin matches /data again. + expect(response.body.error.code).toBe('HOOK_ABORTED'); // [#22588] `toBe`, not `toContain`: a `toContain` pin stayed green // while the wire carried the hook's debug wrapper in front of it. expect(response.body.error.message).toBe(REFUSAL); diff --git a/packages/types/src/data-error-classification.ts b/packages/types/src/data-error-classification.ts index 2569741df03..8cae8db3f21 100644 --- a/packages/types/src/data-error-classification.ts +++ b/packages/types/src/data-error-classification.ts @@ -33,10 +33,15 @@ import { demotedDeclaredCode, declaredUserMessage, declaredRefusalMessage, + UNDECLARED_HOOK_REFUSAL_CODE, } from './thrown-http-error.js'; -import { isNativeErrorName } from './native-error-name.js'; +import { isScriptFaultMessage, sandboxBusinessMessage } from './sandbox-business-message.js'; import { unevaluableHookConditionRefusal } from './hook-condition-refusal.js'; -import type { ErrorCode } from '@objectstack/spec/api'; + +// [#27672] Declared in `sandbox-business-message.ts` so the resolver can read +// it too; exported from here as before, so the package's public names are +// unchanged. +export { sandboxBusinessMessage }; /** * How many characters of a domain error's OWN message reach the client. @@ -157,102 +162,6 @@ const UNCLASSIFIED_FAULT = (): { status: number; body: Record } body: { error: INTERNAL_ERROR_MESSAGE, code: 'INTERNAL_ERROR' }, }); -/** - * [#7543] Does an unwrapped sandbox message name a JS RUNTIME fault rather than - * a business refusal the hook body deliberately reported? - * - * The two sandbox-unwrap branches below exist for ONE shape: a hook or action - * body that runs `throw new Error('删除被阻断:仍有未结清的发票')`, i.e. an - * author writing a business rule whose message IS the remedy. They answer 400 - * with that message verbatim and deliberately no `code` (see each branch). - * - * A body that instead CRASHES — `ctx.input.title.trim()` where `title` is the - * number `12345` — also arrives as a thrown error, so it entered the same - * branch and its raw `TypeError: not a function` went out as the client-facing - * message of a 400 with no `code`. That is two contract breaks at once: an - * internal runtime fault echoed verbatim, and a body outside the ledgered - * envelope (a client keying on `code` gets nothing). - * - * The classification this restores is NOT new policy — it is the ruling - * {@link UNCLASSIFIED_FAULT} already records one door down, which names this - * exact case ("or a plain handler bug (`TypeError: x is not a function`) … - * server faults that a caller cannot fix and a caller SHOULD retry"). The - * sandbox unwraps simply sit ABOVE that branch and were intercepting the crash - * before it could reach the answer the file had already settled on. Same - * separation `quickjs-runner`'s own `sandboxFault` path draws (#4431/#3951): - * 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**, 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 - * its own words. That authoring style is not the documented one, and erring - * toward "a native error name means a crash" is the fail-safe direction — the - * opposite default is what shipped `TypeError: not a function` to a client. - * - * The words are not lost: 500 is outside `isExpectedDataStatus`, so - * `handleRouteError` prints `[REST] Unhandled error` with the whole error, and - * `sendThrownError`'s `logWithheldServerFault` (#5437) covers the routes that bypass - * it — the same operator path {@link UNCLASSIFIED_FAULT} relies on. - */ -function isScriptFaultMessage(message: string): boolean { - return isNativeErrorName(message.trim()); -} - -/** - * [#11588] The caller-addressed BUSINESS text a sandboxed hook/action body - * threw, or `undefined` when this error is not a sandbox refusal. - * - * QuickJS bodies throw a `SandboxError` whose `.message` is the - * ` '' threw: ` debug wrapper and whose `.innerMessage` is the - * text the author addressed to the end user (see - * `runtime/src/sandbox/quickjs-runner.ts`). The wrapper "belongs in server - * logs" — {@link classifyDataError}'s unwrap door exists precisely to keep it - * off the wire. This is that door's read, named once so the door ABOVE it in - * {@link resolveErrorResponse} can ask the same question instead of shipping - * `error.message` raw. - * - * Both conditions are the door's, in the door's order: - * - * - a non-empty string `.innerMessage`, which is what makes this a sandbox - * error at all; - * - NOT {@link isScriptFaultMessage}. A body that CRASHED arrives with the - * same shape, and its `TypeError: not a function` is an internal fault - * rather than a business message (#7543). This answers `undefined` there, - * so a crash is never mistaken for authored text. - * - * ⛔ It is deliberately a READ of a field the sandbox populated, never a - * pattern-strip of the wrapper off `.message`. Stripping prose by regex would - * also rewrite a plain error whose own text happens to contain `threw:`, and - * the caller's message is the remedy on a 4xx (#5423) — the one thing this - * boundary must not paraphrase. `rest-hook-refusal-message-parity.test.ts` §5 - * is the control that keeps it a read. - * - * Exported for the SECOND boundary that has to ask the same question: - * `/analytics/dataset/query` builds its own `{ code, message }` envelope inline - * in `rest-server.ts` and shares no branch with either door here. It reads this - * rather than re-deriving the unwrap, so the analytics face and the `/data` - * face cannot drift into two answers for one refusal — the door-disagreement - * shape #7525/#8016 keeps producing when a boundary open-codes the read. - */ -export function sandboxBusinessMessage(error: any): string | undefined { - if (typeof error?.innerMessage !== 'string' || !error.innerMessage) return undefined; - if (isScriptFaultMessage(error.innerMessage)) return undefined; - return error.innerMessage; -} - /** * [commit 1c7adc73d] The sentence a declared-code structured arm relays to the caller: * {@link sandboxBusinessMessage} first, `error.message` second. @@ -451,17 +360,6 @@ function thrownCodeFields(error: any, status: number): { code?: string; declared return { code: thrown.code, ...(demoted !== undefined ? { declaredCode: demoted } : {}) }; } -/** - * [#27597] The wire `code` a sandboxed hook body's business refusal answers - * with when the body declared none: the hook aborted the operation (its - * `onError: 'abort'`, the default) with free text, so the caller learns that - * fact from `code` instead of from the absence of one. - * - * Registered under `@objectstack/types` in the ADR-0112 ledger, and typed - * `ErrorCode` so a ledger without the row stops this file compiling. - */ -export const UNDECLARED_HOOK_REFUSAL_CODE: ErrorCode = 'HOOK_ABORTED'; - /** * [#27597] {@link thrownCodeFields} for a sandboxed body's business refusal * ({@link sandboxBusinessMessage} answered): the producer's own code when it @@ -480,6 +378,13 @@ export const UNDECLARED_HOOK_REFUSAL_CODE: ErrorCode = 'HOOK_ABORTED'; * wrapper fallback in {@link classifyDataError}, and `@objectstack/rest`'s * declared-status passthrough — so one abort carries one code whichever route * caught it. + * + * [#27672] The doors that answer from `resolveThrownHttpError` instead (package + * publish, the runtime dispatcher's exits) get the same constant from the + * resolver's own `code`, which is where it is declared. The wrapper fallback is + * why this does not simply read that `code`: it recognises an abort whose + * `SandboxError` instance was lost, which carries no `innerMessage` for the + * resolver to read. */ function sandboxRefusalCodeFields(error: any, status: number): { code?: string; declaredCode?: string } { const declared = thrownCodeFields(error, status); diff --git a/packages/types/src/sandbox-business-message.ts b/packages/types/src/sandbox-business-message.ts new file mode 100644 index 00000000000..8043b1dafcb --- /dev/null +++ b/packages/types/src/sandbox-business-message.ts @@ -0,0 +1,110 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#27672] The sandbox's REFUSAL-vs-CRASH read, in a module of its own so + * `resolveThrownHttpError` (`thrown-http-error.ts`) can ask it. + * + * Moved here verbatim from `data-error-classification.ts`, which imports + * `thrown-http-error.ts`: the resolver could not import the read back from + * there without a cycle. `data-error-classification.ts` re-exports + * {@link sandboxBusinessMessage}, so the package's public names are unchanged, + * and {@link isScriptFaultMessage} stays off the package entry as before. + */ + +import { isNativeErrorName } from './native-error-name.js'; + +/** + * [#7543] Does an unwrapped sandbox message name a JS RUNTIME fault rather than + * a business refusal the hook body deliberately reported? + * + * The two sandbox-unwrap branches in `data-error-classification.ts` exist for ONE shape: a hook or action + * body that runs `throw new Error('删除被阻断:仍有未结清的发票')`, i.e. an + * author writing a business rule whose message IS the remedy. They answer 400 + * with that message verbatim and deliberately no `code` (see each branch). + * + * A body that instead CRASHES — `ctx.input.title.trim()` where `title` is the + * number `12345` — also arrives as a thrown error, so it entered the same + * branch and its raw `TypeError: not a function` went out as the client-facing + * message of a 400 with no `code`. That is two contract breaks at once: an + * internal runtime fault echoed verbatim, and a body outside the ledgered + * envelope (a client keying on `code` gets nothing). + * + * The classification this restores is NOT new policy — it is the ruling + * {@link UNCLASSIFIED_FAULT} already records one door down, which names this + * exact case ("or a plain handler bug (`TypeError: x is not a function`) … + * server faults that a caller cannot fix and a caller SHOULD retry"). The + * sandbox unwraps simply sit ABOVE that branch and were intercepting the crash + * before it could reach the answer the file had already settled on. Same + * separation `quickjs-runner`'s own `sandboxFault` path draws (#4431/#3951): + * 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**, 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 + * its own words. That authoring style is not the documented one, and erring + * toward "a native error name means a crash" is the fail-safe direction — the + * opposite default is what shipped `TypeError: not a function` to a client. + * + * The words are not lost: 500 is outside `isExpectedDataStatus`, so + * `handleRouteError` prints `[REST] Unhandled error` with the whole error, and + * `sendThrownError`'s `logWithheldServerFault` (#5437) covers the routes that bypass + * it — the same operator path {@link UNCLASSIFIED_FAULT} relies on. + */ +export function isScriptFaultMessage(message: string): boolean { + return isNativeErrorName(message.trim()); +} + +/** + * [#11588] The caller-addressed BUSINESS text a sandboxed hook/action body + * threw, or `undefined` when this error is not a sandbox refusal. + * + * QuickJS bodies throw a `SandboxError` whose `.message` is the + * ` '' threw: ` debug wrapper and whose `.innerMessage` is the + * text the author addressed to the end user (see + * `runtime/src/sandbox/quickjs-runner.ts`). The wrapper "belongs in server + * logs" — {@link classifyDataError}'s unwrap door exists precisely to keep it + * off the wire. This is that door's read, named once so the door ABOVE it in + * {@link resolveErrorResponse} can ask the same question instead of shipping + * `error.message` raw. + * + * Both conditions are the door's, in the door's order: + * + * - a non-empty string `.innerMessage`, which is what makes this a sandbox + * error at all; + * - NOT {@link isScriptFaultMessage}. A body that CRASHED arrives with the + * same shape, and its `TypeError: not a function` is an internal fault + * rather than a business message (#7543). This answers `undefined` there, + * so a crash is never mistaken for authored text. + * + * ⛔ It is deliberately a READ of a field the sandbox populated, never a + * pattern-strip of the wrapper off `.message`. Stripping prose by regex would + * also rewrite a plain error whose own text happens to contain `threw:`, and + * the caller's message is the remedy on a 4xx (#5423) — the one thing this + * boundary must not paraphrase. `rest-hook-refusal-message-parity.test.ts` §5 + * is the control that keeps it a read. + * + * Exported for the SECOND boundary that has to ask the same question: + * `/analytics/dataset/query` builds its own `{ code, message }` envelope inline + * in `rest-server.ts` and shares no branch with either door here. It reads this + * rather than re-deriving the unwrap, so the analytics face and the `/data` + * face cannot drift into two answers for one refusal — the door-disagreement + * shape #7525/#8016 keeps producing when a boundary open-codes the read. + */ +export function sandboxBusinessMessage(error: any): string | undefined { + if (typeof error?.innerMessage !== 'string' || !error.innerMessage) return undefined; + if (isScriptFaultMessage(error.innerMessage)) return undefined; + return error.innerMessage; +} diff --git a/packages/types/src/thrown-http-error-undeclared-hook-abort.test.ts b/packages/types/src/thrown-http-error-undeclared-hook-abort.test.ts new file mode 100644 index 00000000000..355f88e2ac1 --- /dev/null +++ b/packages/types/src/thrown-http-error-undeclared-hook-abort.test.ts @@ -0,0 +1,71 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#27672] `resolveThrownHttpError` answers `UNDECLARED_HOOK_REFUSAL_CODE` +// (`HOOK_ABORTED`) for a sandboxed body's refusal that declared no code, so the +// doors that read the resolver's `code` — package publish, the runtime +// dispatcher's exits — answer the code the `/data` door has answered since +// #27597, instead of the `VALIDATION_ERROR` a 400 derives. +// +// §1 is the card. §2 is the control: a code the body declared is kept, and +// nothing changes for a producer that is not a sandboxed refusal, for a crash, +// or for a 5xx. + +import { describe, it, expect } from 'vitest'; +import { resolveThrownHttpError, demotedDeclaredCode, UNDECLARED_HOOK_REFUSAL_CODE } from './thrown-http-error.js'; +import { sandboxBusinessMessage } from './data-error-classification.js'; + +/** The shape `runtime/src/sandbox/quickjs-runner.ts` throws for a body's own `throw`. */ +function sandboxRefusal(businessMessage: string, extra: Record = {}) { + const err: any = new Error(`hook 'guard' threw: Error: ${businessMessage}`); + err.name = 'SandboxError'; + err.innerMessage = businessMessage; + return Object.assign(err, extra); +} + +describe('[#27672] §1 an undeclared hook abort resolves to HOOK_ABORTED', () => { + it('at the 400 a door falls back to (publish, /actions)', () => { + const thrown = resolveThrownHttpError(sandboxRefusal('Opportunity is closed.'), 400); + expect(thrown.status).toBe(400); + expect(thrown.code).toBe('HOOK_ABORTED'); + expect(thrown.code).toBe(UNDECLARED_HOOK_REFUSAL_CODE); + // The body declared no code, so nothing is reported as declared or demoted. + expect(thrown.declaredCode).toBeUndefined(); + expect(demotedDeclaredCode(thrown)).toBeUndefined(); + }); + + it('at a 4xx status the body declared without a code (`status` or `statusCode`)', () => { + expect(resolveThrownHttpError(sandboxRefusal('Closed.', { status: 422 }))).toMatchObject({ status: 422, code: 'HOOK_ABORTED' }); + expect(resolveThrownHttpError(sandboxRefusal('Closed.', { statusCode: 403 }))).toMatchObject({ status: 403, code: 'HOOK_ABORTED' }); + }); + + it('the constant is the wire spelling', () => { + expect(UNDECLARED_HOOK_REFUSAL_CODE).toBe('HOOK_ABORTED'); + }); +}); + +describe('[#27672] §2 CONTROLS — nothing else moves', () => { + it('a registered code the body declared is its own code', () => { + expect(resolveThrownHttpError(sandboxRefusal('Locked.', { code: 'RECORD_LOCKED', status: 409 })).code).toBe('RECORD_LOCKED'); + }); + + it('an unregistered code the body declared (a `refusals` catalogue entry) keeps the derived code and its declaredCode', () => { + const thrown = resolveThrownHttpError(sandboxRefusal('Over the limit.', { code: 'CREDIT_LIMIT', status: 400 })); + expect(thrown.code).toBe('VALIDATION_ERROR'); + expect(thrown.declaredCode).toBe('CREDIT_LIMIT'); + }); + + it('a non-sandbox producer that declared a 400 and no code still derives VALIDATION_ERROR', () => { + expect(resolveThrownHttpError(Object.assign(new Error('Closed.'), { status: 400 })).code).toBe('VALIDATION_ERROR'); + }); + + it('a body that CRASHED is not an abort', () => { + const crash = sandboxRefusal('TypeError: not a function'); + expect(sandboxBusinessMessage(crash)).toBeUndefined(); + expect(resolveThrownHttpError(crash, 400).code).toBe('VALIDATION_ERROR'); + }); + + it('a sandboxed refusal resolved at a 5xx keeps the fault code', () => { + expect(resolveThrownHttpError(sandboxRefusal('Closed.'), 500).code).toBe('INTERNAL_ERROR'); + expect(resolveThrownHttpError(sandboxRefusal('Down.', { status: 503 })).code).toBe('SERVICE_UNAVAILABLE'); + }); +}); diff --git a/packages/types/src/thrown-http-error.ts b/packages/types/src/thrown-http-error.ts index 8742ad4f1ac..8deb2a09b92 100644 --- a/packages/types/src/thrown-http-error.ts +++ b/packages/types/src/thrown-http-error.ts @@ -94,6 +94,23 @@ import { ErrorCode, standardErrorCodeForHttpStatus } from '@objectstack/spec/api import { declaresServerFault, looksLikeInternalErrorLeak } from './error-leak.js'; import { validationFailureDetails, VALIDATION_FAILED_STATUS } from './validation-failure.js'; import { unevaluableHookConditionRefusal } from './hook-condition-refusal.js'; +import { sandboxBusinessMessage } from './sandbox-business-message.js'; + +/** + * [#27597] The wire `code` a sandboxed hook body's business refusal answers + * with when the body declared none: the hook aborted the operation (its + * `onError: 'abort'`, the default) with free text, so the caller learns that + * fact from `code` instead of from the absence of one. + * + * Registered under `@objectstack/types` in the ADR-0112 ledger, and typed + * `ErrorCode` so a ledger without the row stops this file compiling. + * + * [#27672] Declared beside {@link resolveThrownHttpError}, which answers it, + * so every door that reads the resolver's `code` (package publish, the + * runtime dispatcher's exits) answers the code the `/data` door answers. + * `data-error-classification.ts` reads this same constant. + */ +export const UNDECLARED_HOOK_REFUSAL_CODE: ErrorCode = 'HOOK_ABORTED'; /** The HTTP answer a thrown error declares. See {@link resolveThrownHttpError}. */ export interface ThrownHttpError { @@ -194,7 +211,7 @@ export interface ThrownHttpError { * |---|---| * | status | `.status` → `.statusCode` → 400 if it is a validation failure → 400 if it is an unevaluable hook condition → `fallbackStatus` | * | declaredStatus | the same chain WITHOUT the fallback — absent when the throw declared none | - * | code | `VALIDATION_FAILED` if it is one → a REGISTERED `.code` → derived from the status | + * | code | `VALIDATION_FAILED` if it is one → a REGISTERED `.code` → {@link UNDECLARED_HOOK_REFUSAL_CODE} if it is a sandboxed body's refusal that declared no code, at a 4xx status → derived from the status | * | declaredCode | `VALIDATION_FAILED` if it is one → any non-empty string `.code` → the derived code if it is an unevaluable hook condition → absent | * | message | `.message` when it is a string → `String(error)` | * | userMessage | a non-empty string `.userMessage` → absent (see {@link declaredUserMessage}) | @@ -238,10 +255,22 @@ export function resolveThrownHttpError(error: unknown, fallbackStatus = 500): Th const registered = spelled !== undefined && ErrorCode.safeParse(spelled).success ? (spelled as ErrorCode) : undefined; + const declaredCode = validation ? validation.code : (spelled ?? hookCondition?.code); + // [#27672] A sandboxed body's business refusal that declared no code is a + // hook ABORT, and the `/data` door has answered it `HOOK_ABORTED` since + // #27597. Answered here, so the doors that read this `code` instead of that + // classification answer the same code rather than the 400's derived + // `VALIDATION_ERROR`. Two limits keep it to that one case: a code the body + // declared is kept (`declaredCode` is set), and only a 4xx answer is a + // refusal — an undeclared throw resolved at a 5xx fallback is a fault, and + // keeps the fault's code. `declaredCode` stays absent: the body declared none. + const undeclaredHookAbort = declaredCode === undefined + && status >= 400 && status < 500 + && sandboxBusinessMessage(e) !== undefined; const code: ErrorCode = validation ? validation.code - : (registered ?? standardErrorCodeForHttpStatus(status)); - const declaredCode = validation ? validation.code : (spelled ?? hookCondition?.code); + : (registered + ?? (undeclaredHookAbort ? UNDECLARED_HOOK_REFUSAL_CODE : standardErrorCodeForHttpStatus(status))); const issues = Array.isArray(e?.issues) ? e.issues : undefined; const details: Record = {