From f286c443d998ab9483cae649e4de29c606d81553 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 20:27:47 +0000 Subject: [PATCH] fix(metadata-protocol): an undeclared hook abort is HOOK_ABORTED on the batch row too The per-object batch row (POST /data/:object/batch, every verb and mode, and updateManyData/deleteManyData rows) derived a sandboxed free-text refusal's code from its 400 (VALIDATION_ERROR), while /data, /createMany and /batch answer HOOK_ABORTED since #27597. toRowApiError now picks UNDECLARED_HOOK_REFUSAL_CODE for that case; a declared code still wins. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01WGufzeV9ZbECNr1Z2G2Puu --- .changeset/29894-batch-row-hook-aborted.md | 12 ++ .../src/protocol-client-failures.ts | 32 +++- ...protocol.batch-row-sandbox-refusal.test.ts | 17 +- .../batch-row-hook-aborted.dogfood.test.ts | 153 ++++++++++++++++++ 4 files changed, 205 insertions(+), 9 deletions(-) create mode 100644 .changeset/29894-batch-row-hook-aborted.md create mode 100644 packages/qa/dogfood/test/batch-row-hook-aborted.dogfood.test.ts diff --git a/.changeset/29894-batch-row-hook-aborted.md b/.changeset/29894-batch-row-hook-aborted.md new file mode 100644 index 00000000000..6e0011144c1 --- /dev/null +++ b/.changeset/29894-batch-row-hook-aborted.md @@ -0,0 +1,12 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol): a hook that aborts with free text answers `HOOK_ABORTED` on the per-object batch row too, not `VALIDATION_ERROR` + +Clause-②: no +Metadata-protocol: none + +- **Before.** Since #27597 (and #27672) a sandboxed hook body that aborts with free text (`throw new Error('…')`, no `code`) answers `/api/v1/data/:object`, `/createMany` and `/api/v1/batch` with `400 HOOK_ABORTED` and the hook's sentence. `POST /api/v1/data/:object/batch` answered the same abort as a row `{ code: 'VALIDATION_ERROR', message: }`: the row mapper (`toRowApiError`) derived the code from the `400` the doors serve, so a bulk client branching on the code read a business refusal as malformed input. +- **Now.** The row answers `HOOK_ABORTED` (the `UNDECLARED_HOOK_REFUSAL_CODE` the other doors read). The value moves, from `VALIDATION_ERROR` to `HOOK_ABORTED`, on every row that shares the mapper: `POST /api/v1/data/:object/batch` with `operation` `create`, `update`, `upsert` or `delete`, in every mode (plain, `atomic`, `continueOnError`), and the per-row results of `updateManyData` / `deleteManyData`. +- **Unchanged.** A code the hook declared still wins (a registered code is its own row code; an unregistered one still gets the code its status derives). The row's `message` is still the hook's sentence and `httpStatus` is still absent when the body declared no status. A crashed body and a non-sandbox producer answer as before. No error code is added; `HOOK_ABORTED` is the one #27597 registered. diff --git a/packages/metadata-protocol/src/protocol-client-failures.ts b/packages/metadata-protocol/src/protocol-client-failures.ts index ee6d5502c25..8764bbd6955 100644 --- a/packages/metadata-protocol/src/protocol-client-failures.ts +++ b/packages/metadata-protocol/src/protocol-client-failures.ts @@ -7,7 +7,7 @@ // A MOVE, not a redesign: every declaration below is byte-identical to the text that stood // above `ObjectStackProtocolImplementation`; only the imports and the export list at the foot // are new. `protocol.ts` re-exports what it exported. -import { declaredHttpStatus, declaredUserMessage, isSandboxOrigin, resolveThrownHttpError, sandboxBusinessMessage } from '@objectstack/types'; +import { declaredHttpStatus, declaredUserMessage, isSandboxOrigin, resolveThrownHttpError, sandboxBusinessMessage, UNDECLARED_HOOK_REFUSAL_CODE } from '@objectstack/types'; import type { ApiError, BatchOperationResult } from '@objectstack/spec/api'; import { ErrorCode, standardErrorCodeForHttpStatus } from '@objectstack/spec/api'; import { zodIssuesToMetadataIssues } from './protocol-metadata-issues.js'; @@ -139,6 +139,9 @@ export type BatchDataRowResult = BatchOperationResult; * answers, the `code` limb derives from the `400` the doors serve (the * `declared ?? 400` of `classifyDataError`, `@objectstack/types`). A code or * status the body DID declare still wins, as for every other producer. + * [#29894] Since #27597 those doors answer that refusal `HOOK_ABORTED`, not the + * 400's `VALIDATION_ERROR`, so the row does too: {@link isUndeclaredHookAbort} + * picks {@link UNDECLARED_HOOK_REFUSAL_CODE} before the status derivation. * * ⛔ `httpStatus` is NOT stamped for it. That limb carries only a status the * producer declared (above), and this body declared none. @@ -150,14 +153,37 @@ export function toRowApiError(err: any, fallback: string): ApiError { ? (err.code as ApiError['code']) : undefined; const { declaredStatus } = resolveThrownHttpError(err); - const servedStatus = declaredStatus ?? (sandboxRefusalSentence(err) !== undefined ? 400 : undefined); + const refusal = sandboxRefusalSentence(err) !== undefined; + const servedStatus = declaredStatus ?? (refusal ? 400 : undefined); return { - code: thrown ?? (servedStatus !== undefined ? standardErrorCodeForHttpStatus(servedStatus) : 'INTERNAL_ERROR'), + code: thrown + ?? (isUndeclaredHookAbort(err, refusal, servedStatus) ? UNDECLARED_HOOK_REFUSAL_CODE + : servedStatus !== undefined ? standardErrorCodeForHttpStatus(servedStatus) : 'INTERNAL_ERROR'), message: clientFacingRowFailureText(err, fallback), ...(declaredStatus !== undefined ? { httpStatus: declaredStatus } : {}), }; } +/** + * [#29894] Is this row failure a hook ABORT that declared no code? + * + * #27597 gave a sandboxed body's free-text refusal one wire code, + * {@link UNDECLARED_HOOK_REFUSAL_CODE} (`HOOK_ABORTED`), on `/data`, and + * #27672 moved `resolveThrownHttpError`'s doors to it. The batch row kept the + * code its `400` derives (`VALIDATION_ERROR`), so `POST /data/:object/batch` + * named the same refusal differently from `/data`, `/createMany` and + * `/batch`. This is the resolver's own test, restated over the row's inputs: + * a sandboxed refusal ({@link sandboxRefusalSentence} answered), no `code` + * spelled at all (an unregistered one is a declaration and keeps the + * status-derived code, as on the resolver's doors), served at a 4xx. + */ +function isUndeclaredHookAbort(err: unknown, refusal: boolean, servedStatus: number | undefined): boolean { + const code = (err as { code?: unknown } | null | undefined)?.code; + return refusal + && !(typeof code === 'string' && code !== '') + && servedStatus !== undefined && servedStatus >= 400 && servedStatus < 500; +} + /** * [commit 65846bc46] Is this thrown value the ENGINE's unique-violation envelope? * diff --git a/packages/metadata-protocol/src/protocol.batch-row-sandbox-refusal.test.ts b/packages/metadata-protocol/src/protocol.batch-row-sandbox-refusal.test.ts index 539dbe3d298..736e251106f 100644 --- a/packages/metadata-protocol/src/protocol.batch-row-sandbox-refusal.test.ts +++ b/packages/metadata-protocol/src/protocol.batch-row-sandbox-refusal.test.ts @@ -11,7 +11,8 @@ * created.')`: * * - `POST /api/v1/data/:object`, `/createMany` and `/import` answered the - * sentence (`400 VALIDATION_ERROR` on the first two, the row's `error` on the + * sentence (`400 VALIDATION_ERROR` on the first two — `HOOK_ABORTED` since + * #27597, and on the batch row since #29894 — the row's `error` on the * third); * - `POST /api/v1/data/:object/batch` answered `200` with the row * `{ code: 'INTERNAL_ERROR', message: 'The create of this record failed. The @@ -179,13 +180,15 @@ async function batchCreate(error: Error): Promise { // ─── §1 ────────────────────────────────────────────────────────────────────── describe('[#24561] §1 the card: a sandboxed refusal on a batch create reads as the hook\'s sentence', () => { - it('the row carries the sentence and the 400 floor code, and no withhold warning is written', async () => { + // [#29894] The code is `HOOK_ABORTED` (#27597's ledgered code for an undeclared + // hook abort), not the 400 floor `VALIDATION_ERROR` the row used to derive. + it('the row carries the sentence and HOOK_ABORTED, and no withhold warning is written', async () => { const warn = captureWarn(); try { const res = await batchCreate(sandboxRefusal()); expect(res.results[0].success).toBe(false); - expect(res.results[0].errors).toEqual([{ code: 'VALIDATION_ERROR', message: SENTENCE }]); + expect(res.results[0].errors).toEqual([{ code: 'HOOK_ABORTED', message: SENTENCE }]); // No `httpStatus`: the body declared none, and the limb reports only a // declared status (#8570). The row's shape is what it was. expect(res.results[0].errors[0]).not.toHaveProperty('httpStatus'); @@ -197,14 +200,16 @@ describe('[#24561] §1 the card: a sandboxed refusal on a batch create reads as } }); - it('the row agrees with the data door: the same sentence, and the code the door\'s status names', async () => { + it('the row agrees with the data door: the same sentence, and the same code', async () => { const refusal = sandboxRefusal(); const door = mapDataError(refusal, OBJECT); const res = await batchCreate(refusal); expect(door.status).toBe(400); expect(res.results[0].errors[0].message).toBe(door.body.error); - expect(res.results[0].errors[0].code).toBe(standardErrorCodeForHttpStatus(door.status)); + // [#29894] The door's own code, not the code its status derives. + expect(door.body.code).toBe('HOOK_ABORTED'); + expect(res.results[0].errors[0].code).toBe(door.body.code); }); }); @@ -225,7 +230,7 @@ describe('[#24561] §2 every loop that shares the row sink answers the same way' const warn = captureWarn(); try { const res = await run(refusing()); - expect(res.results[0].errors).toEqual([{ code: 'VALIDATION_ERROR', message: SENTENCE }]); + expect(res.results[0].errors).toEqual([{ code: 'HOOK_ABORTED', message: SENTENCE }]); expect(JSON.stringify(res)).not.toMatch(WRAPPER_RE); expect(withholdCalls(warn)).toHaveLength(0); } finally { diff --git a/packages/qa/dogfood/test/batch-row-hook-aborted.dogfood.test.ts b/packages/qa/dogfood/test/batch-row-hook-aborted.dogfood.test.ts new file mode 100644 index 00000000000..9440ec4f204 --- /dev/null +++ b/packages/qa/dogfood/test/batch-row-hook-aborted.dogfood.test.ts @@ -0,0 +1,153 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#29894] An undeclared hook abort answers `HOOK_ABORTED` on the per-object +// batch row too, the code `/data`, `/createMany` and `/batch` give it. +// +// #27597 gave a sandboxed hook body's free-text refusal (`throw new Error('…')`, +// no `code`) the ledgered wire code `HOOK_ABORTED`. The per-object batch door +// (`POST /data/:object/batch`) reports a row failure through `toRowApiError` +// (`@objectstack/metadata-protocol`), which derived the code from the 400 the +// sibling doors serve, so its row said `VALIDATION_ERROR` for the same +// refusal. This file is the card's four-door reproduction over the real HTTP +// stack of one boot, plus the batch row's other modes and verbs, and the +// control that a code the hook DID declare still wins on every door. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; + +const SENTENCE = 'A QA budget line needs an approver before it can be recorded.'; +const DECLARED = 'This budget period is locked.'; + +const obj = (name: string) => ObjectSchema.create({ + name, + sharingModel: 'public_read_write', + label: name, + pluralLabel: name, + fields: { name: Field.text({ label: 'Name', required: true }) }, +}); + +const throwUndeclared = { language: 'js', source: `throw new Error(${JSON.stringify(SENTENCE)});` } as const; + +const stack29894 = defineStack({ + manifest: { + id: 'com.dogfood.batch-row-hook-aborted', + namespace: 'bra', + version: '0.0.0', + type: 'app', + name: 'Batch Row Hook Aborted Fixture', + description: 'Hooks that abort a write in their own words, with and without a declared code.', + }, + objects: [obj('bra_ins'), obj('bra_upd'), obj('bra_del'), obj('bra_declared')], + hooks: [ + { name: 'bra_ins_refuse', object: 'bra_ins', events: ['beforeInsert'], body: throwUndeclared }, + { name: 'bra_upd_refuse', object: 'bra_upd', events: ['beforeUpdate'], body: throwUndeclared }, + { name: 'bra_del_refuse', object: 'bra_del', events: ['beforeDelete'], body: throwUndeclared }, + { + name: 'bra_declared_refuse', + object: 'bra_declared', + events: ['beforeInsert'], + body: { language: 'js', source: `throw Object.assign(new Error(${JSON.stringify(DECLARED)}), { code: 'RECORD_LOCKED' });` }, + }, + ], +} as any); + +const SYS = { context: { isSystem: true } } as const; + +/** The failing row of a per-object batch answer: the one that is not a sibling's NOT_ATTEMPTED / ROLLED_BACK. */ +function causalRowError(body: any): any { + const rows: any[] = body?.results ?? body?.data?.results ?? body?.error?.details?.results ?? []; + const errors = rows.flatMap((r) => r?.errors ?? []); + return errors.find((e) => e?.code !== 'NOT_ATTEMPTED' && e?.code !== 'ROLLED_BACK'); +} + +describe('[#29894] an undeclared hook abort is HOOK_ABORTED on every data door, the batch row included', () => { + let stack: VerifyStack; + let admin = ''; + let ql: any; + let updId = ''; + let delId = ''; + + beforeAll(async () => { + stack = await bootStack(stack29894 as never, {}); + admin = await stack.signIn(); + ql = await stack.kernel.getServiceAsync('objectql'); + updId = String((await ql.insert('bra_upd', { name: 'seed' }, SYS))?.id); + delId = String((await ql.insert('bra_del', { name: 'seed' }, SYS))?.id); + }, 240_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + describe('§1 the card: the whole-request doors answer 400 HOOK_ABORTED with the sentence', () => { + const doors: Array<[string, () => Promise]> = [ + ['POST /data/OBJECT', () => stack.apiAs(admin, 'POST', '/data/bra_ins', { name: 'x' })], + ['POST /data/OBJECT/createMany', () => stack.apiAs(admin, 'POST', '/data/bra_ins/createMany', [{ name: 'x' }])], + ['POST /batch', () => stack.apiAs(admin, 'POST', '/batch', { operations: [{ action: 'create', object: 'bra_ins', data: { name: 'x' } }] })], + ]; + for (const [door, send] of doors) { + it(door, async () => { + const res = await send(); + const body: any = await res.json(); + expect(res.status, JSON.stringify(body)).toBe(400); + expect(JSON.stringify(body)).toContain(SENTENCE); + expect(JSON.stringify(body)).toContain('"code":"HOOK_ABORTED"'); + }); + } + }); + + describe('§2 the per-object batch row answers the same code, in every mode and verb', () => { + const cases: Array<[string, () => Promise]> = [ + ['create', () => stack.apiAs(admin, 'POST', '/data/bra_ins/batch', { operation: 'create', records: [{ data: { name: 'x' } }] })], + ['create atomic', () => stack.apiAs(admin, 'POST', '/data/bra_ins/batch', { operation: 'create', records: [{ data: { name: 'x' } }, { data: { name: 'y' } }], options: { atomic: true } })], + ['create continueOnError', () => stack.apiAs(admin, 'POST', '/data/bra_ins/batch', { operation: 'create', records: [{ data: { name: 'x' } }, { data: { name: 'y' } }], options: { continueOnError: true } })], + ['update', () => stack.apiAs(admin, 'POST', '/data/bra_upd/batch', { operation: 'update', records: [{ id: updId, data: { name: 'moved' } }] })], + ['upsert (existing id)', () => stack.apiAs(admin, 'POST', '/data/bra_upd/batch', { operation: 'upsert', records: [{ id: updId, data: { name: 'moved' } }] })], + ['delete', () => stack.apiAs(admin, 'POST', '/data/bra_del/batch', { operation: 'delete', records: [{ id: delId }] })], + ]; + for (const [label, send] of cases) { + it(label, async () => { + const res = await send(); + const body: any = await res.json(); + const row = causalRowError(body); + expect(row, JSON.stringify(body)).toBeDefined(); + expect(row.message, JSON.stringify(body)).toBe(SENTENCE); + expect(row.code, `#29894: the row answers the code its sibling doors give the same abort: ${JSON.stringify(body)}`).toBe('HOOK_ABORTED'); + }); + } + + it('nothing was written or moved', async () => { + expect(await ql.find('bra_ins', { where: {}, ...SYS })).toHaveLength(0); + const [upd] = await ql.find('bra_upd', { where: {}, ...SYS }); + expect(upd?.name).toBe('seed'); + expect(await ql.find('bra_del', { where: {}, ...SYS })).toHaveLength(1); + }); + }); + + describe('§3 control: a code the hook declared still wins on every door', () => { + const doors: Array<[string, () => Promise]> = [ + ['POST /data/OBJECT', () => stack.apiAs(admin, 'POST', '/data/bra_declared', { name: 'x' })], + ['POST /data/OBJECT/createMany', () => stack.apiAs(admin, 'POST', '/data/bra_declared/createMany', [{ name: 'x' }])], + ['POST /batch', () => stack.apiAs(admin, 'POST', '/batch', { operations: [{ action: 'create', object: 'bra_declared', data: { name: 'x' } }] })], + ]; + for (const [door, send] of doors) { + it(door, async () => { + const res = await send(); + const text = await res.text(); + expect(text).toContain(DECLARED); + expect(text).toContain('"code":"RECORD_LOCKED"'); + expect(text).not.toContain('HOOK_ABORTED'); + }); + } + + it('POST /data/OBJECT/batch', async () => { + const res = await stack.apiAs(admin, 'POST', '/data/bra_declared/batch', { operation: 'create', records: [{ data: { name: 'x' } }] }); + const body: any = await res.json(); + const row = causalRowError(body); + expect(row?.message, JSON.stringify(body)).toBe(DECLARED); + expect(row?.code, JSON.stringify(body)).toBe('RECORD_LOCKED'); + }); + }); +});