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
12 changes: 12 additions & 0 deletions .changeset/24662-hook-condition-coded-refusal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
'@objectstack/types': patch
---

fix(types): a write an authored hook condition refused because it could not be evaluated answers a coded `400 VALIDATION_ERROR`, not the unclassified `500 INTERNAL_ERROR`

Clause-②: no (the write is refused before and after, with nothing stored; only the refusal's status and code move, onto the published `StandardErrorCode` member `VALIDATION_ERROR`; no accept set moves and no code is added)

- **What changed on the wire.** A `HookConditionError` whose condition compiled and then faulted on the record it judged (`reason: 'unevaluable'`) is now a refusal at every data door: `400`, code `VALIDATION_ERROR`, and the hook's own message, which names the hook and the CEL fault and never the condition source or a record value. Before, it matched nothing and was answered `500 INTERNAL_ERROR` — on `@objectstack/rest`'s doors with a `[REST] Unhandled error` line, and on the runtime dispatcher's `/data` door (through `@objectstack/hono`'s catch-all) with the hook's message at status 500. A `/batch` row for the same throw now reads `VALIDATION_ERROR` with `httpStatus: 400` and the hook's message, where it read `INTERNAL_ERROR` with a withheld message.
- **Why `400` and `VALIDATION_ERROR`.** The status is the one a validation rule that cannot be evaluated answers, and the two share one message builder. The code is the catalog member `400` names, because a hook's gate addresses no field and the per-field `VALIDATION_FAILED` envelope does not fit it.
- **The log.** The refusal is handed to the caller, so `@objectstack/rest` no longer prints `[REST] Unhandled error` for it. Its log verdict reads the declaration from `resolveThrownHttpError`, as it does for any declared client refusal.
- **Unchanged.** A condition that never compiled (`reason: 'uncompilable'`) keeps its sanitised `500` and its log line. That fault does not depend on the request, and its message quotes the condition source. A hook that refuses on purpose answers exactly as before.
5 changes: 4 additions & 1 deletion content/docs/automation/hooks.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,10 @@ condition" carry *opposite* risks depending on the hook: swallowed into a `befor
guard it silently lets the write through; swallowed into an audit hook it silently
drops the record. They are now distinct outcomes, and the second **fails the write**.
Hooks that have been getting by on that skip will start failing — that is how you
find out they were never enforcing anything.
find out they were never enforcing anything. Over the API the refused write answers
`400` with the code `VALIDATION_ERROR`, and its message names the hook and the fault,
never the condition's source — the same status a validation rule that cannot be
evaluated answers.

**The condition reads the record, not the payload (#4770).** It used to evaluate
against `ctx.input.data` — only the fields the current write happened to carry — so
Expand Down
12 changes: 9 additions & 3 deletions docs/qa/platform-checklist/areas/records-forms.json
Original file line number Diff line number Diff line change
Expand Up @@ -3337,7 +3337,7 @@
"title": "Object lifecycle hooks fire on the write path with the right timing, gate, async and error semantics — driven over /api/v1/data/*, oracled by the record effect and the log line",
"since": "v15",
"status": "active",
"revision": 6,
"revision": 7,
"priority": "P1",
"surface": "mixed",
"personas": [
Expand Down Expand Up @@ -3442,8 +3442,8 @@
{
"clause": "a condition that cannot be evaluated FAILS CLOSED (#4775): the write is ABORTED, not silently skipped — and it is NOT softened by onError:'log' nor fire-and-forgotten, because the condition gate runs OUTSIDE the async/retry/onError wrappers",
"oracle": "api",
"verify": "scratch hook whose condition COMPILES but cannot be evaluated at runtime (e.g. int(record.name) > 0, written to a row whose name is not numeric); a write to that object is refused and the row does not land (hook-wrappers.ts: conditionFn throws outside runWithErrorPolicy). The 18.0 wire envelope is a generic 500 {code: 'INTERNAL_ERROR'}, with the class only in the server log ('[REST] Unhandled error: HookConditionError …') — #24619 row (b), open, tracks the coded refusal it should be. Until that lands score the clause on the refusal, the absent row and the log line, and record the 500 against #24619 rather than as a new finding. An UNCOMPILABLE condition never reaches this gate: it is refused at authoring (PUT /api/v1/meta/hook → 422 INVALID_METADATA, expression-invalid), so it cannot stand in for the runtime fail-closed probe",
"evidence": "the refusal (status + body) + the HookConditionError server-log line + a post-attempt count showing no row"
"verify": "scratch hook whose condition COMPILES but cannot be evaluated at runtime (e.g. int(record.name) > 0, written to a row whose name is not numeric); a write to that object is refused and the row does not land (hook-wrappers.ts: conditionFn throws outside runWithErrorPolicy). The wire envelope is a coded refusal: 400 {code: 'VALIDATION_ERROR'}, whose error text names the hook and the CEL fault (\"Hook 'NAME' could not evaluate its condition (FAULT) — operation aborted.\") and never the condition source, and no '[REST] Unhandled error' line is logged for it (#24662 retired the generic 500 INTERNAL_ERROR that #24619 row (b) recorded). A 500 INTERNAL_ERROR here is a FAIL. An UNCOMPILABLE condition never reaches this gate: it is refused at authoring (PUT /api/v1/meta/hook → 422 INVALID_METADATA, expression-invalid), so it cannot stand in for the runtime fail-closed probe",
"evidence": "the refusal (status 400 + code VALIDATION_ERROR + the hook named in its error text) + a post-attempt count showing no row"
},
{
"clause": "two hooks on the SAME object+event run lowest-priority-first: scratch hooks at priority 10 and 90 log in that order on one write (hook-binder passes priority to engine.registerHook; the engine orders by it, lower first)",
Expand Down Expand Up @@ -3516,6 +3516,12 @@
"date": "2026-10-19",
"change": "steps[0], negative[2], fixtures.requires[3], acceptance[8] (verify, evidence) and automated.ref corrected from wave anchor #24579 round 2 part 1 (6272032736) and QA runs #24598 / #24616 (ba4afcc2), re-read at objectstack f89d29e382. (1) The hook list route is GET /api/v1/meta/hook; /meta/types/hook answers 404. (2) acceptance[8]'s refusal answers 500 INTERNAL_ERROR with the class in the server log only; #24619 row (b) (open) tracks the coded refusal, so the clause states the shipped envelope and scores against #24619. (3) The ref named source files (hook-wrappers.ts, hook-binder.ts) no resolver can run; it now names four test files, each measured at base under os-verify-lock (83 passed), mapped to the clause whose mechanism it pins. (4) The run-proven scratch-hook recipe is recorded in requires[3]. Part 1's note on a `$select` in the steps does not hold: no step of this item spells `$select` (0 hits in records-forms.json)",
"ref": "#24671"
},
{
"revision": 7,
"date": "2026-10-19",
"change": "acceptance[8] (verify, evidence) follow the shipped envelope. An unevaluable hook condition's refusal is now a coded 400 VALIDATION_ERROR whose error text names the hook and the CEL fault, and no '[REST] Unhandled error' line is logged for it (#24662, split from #24619 row (b)). The 500 INTERNAL_ERROR that revision 6 recorded is retired and is now a FAIL; the server-log evidence goes with it. The clause is unchanged: the write is still refused and the row still absent",
"ref": "#24662"
}
]
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
// its handler throws; an `abort` hook with the same unbindable condition is
// rolled back as #22782 leaves it. One more: an `async: true` hook whose
// condition evaluates now starts inside the door's unit and writes after it
// commits — its late write must still land (#22855). The refusal is REST's
// sanitised `500 INTERNAL_ERROR` on both sides of the fix; what this file pins
// is what is STORED.
// commits — its late write must still land (#22855). What this file pins is
// what is STORED, and the refusal's status and code: since #24662 an
// unevaluable condition is REST's coded `400 VALIDATION_ERROR` (it was the
// unclassified `500 INTERNAL_ERROR` on both sides of #22856's fix).

import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import { bootStack, type VerifyStack } from '@objectstack/verify';
Expand Down Expand Up @@ -72,13 +73,13 @@ describe('an after* hook whose condition cannot be evaluated rolls the write bac

it('onError: \'log\' — REST createData is refused and the row is absent', async () => {
const name = uniq('c');
expect(await refusal(await create(LOGGED, name))).toEqual({ status: 500, code: 'INTERNAL_ERROR' });
expect(await refusal(await create(LOGGED, name))).toEqual({ status: 400, code: 'VALIDATION_ERROR' });
expect(await stored(LOGGED, name)).toBe(0);
});

it('async: true — the condition is evaluated before the hook detaches: refused, the row absent', async () => {
const name = uniq('c');
expect(await refusal(await create(DETACHED, name))).toEqual({ status: 500, code: 'INTERNAL_ERROR' });
expect(await refusal(await create(DETACHED, name))).toEqual({ status: 400, code: 'VALIDATION_ERROR' });
expect(await stored(DETACHED, name)).toBe(0);
});

Expand All @@ -90,7 +91,7 @@ describe('an after* hook whose condition cannot be evaluated rolls the write bac

it('CONTROL an abort hook with the same condition: refused, the row absent — as #22782 leaves it', async () => {
const name = uniq('c');
expect(await refusal(await create(ABORTING, name))).toEqual({ status: 500, code: 'INTERNAL_ERROR' });
expect(await refusal(await create(ABORTING, name))).toEqual({ status: 400, code: 'VALIDATION_ERROR' });
expect(await stored(ABORTING, name)).toBe(0);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ import { describe, it, expect } from 'vitest';
import { readFileSync } from 'node:fs';
import { dirname, resolve } from 'node:path';
import { fileURLToPath } from 'node:url';
import { DuplicateRecordError } from '@objectstack/objectql';
import { DuplicateRecordError, HookConditionError } from '@objectstack/objectql';
import { ConcurrentUpdateError } from '@objectstack/metadata-protocol';
import {
mapDataError,
Expand Down Expand Up @@ -436,6 +436,32 @@ const CASES: readonly Case[] = [
},
},
},
{
// [#24662] An arm keyed on the CLASS (`name`) and its declared
// `reason`, so the code-literal census below cannot see it; listed
// here so the three doors are held together for it anyway. Its
// producer declares no `status` either.
covers: ['HookConditionError'],
what: "objectql's HookConditionError, reason 'unevaluable' — the condition compiled and faulted on this record",
error: () => new HookConditionError(
"Hook 'qa_failclosed' could not evaluate its condition (runtime: int() type error: cannot convert to int) — operation aborted.",
{
hook: 'qa_failclosed', object: 'inquiry', event: 'beforeInsert',
condition: "record.name.startsWith('qa-fc') && int(record.name) > 0",
reason: 'unevaluable', fault: 'runtime: int() type error: cannot convert to int',
},
),
object: 'inquiry',
restored: [],
expect: {
status: 400,
body: {
error: "Hook 'qa_failclosed' could not evaluate its condition (runtime: int() type error: cannot convert to int) — operation aborted.",
code: 'VALIDATION_ERROR',
object: 'inquiry',
},
},
},
{
// The third control: a 5xx ARM. Its producer declares no `status`
// either, so both doors reach it today; §4 pins that it can never
Expand Down
Loading
Loading