fix(core): an undeclared hook abort is HOOK_ABORTED on the import row too

.changeset/29978-import-row-hook-aborted.md

@@ -0,0 +1,12 @@
+---
+'@objectstack/core': patch
+---
+
+fix(core): a hook that aborts with free text answers `HOOK_ABORTED` on the import row too, not `IMPORT_ROW_FAILED`
+
+Clause-②: no
+Metadata-protocol: none
+
+- **Before.** A sandboxed hook body that aborts with free text (`throw new Error('…')`, no `code`) answers `/api/v1/data/:object`, `/createMany`, `/api/v1/batch` and every `POST /api/v1/data/:object/batch` row with `HOOK_ABORTED` and the hook's sentence. `POST /api/v1/data/:object/import` and the async import job answered the same abort as a row `{ ok: false, code: 'IMPORT_ROW_FAILED', error: <sentence> }`, so an import client branching on the row's `code` could not tell a business rule's refusal from any other failed row.
+- **Now.** The import row answers `HOOK_ABORTED` (the `UNDECLARED_HOOK_REFUSAL_CODE` the other doors read), judged by `resolveThrownHttpError`, the resolver that declares that code. The value moves, from `IMPORT_ROW_FAILED` to `HOOK_ABORTED`, on every row the core import runner builds from a write failure: the synchronous import, the async import job's stored results, the create and update halves of an upsert, and every write path (single-row, batched, partial-success).
+- **Unchanged.** A code the hook declared still wins, registered or not. The row's `error` is still the hook's sentence. A body that crashed, a refusal that declared a 5xx status, a driver fault and every other row failure keep the code they had. The row's `code` is an open string in `ImportRowResultSchema`, so no schema moves, and no error code is added: `HOOK_ABORTED` is the one the ledger already registers.

packages/core/src/utils/import-runner-sandbox-refusal-row.test.ts

@@ -32,10 +32,16 @@
  * §5 (#22718) holds the two sandboxed failures the door answers as a fault, a
  * crashed body and a refusal that declared a 5xx: their row reads the door's
  * sentence.
+ *
+ * [#29978] The row's `code` for a refusal that declared none is
+ * `HOOK_ABORTED` (`UNDECLARED_HOOK_REFUSAL_CODE`), the code the create door
+ * answers for it, where it was `IMPORT_ROW_FAILED`; §1 pins it on each write
+ * path and §6 pins door-to-row agreement and the failures that keep
+ * `IMPORT_ROW_FAILED`.
  */
 
 import { describe, it, expect, vi } from 'vitest';
-import { mapDataError, INTERNAL_ERROR_MESSAGE } from '@objectstack/types';
+import { mapDataError, INTERNAL_ERROR_MESSAGE, UNDECLARED_HOOK_REFUSAL_CODE } from '@objectstack/types';
 import { runImport, sanitizeRowError, type ImportProtocolLike } from './import-runner';
 import type { ExportFieldMeta } from './import-field-meta.js';
 
@@ -98,7 +104,7 @@ describe('[#22694] §1 — each write path reports the hook\'s sentence', () =>
     const summary = await runImport({ ...baseOpts, p, rows: [{ name: 'r0' }, { name: 'r1' }, { name: 'r2' }] });
 
     expect(summary.created).toBe(2);
-    expect(summary.results[1]).toEqual({ row: 2, ok: false, action: 'failed', error: SENTENCE, code: 'IMPORT_ROW_FAILED' });
+    expect(summary.results[1]).toEqual({ row: 2, ok: false, action: 'failed', error: SENTENCE, code: UNDECLARED_HOOK_REFUSAL_CODE });
     expect(JSON.stringify(summary)).not.toMatch(WRAPPER_RE);
   });
 
@@ -110,7 +116,7 @@ describe('[#22694] §1 — each write path reports the hook\'s sentence', () =>
     // Anti-vacuity: the batch really ran and really degraded.
     expect(createManyData).toHaveBeenCalledTimes(1);
     expect(p.createData).toHaveBeenCalledTimes(3);
-    expect(summary.results[1]).toMatchObject({ ok: false, action: 'failed', error: SENTENCE, code: 'IMPORT_ROW_FAILED' });
+    expect(summary.results[1]).toMatchObject({ ok: false, action: 'failed', error: SENTENCE, code: UNDECLARED_HOOK_REFUSAL_CODE });
     expect(JSON.stringify(summary)).not.toMatch(WRAPPER_RE);
   });
 
@@ -126,7 +132,7 @@ describe('[#22694] §1 — each write path reports the hook\'s sentence', () =>
 
     expect(insertManyData).toHaveBeenCalledTimes(1);
     expect(createManyData).not.toHaveBeenCalled();
-    expect(summary.results[1]).toMatchObject({ ok: false, action: 'failed', error: SENTENCE, code: 'IMPORT_ROW_FAILED' });
+    expect(summary.results[1]).toMatchObject({ ok: false, action: 'failed', error: SENTENCE, code: UNDECLARED_HOOK_REFUSAL_CODE });
     expect(JSON.stringify(summary)).not.toMatch(WRAPPER_RE);
   });
 
@@ -140,7 +146,7 @@ describe('[#22694] §1 — each write path reports the hook\'s sentence', () =>
     const summary = await runImport({ ...baseOpts, writeMode: 'upsert', matchFields: ['name'], p, rows: [{ name: 'r1' }] });
 
     expect(updateData).toHaveBeenCalledTimes(1);
-    expect(summary.results[0]).toMatchObject({ ok: false, action: 'failed', error: SENTENCE, code: 'IMPORT_ROW_FAILED' });
+    expect(summary.results[0]).toMatchObject({ ok: false, action: 'failed', error: SENTENCE, code: UNDECLARED_HOOK_REFUSAL_CODE });
   });
 });
 
@@ -223,3 +229,32 @@ describe('[#22718] §5 — a sandboxed body the door answers as a fault reads as
     expect(JSON.stringify(row)).not.toMatch(leak);
   });
 });
+
+describe('[#29978] §6 — an undeclared abort answers the create door\'s code', () => {
+  it('the row\'s code is the code `mapDataError` answers for the same refusal', async () => {
+    const err = () => sandboxRefusal(SENTENCE);
+    const door = mapDataError(err(), OBJECT);
+    // Anti-vacuity: the door really answers the abort with that code.
+    expect(door.body.code).toBe('HOOK_ABORTED');
+
+    const row = await rowFor(err);
+    expect(row).toEqual({ row: 1, ok: false, action: 'failed', error: SENTENCE, code: door.body.code });
+  });
+
+  it('a refusal that declared a 4xx status but no code is still an abort', async () => {
+    const row = await rowFor(() => sandboxRefusal(SENTENCE, { status: 422 }));
+    expect(row).toMatchObject({ error: SENTENCE, code: UNDECLARED_HOOK_REFUSAL_CODE });
+  });
+
+  it.each([
+    ['a declared registered code', () => sandboxRefusal(SENTENCE, { code: 'RECORD_LOCKED' }), 'RECORD_LOCKED'],
+    ['a declared unregistered code', () => sandboxRefusal(SENTENCE, { code: 'MZ_PERIOD_CLOSED' }), 'MZ_PERIOD_CLOSED'],
+    ['a body that CRASHED', sandboxCrash, 'IMPORT_ROW_FAILED'],
+    ['a refusal that declared a 5xx and no code', () => sandboxRefusal(SENTENCE, { status: 503 }), 'IMPORT_ROW_FAILED'],
+    ['a plain error carrying the wrapper text but no `innerMessage`', () => new Error(`hook 'g' threw: Error: ${SENTENCE}`), 'IMPORT_ROW_FAILED'],
+    ['a driver fault', () => new Error('SQLITE_BUSY: database is locked'), 'IMPORT_ROW_FAILED'],
+  ] as const)('CONTROL — %s keeps its own code', async (_label, err, code) => {
+    const row = await rowFor(err);
+    expect(row.code).toBe(code);
+  });
+});

packages/core/src/utils/import-runner.ts

@@ -12,7 +12,7 @@ import type { ExecutionContext } from '@objectstack/spec/kernel';
 import type { CreateDataRequest, FindDataRequest, GetMetaItemRequest, UpdateDataRequest, ValidateDataIssue, ValidateDataRequest, ValidateDataResponse } from '@objectstack/spec/api';
 import { isMatchKeySets } from '@objectstack/spec/api';
 import { bulkWrite, withTransientRetry, defaultIsTransientError, type BulkWriteRowResult } from './bulk-write.js';
-import { isUniqueViolationError, isEngineDuplicateRecordEnvelope, mapDataError, sandboxBusinessMessage, isSandboxOrigin, isSandboxOwnFault } from '@objectstack/types';
+import { isUniqueViolationError, isEngineDuplicateRecordEnvelope, mapDataError, sandboxBusinessMessage, isSandboxOrigin, isSandboxOwnFault, resolveThrownHttpError, UNDECLARED_HOOK_REFUSAL_CODE } from '@objectstack/types';
 
 /**
  * import-runner — the shared row-processing core for bulk import.
@@ -527,6 +527,17 @@ export function sanitizeRowError(raw: unknown): string {
  * rule applies: a sandboxed error the door answers with a 5xx takes the door's
  * sentence. ⛔ Still not "every door 5xx": a driver error is neither a sandbox
  * origin nor a sandbox fault, and its row keeps {@link sanitizeRowError}'s text.
+ *
+ * ## An undeclared hook abort is `HOOK_ABORTED`, as on every other write door (#29978)
+ *
+ * A sandboxed body's free-text refusal that declared no code answers
+ * {@link UNDECLARED_HOOK_REFUSAL_CODE} on `/data`, `/createMany`, `/batch` and
+ * the per-object batch row. This row said `IMPORT_ROW_FAILED`, so an import
+ * client branching on `code` could not tell a business rule's refusal from any
+ * other failed row. {@link isUndeclaredHookAbort} now picks that code where
+ * the row had none of its own: a finding's code and a code the body declared
+ * still win, and every other failure keeps `IMPORT_ROW_FAILED`. The row's
+ * sentence is unchanged; it was already the hook's.
  */
 function toFailedResult(rowNo: number, err: unknown, objectName: string): ImportRowResult {
   const e = err as { code?: unknown; message?: unknown; fields?: unknown; field?: unknown } | null | undefined;
@@ -537,7 +548,7 @@ function toFailedResult(rowNo: number, err: unknown, objectName: string): Import
     if (adopted !== undefined) return adopted;
   }
   const thrownCode = isEngineDuplicateRecordEnvelope(e) ? 'UNIQUE_VIOLATION' : e?.code;
-  const code = first?.code ?? thrownCode ?? 'IMPORT_ROW_FAILED';
+  const code = first?.code ?? thrownCode ?? (isUndeclaredHookAbort(err) ? UNDECLARED_HOOK_REFUSAL_CODE : 'IMPORT_ROW_FAILED');
   const field = first?.field != null && first.field !== '' ? first.field : e?.field;
   const sandboxed = isSandboxOrigin(err) || isSandboxOwnFault(err);
   const sandboxFault = sandboxed && door.status >= 500 ? String(door.body.error) : undefined;
@@ -548,6 +559,24 @@ function toFailedResult(rowNo: number, err: unknown, objectName: string): Import
   };
 }
 
+/**
+ * [#29978] Is this row failure a hook ABORT that declared no code?
+ *
+ * Asked of `resolveThrownHttpError`, which declares
+ * {@link UNDECLARED_HOOK_REFUSAL_CODE} and answers it for exactly that throw:
+ * a sandboxed body's business refusal ({@link sandboxBusinessMessage}
+ * answered), no `code` spelled, served at a 4xx. The fallback is `400`, the
+ * status the data doors serve an undeclared sandbox refusal at, so a refusal
+ * that declared no status resolves into the 4xx band, and one that declared a
+ * 5xx stays a fault. This is the test the per-object batch row's
+ * `toRowApiError` (`@objectstack/metadata-protocol`) restates; that package
+ * sits above this one, so the resolver is the shared judge both reach.
+ * ⛔ Never a second reading of `innerMessage`, `code` or the status here.
+ */
+function isUndeclaredHookAbort(err: unknown): boolean {
+  return resolveThrownHttpError(err, 400).code === UNDECLARED_HOOK_REFUSAL_CODE;
+}
+
 /** The head of a `fields` list when it is a finding (an object); a bare name or an empty list is none. */
 function firstFinding(fields: unknown): { field?: unknown; code?: unknown } | undefined {
   const head: unknown = Array.isArray(fields) ? fields[0] : undefined;

packages/qa/dogfood/test/batch-row-hook-aborted.dogfood.test.ts

@@ -11,6 +11,12 @@
 // 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.
+//
+// [#29978] §4 drives the same fixture through both import doors
+// (`POST /data/:object/import` and its async job): the import row's `code`
+// was `IMPORT_ROW_FAILED` for the abort, from the core import runner's
+// `toFailedResult`, and is now `HOOK_ABORTED` with the hook's sentence. A
+// declared code, a crashed body and a malformed row keep their own codes.
 
 import { describe, it, expect, beforeAll, afterAll } from 'vitest';
 import { bootStack, type VerifyStack } from '@objectstack/verify';
@@ -39,7 +45,7 @@ const stack29894 = defineStack({
     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')],
+  objects: [obj('bra_ins'), obj('bra_upd'), obj('bra_del'), obj('bra_declared'), obj('bra_crash'), obj('bra_plain')],
   hooks: [
     { name: 'bra_ins_refuse', object: 'bra_ins', events: ['beforeInsert'], body: throwUndeclared },
     { name: 'bra_upd_refuse', object: 'bra_upd', events: ['beforeUpdate'], body: throwUndeclared },
@@ -50,6 +56,8 @@ const stack29894 = defineStack({
       events: ['beforeInsert'],
       body: { language: 'js', source: `throw Object.assign(new Error(${JSON.stringify(DECLARED)}), { code: 'RECORD_LOCKED' });` },
     },
+    // [#29978] A body that CRASHED rather than refused: a fault, not an abort.
+    { name: 'bra_crash_fault', object: 'bra_crash', events: ['beforeInsert'], body: { language: 'js', source: 'const v = null; v.missing();' } },
   ],
 } as any);
 
@@ -150,4 +158,55 @@ describe('[#29894] an undeclared hook abort is HOOK_ABORTED on every data door,
       expect(row?.code, JSON.stringify(body)).toBe('RECORD_LOCKED');
     });
   });
+  describe('§4 [#29978] the import row answers the same code, on the sync door and the async job', () => {
+    type Row = { row: number; ok: boolean; action: string; code?: string; error?: string; field?: string };
+    const doors = {
+      sync: async (object: string, rows: object[]): Promise<Row[]> => {
+        const res = await stack.apiAs(admin, 'POST', `/data/${object}/import`, { format: 'json', rows });
+        expect(res.status, await res.clone().text()).toBe(200);
+        return ((await res.json()) as { results: Row[] }).results;
+      },
+      jobs: async (object: string, rows: object[]): Promise<Row[]> => {
+        const res = await stack.apiAs(admin, 'POST', `/data/${object}/import/jobs`, { format: 'json', rows });
+        expect(res.status, await res.clone().text()).toBe(201);
+        const { jobId } = (await res.json()) as { jobId: string };
+        for (let i = 0; i < 400; i++) {
+          const body: any = await (await stack.apiAs(admin, 'GET', `/data/import/jobs/${jobId}/results`)).json();
+          if (body.status === 'succeeded' || body.status === 'failed') return body.results;
+          expect(['pending', 'running'], JSON.stringify(body)).toContain(body.status);
+          await new Promise((r) => setTimeout(r, 25));
+        }
+        throw new Error(`import job ${jobId} did not finish`);
+      },
+    };
+
+    for (const door of ['sync', 'jobs'] as const) {
+      it(`${door}: an undeclared abort is HOOK_ABORTED with the hook's sentence`, async () => {
+        const [row] = await doors[door]('bra_ins', [{ name: 'x' }]);
+        expect(row, JSON.stringify(row)).toMatchObject({ ok: false, action: 'failed', error: SENTENCE });
+        expect(row!.code, `#29978: the import row answers the code every other write door gives the same abort: ${JSON.stringify(row)}`).toBe('HOOK_ABORTED');
+      });
+
+      it(`${door}: CONTROL a code the hook declared still wins`, async () => {
+        const [row] = await doors[door]('bra_declared', [{ name: 'x' }]);
+        expect(row, JSON.stringify(row)).toMatchObject({ ok: false, error: DECLARED, code: 'RECORD_LOCKED' });
+      });
+
+      it(`${door}: CONTROL a body that crashed keeps IMPORT_ROW_FAILED`, async () => {
+        const [row] = await doors[door]('bra_crash', [{ name: 'x' }]);
+        expect(row, JSON.stringify(row)).toMatchObject({ ok: false, action: 'failed', code: 'IMPORT_ROW_FAILED' });
+      });
+
+      it(`${door}: CONTROL a malformed row keeps its own cell code`, async () => {
+        const [row] = await doors[door]('bra_plain', [{ name: '' }]);
+        expect(row, JSON.stringify(row)).toMatchObject({ ok: false, action: 'failed', code: 'required', field: 'name' });
+      });
+    }
+
+    it('nothing was written by the refused imports', async () => {
+      expect(await ql.find('bra_ins', { where: {}, ...SYS })).toHaveLength(0);
+      expect(await ql.find('bra_declared', { where: {}, ...SYS })).toHaveLength(0);
+      expect(await ql.find('bra_crash', { where: {}, ...SYS })).toHaveLength(0);
+    });
+  });
 });