Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/27597-undeclared-hook-abort-code.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@objectstack/spec': patch
'@objectstack/types': patch
'@objectstack/rest': patch
---

fix(rest): a hook that aborts with free text answers the data door with `HOOK_ABORTED` instead of no `code`

Clause-②: yes (widening)
Metadata-protocol: none

- **Before.** A sandboxed hook body that aborted a write with free text (`throw new Error('…')`, no `code`, no `refusals` catalogue) answered `/api/v1/data` with `400 {"error":"…","object":"…"}` and no `code`, the one refusal on that door a client could not branch on.
- **Now.** The same abort answers `400 {"error":"…","code":"HOOK_ABORTED","object":"…"}`; a body that declared a 4xx `status` (or `statusCode`) keeps that status and gains the same code. `HOOK_ABORTED` is registered in the ADR-0112 ledger under `@objectstack/types` (`@objectstack/spec`), stamped by `mapDataError`'s sandbox unwrap door and its wrapper fallback (`@objectstack/types`, exported as `UNDECLARED_HOOK_REFUSAL_CODE`) and by the route door's declared-status passthrough (`@objectstack/rest`).
- **Moved.** The same undeclared abort also changes value on doors that already carried a code, from the status-derived `VALIDATION_ERROR` to `HOOK_ABORTED`: the nested refusal envelopes of the sharing rules routes (`POST /api/v1/sharing/rules`, `DELETE /api/v1/sharing/rules/:idOrName`, `POST /api/v1/sharing/rules/:idOrName/evaluate`), the security suggested-bindings routes (`POST /api/v1/security/suggested-bindings/:id/confirm`, `…/dismiss`, `…/reopen`), the approvals request routes (`POST /api/v1/approvals/requests/:id/approve`, `…/reject`, `…/recall`, `…/reassign`, `…/comment`, `…/remind`, `…/request-info`, `…/resubmit`, `…/revise`), and each row of the `failures` list that `POST /api/v1/data/import/jobs/:jobId/undo` returns.
- **Unchanged.** A refusal that declared a code keeps it: a `refusals` catalogue entry still answers `declaredCode` beside the status-derived `code`, and a registered code a body throws is still its own wire `code`. A non-hook producer that declared no code still gets none, and a crashed body is still the sanitised `500 INTERNAL_ERROR`. The server log line for the abort is not changed here.
4 changes: 3 additions & 1 deletion content/docs/protocol/kernel/error-handling.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,9 @@ nested envelope would nest under `details` rides as **top-level siblings** here.
- `error` — the human-readable message, **a string**.
- `code` — the machine-readable code. **Optional**: ADR-0112 invents nothing for the half
a producer did not name, so a refusal that declared a status but no code arrives with
`error` alone. Branch on `code` only after checking it is there.
`error` alone. Branch on `code` only after checking it is there. One producer is named
instead: a sandboxed hook body that aborts with free text and no code answers
`HOOK_ABORTED`, at the 4xx status it declared, else `400`.
- `object` — the object the refusal is about, when the door knows it.
- `declaredCode` / `userMessage` — the same two optional channels as above, as top-level
siblings.
Expand Down
3 changes: 2 additions & 1 deletion content/docs/references/api/automation-api.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ const result = AutomationApiErrorCode.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +351 more>` | ✅ | The inner refusal's code in its WIRE spelling - the ADR-0112 member a record save of the same write answers in `error.code` (e.g. `UNIQUE_VIOLATION`); the closed set `ApiErrorSchema.code` parses against |
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +352 more>` | ✅ | The inner refusal's code in its WIRE spelling - the ADR-0112 member a record save of the same write answers in `error.code` (e.g. `UNIQUE_VIOLATION`); the closed set `ApiErrorSchema.code` parses against |
| **message** | `string` | ✅ | The inner refusal's own diagnostic text, verbatim, without the node and operation prefixes the run's failure text adds |

### Allowed Values: `FlowInnerRefusal.code`
Expand Down Expand Up @@ -520,6 +520,7 @@ const result = AutomationApiErrorCode.parse(data);
* `FORBIDDEN`
* `FORM_NOT_FOUND`
* `FORM_RESOLVE_FAILED`
* `HOOK_ABORTED`
* `HOOK_UNSCOPED_DATA_ACCESS`
* `IMPORT_JOB_CREATE_FAILED`
* `IMPORT_ROW_FAILED`
Expand Down
3 changes: 2 additions & 1 deletion content/docs/references/api/contract.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const result = ApiErrorSchema.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +351 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) |
| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +352 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) |
| **declaredCode** | `string` | optional | The producer-declared code, verbatim, when it is not a member of the closed `code` vocabulary — the open, author-authored channel (app-specific spellings; ADR-0112) |
| **message** | `string` | ✅ | Readable error message |
| **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim. Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution for anything unmarked. Status-agnostic; never replaces `message`. |
Expand Down Expand Up @@ -208,6 +208,7 @@ const result = ApiErrorSchema.parse(data);
* `FORBIDDEN`
* `FORM_NOT_FOUND`
* `FORM_RESOLVE_FAILED`
* `HOOK_ABORTED`
* `HOOK_UNSCOPED_DATA_ACCESS`
* `IMPORT_JOB_CREATE_FAILED`
* `IMPORT_ROW_FAILED`
Expand Down
1 change: 1 addition & 0 deletions content/docs/references/api/error-code-ledger.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ const result = ErrorCode.parse(data);
* `FORBIDDEN`
* `FORM_NOT_FOUND`
* `FORM_RESOLVE_FAILED`
* `HOOK_ABORTED`
* `HOOK_UNSCOPED_DATA_ACCESS`
* `IMPORT_JOB_CREATE_FAILED`
* `IMPORT_ROW_FAILED`
Expand Down
15 changes: 9 additions & 6 deletions packages/qa/dogfood/test/hook-error-format.dogfood.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
// deliberate business rule (e.g. referential-integrity "记录被引用,删除被
// 阻断"). The console shows the REST body's `error` string verbatim in its
// toast, so that string must be ONLY the author's message — not the sandbox
// debug wrapper (`hook 'x' threw: Error: …`, which belongs in server logs)
// and not a `code` field: `throw new Error(...)` declares no code, and
// ADR-0112 invents nothing for a producer that declared none.
// debug wrapper (`hook 'x' threw: Error: …`, which belongs in server logs).
//
// [#27597] The body still carries a `code`, and not one the hook declared:
// `throw new Error(...)` declares none, so the abort answers the ledgered
// `HOOK_ABORTED`, the one code a client branches on for "a hook refused this
// write in its own words". It used to carry no `code` at all.
//
// [#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
Expand Down Expand Up @@ -115,10 +118,10 @@ describe('objectstack verify: sandboxed hook error message format (#hef)', () =>
// The console toast renders this string verbatim — it must be exactly
// what the hook author threw.
expect(body.error).toBe(BUSINESS_MSG);
// No sandbox debug wrapper, no branding, and no `code` — the hook threw
// no code, and ADR-0112 invents nothing for a producer that declared none.
// No sandbox debug wrapper and no branding. [#27597] The hook threw no
// code, so the abort answers the ledgered `HOOK_ABORTED`.
expect(JSON.stringify(body)).not.toMatch(/threw:|hook '|\[ObjectStack\]/);
expect(body.code).toBeUndefined();
expect(body.code).toBe('HOOK_ABORTED');
});

it('ground truth: the blocked delete did not remove the record', async () => {
Expand Down
12 changes: 11 additions & 1 deletion packages/rest/src/error-response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {
mapDataError,
resolveThrownHttpError,
sandboxBusinessMessage,
sandboxRefusalCodeFields,
structuredCodeAnswer,
thrownCodeFields,
truncateClientMessage,
Expand Down Expand Up @@ -585,11 +586,20 @@ function resolveErrorResponse(error: any, object?: string): { status: number; bo
// route that passes nothing still answers exactly the bytes it answers
// today (`classifiedRefusalAnswer` calls this door with no object at
// all, and its families' key sets are unmoved).
//
// [#27597] A sandboxed body's refusal that declared no code answers
// `UNDECLARED_HOOK_REFUSAL_CODE`, read through the same
// `sandboxRefusalCodeFields` the unwrap door in `mapDataError` reads,
// so a hook that sets `status` and the one that sets `statusCode` (or
// neither) still answer one code. Every other producer keeps
// `thrownCodeFields`' answer, which invents nothing.
return withDeclaredUserMessage(error, {
status: error.status,
body: {
error: safeMsg,
...thrownCodeFields(error, error.status),
...(businessMessage !== undefined
? sandboxRefusalCodeFields(error, error.status)
: thrownCodeFields(error, error.status)),
...(Array.isArray(error.issues) ? { issues: error.issues } : {}),
...(object ? { object } : {}),
},
Expand Down
7 changes: 5 additions & 2 deletions packages/rest/src/import-undo-controls.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -328,9 +328,12 @@ describe('[#24380] an import undo runs under the object\'s declared controls for

const res = await ctx.undo(jobId);
expect(res._json).toMatchObject({ success: false, deleted: 0, restored: 0, failed: 2 });
// [#27597] The freeze is a sandboxed hook's free-text abort, so each row
// carries the ledgered `HOOK_ABORTED` (the status-derived `VALIDATION_ERROR`
// before).
expect(res._json.failures).toEqual([
{ id: 'c_new', action: 'delete', code: 'VALIDATION_ERROR', error: FROZEN },
{ id: 'c_old', action: 'restore', code: 'VALIDATION_ERROR', error: FROZEN },
{ id: 'c_new', action: 'delete', code: 'HOOK_ABORTED', error: FROZEN },
{ id: 'c_old', action: 'restore', code: 'HOOK_ABORTED', error: FROZEN },
]);
expect(await ctx.record('ou_case', 'c_new')).toMatchObject({ title: 'fresh', status: 'closed' });
expect(await ctx.record('ou_case', 'c_old')).toMatchObject({ title: 'after', status: 'closed' });
Expand Down
9 changes: 6 additions & 3 deletions packages/rest/src/rest-declared-client-refusal-log.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,17 +273,20 @@ describe('[#27508] a hook `onError: abort` is a refusal the route answered — q
innerMessage: 'boom abort',
});

it('POST /data/:object answers the same code-less 400 and prints no "[REST] Unhandled error"', async () => {
it('POST /data/:object answers the same 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' });
// [#27597] The door adds the ledgered `HOOK_ABORTED` to an undeclared
// abort's answer; the log verdict reads the thrown error, so it stays quiet.
expect(res.body).toEqual({ error: 'boom abort', code: 'HOOK_ABORTED', 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' });
// [#27597] Same door-added code as above.
expect(res.body).toEqual({ error: 'boom abort', code: 'HOOK_ABORTED' });
expect(unhandledLogs()).toHaveLength(0);
});

Expand Down
16 changes: 9 additions & 7 deletions packages/rest/src/rest-hook-refusal-code-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,15 +350,15 @@ describe('[#10345] the sandbox unwrap is the branch, and it was never status-sha
// ---------------------------------------------------------------------------

describe('[#10345] the pinned defaults the fix must not disturb', () => {
it('a refusal that declares NO code still carries none — nothing is invented', () => {
// ADR-0112 D4 governs the semantic-CODE channel — the producer names
// the condition on the CODE axis, and the ADR rules no HTTP status for
// an undeclared throw. Narrowing or widening the
// vocabulary must never start ADDING codes to bodies that carried none.
it('a refusal that declares NO code answers the one ledgered HOOK_ABORTED, and declares nothing', () => {
// [#27597] This used to pin `code` absent ("nothing is invented"). The
// undeclared abort now answers one registered code naming the event —
// a hook aborted with free text — and still no `declaredCode`: the
// producer declared none, so nothing was demoted.
const r = mapDataError(sandboxRefusal('month-end close is in progress'), 'crm_account');
expect(r.status).toBe(400);
expect(r.body.error).toBe('month-end close is in progress');
expect(r.body).not.toHaveProperty('code');
expect(r.body.code).toBe('HOOK_ABORTED');
expect(r.body).not.toHaveProperty('declaredCode');
});

Expand Down Expand Up @@ -426,9 +426,11 @@ describe('[#10345 / #9232] the unwrap door speaks the closed ADR-0112 vocabulary
});

it('CONTROL — a numeric errno and an empty string are not declarations', () => {
// [#27597] So the refusal is an undeclared abort: `HOOK_ABORTED`, never
// the errno or the empty string.
for (const code of [1062, '']) {
const r = mapDataError(sandboxRefusal('refused', { code, status: 409 }), 'crm_account');
expect(r.body.code, String(code)).toBeUndefined();
expect(r.body.code, String(code)).toBe('HOOK_ABORTED');
expect(r.body.declaredCode, String(code)).toBeUndefined();
}
});
Expand Down
10 changes: 5 additions & 5 deletions packages/rest/src/rest-hook-refusal-message-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -693,15 +693,15 @@ describe('[#11588] the analytics dataset face answers in the hook\'s words too',
// `rest-share-refusal-classification.test.ts`. No live pin was found
// on the other side of the question, so the fork clause did not fire.
//
// No code, deliberately: the producer declared none and nothing is
// invented for it, which is `thrownCodeFields`' answer on `/data` and
// ADR-0112's rule. `ANALYTICS_QUERY_FAILED` is ③'s code and ③ is not
// where this refusal belongs any more.
// [#27597] The code is `/data`'s too: the producer declared none, so
// the abort answers the ledgered `HOOK_ABORTED` (it carried no code
// before). `ANALYTICS_QUERY_FAILED` is ③'s code and ③ is not where
// this refusal belongs any more.
const res = await analyticsRefusal(sandboxRefusal('month-end close is in progress'));

expect(res.statusCode).toBe(400);
expect(res.body.message).toBe('month-end close is in progress');
expect(res.body.code).toBeUndefined();
expect(res.body.code).toBe('HOOK_ABORTED');
expect(JSON.stringify(res.body)).not.toMatch(WRAPPER_RE);
}, 60_000);

Expand Down
8 changes: 6 additions & 2 deletions packages/rest/src/rest-hook-script-fault-envelope.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,16 +324,20 @@ describe('[#7543] a hook that deliberately refuses still speaks in its own words
expect(r.body.error).not.toBe(INTERNAL_ERROR_MESSAGE);
});

it('the refusal envelope carries no `code` when the hook declared none', () => {
it('the refusal envelope carries `HOOK_ABORTED` when the hook declared no code', () => {
// [commit cad8b42f0] Re-read, not rewritten to fit. This assertion always tested a
// producer that declares NO code, so it pins ADR-0112's "nothing is
// invented for a half-declaration" and is green on both sides of that
// fix. What it never tested — and what the comment here used to claim
// — is that the door withholds a code the producer DID declare; that
// claim was the defect, and its "older clients prepend the code to the
// message" rationale is retired (see `error-response.ts`).
//
// [#27597] Flipped: the undeclared abort answers the ledgered
// `HOOK_ABORTED`, and still no `declaredCode`.
const r = mapDataError(hookRefusal('h', 'month-end close is in progress'), 'showcase_task');
expect(r.body).not.toHaveProperty('code');
expect(r.body.code).toBe('HOOK_ABORTED');
expect(r.body).not.toHaveProperty('declaredCode');
});

it('an English refusal that merely MENTIONS a native error name is still a refusal', () => {
Expand Down
12 changes: 8 additions & 4 deletions packages/rest/src/rest-sandbox-declared-status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,9 @@ describe('[#9967] mapDataError: a sandboxed body that declares a 4xx status keep
const undeclared = mapDataError(sandboxRefusal(), 'showcase_task');

expect(declared.body).toEqual(undeclared.body);
expect(declared.body.code).toBeUndefined();
// [#27597] Both carry `HOOK_ABORTED` now (no `code` before): the
// body declared a status and no code, so the code is the abort's.
expect(declared.body.code).toBe('HOOK_ABORTED');
});

it('the whole client band is served, off either spelling — same read as every other exit', () => {
Expand Down Expand Up @@ -166,12 +168,13 @@ describe('[#9967] a sandboxed body that declares a 5xx takes the sanitised 5xx a
const CRASH_WORDS = '__OS_SANDBOX_FAULT_7f3a9c__';

describe('[#9967] the pinned defaults are untouched', () => {
it('an UNDECLARED body throw keeps the verbatim-message 400 with no code', () => {
it('an UNDECLARED body throw keeps the verbatim-message 400, coded HOOK_ABORTED', () => {
const r = mapDataError(sandboxRefusal(), 'showcase_task');

expect(r.status).toBe(400);
expect(r.body.error).toBe('close-period lock');
expect(r.body.code).toBeUndefined();
// [#27597] It carried no code before.
expect(r.body.code).toBe('HOOK_ABORTED');
});

it('a body CRASH stays the sanitised 500 even when it carries a stray `status`', () => {
Expand Down Expand Up @@ -252,7 +255,8 @@ describe('[#9967] the reported request on the real data route', () => {

expect(res.statusCode).toBe(403);
expect(res.body.error).toBe('close-period lock');
expect(res.body.code).toBeUndefined();
// [#27597] The status is the body's; the code is the undeclared abort's.
expect(res.body.code).toBe('HOOK_ABORTED');
}, 60_000);

it('the declared refusal is not logged as an unhandled fault — 403 is an expected outcome', async () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/rest/src/rest-share-declared-code.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,11 @@ const ABSENT: Array<{ name: string; error: unknown; status: number; code: string
status: 403, code: 'FORBIDDEN',
},
{
// [#27597] `code` is the undeclared abort's `HOOK_ABORTED` (the
// status-derived `VALIDATION_ERROR` before); still nothing to declare.
name: 'a sandboxed body that declared NO code has nothing to declare',
error: sandboxRefusal('sharing is frozen until the quarterly access review closes'),
status: 400, code: 'VALIDATION_ERROR',
status: 400, code: 'HOOK_ABORTED',
},
{
name: "the ADR-0111 `CODE: message` prefix idiom declares no envelope at all",
Expand Down
7 changes: 4 additions & 3 deletions packages/rest/src/rest-share-user-message.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,14 +302,15 @@ const MARKED: Array<{
},
{
// The sandbox limb with NO declared status: `classifyDataError`'s unwrap
// door answers 400 and the nested envelope fills the required `code`
// from the catalog floor. The mark rides that arm too — commit 79c46da90's rule is
// door answers 400 with the undeclared abort's `HOOK_ABORTED` (#27597;
// the nested envelope filled the catalog floor `VALIDATION_ERROR`
// before). The mark rides that arm too — commit 79c46da90's rule is
// branch-agnostic by construction.
name: 'a sandboxed hook body with a sentence and NO declared status',
error: sandboxRefusal('sharing is frozen until the access review closes', {
userMessage: 'The quarterly access review closes on Friday.',
}),
status: 400, code: 'VALIDATION_ERROR',
status: 400, code: 'HOOK_ABORTED',
userMessage: 'The quarterly access review closes on Friday.',
},
{
Expand Down
Loading
Loading