diff --git a/.changeset/29422-hook-write-stale-summary-refusal.md b/.changeset/29422-hook-write-stale-summary-refusal.md new file mode 100644 index 00000000000..58b25293e89 --- /dev/null +++ b/.changeset/29422-hook-write-stale-summary-refusal.md @@ -0,0 +1,21 @@ +--- +'@objectstack/objectql': patch +--- + +fix(objectql)!: a hook's own write's stale-roll-up refusal (`ERR_SUMMARY_RECOMPUTE`) never answers for the write the hook is bound to, and the unlanded-write refusal leads with the outer write and declares `500 INTERNAL_ERROR` + +Clause-②: no (narrowing) +Metadata-protocol: none + +Narrowing note: a write whose hook let its own write's stale-roll-up refusal escape used to answer success with the hook's record. It now refuses, and an aborting `after*` hook's unit rolls both rows back. The action door also answers these refusals `500` instead of `400`. Hence the `!` in the title, the full dogfood run and the seat's contract review. The claim was amended by seat 5 on #29422. + + + +**BREAKING** for these answers only. `ERR_SUMMARY_RECOMPUTE` means "this write is stored, only a parent roll-up recompute failed". The data doors (REST, MCP) and the action, sandbox and flow writers answer it as the write's own success, using the refusal's `written` as the result. + +- **A hook's own write.** Suppose a hook writes through `ctx.api` and lets that write's refusal escape. The refusal reached the caller of the write the hook is bound to, carrying the HOOK's record. So `POST /api/v1/data/:object` answered `201` with the id and record of the row the hook wrote. Under an aborting `after*` hook the write's own unit then committed both rows. Now the refusal is converted where it leaves the handler, for `before*` and `after*` hooks alike, and for `beforeFind` / `afterFind` too: a find hook whose own write refuses now fails the read with the same declared `500` (the read failed before as well, only its sentence and status were inferred). The write refuses with an internal failure that names it, the hook and the stale summary, and keeps the original refusal as `cause`. An aborting `after*` hook's unit rolls both rows back, as `HookSchema.onError` `abort` says. With no unit (a code-registered hook, a driver without transactions, a caller's own unit), the message says that what was written before that point stays written. Every other error a hook throws passes through unchanged, and a hook declared `onError: 'log'` still does not fail the write. +- **The refusal's lead on a nested cascade.** When a grandchild's delete refused, the parent's caller read `The delete of "" did not complete`. Each outer cascade walk and each hook now re-leads the refusal, so the caller reads the write it asked for. `cause` stays the original refusal at every depth. +- **A declared status.** The refusal a write answers when a nested write's refusal stopped it (a cascade delete, a rolled-back transaction, and now a hook) declares `status: 500` and `code: 'INTERNAL_ERROR'`. It no longer carries no `code`, which supersedes the sentence of the pending `@objectstack/objectql` changeset that said it did. For the refusal itself, the REST data door's answer is byte-identical: `500 { error: "Internal server error", code: "INTERNAL_ERROR" }`, now declared instead of reached by falling through every message heuristic. **The action door now answers these refusals `500 INTERNAL_ERROR`, not `400`**, with the same sentence. **A converted refusal relayed out of a sandboxed hook body** (its `ctx.api` cascade delete or transaction was stopped, and the body let the error escape) **now answers `500 INTERNAL_ERROR` at the REST data door, where it answered `400` with the inner sentence.** The dispatcher data door and the MCP tools keep status `500` and code `INTERNAL_ERROR` for these refusals, but now withhold the prose, as they do for any declared 5xx. +- **A failed rollback.** When `ObjectQL.transaction`'s rollback itself threw, the refusal still said "The transaction rolled back, so nothing written inside it was stored". It now says that the rollback failed and that what was written inside it may or may not be stored. + +**The remedy.** Fix the summary field's `summaryOperations` (the engine's warning names why the recompute was refused), then retry. A hook that can tolerate a stale summary on its own write catches `ERR_SUMMARY_RECOMPUTE` itself. diff --git a/packages/objectql/src/engine-cascade-summary-rollback.test.ts b/packages/objectql/src/engine-cascade-summary-rollback.test.ts index 47b6b981f47..5de001abb3d 100644 --- a/packages/objectql/src/engine-cascade-summary-rollback.test.ts +++ b/packages/objectql/src/engine-cascade-summary-rollback.test.ts @@ -144,7 +144,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren const { engine, driver } = await makeEngine(true); const err: any = await engine.delete('acct', { where: { id: 'a1' } }).then(() => null, (e: unknown) => e); expect(err).toBeInstanceOf(Error); - expect(err.code).toBeUndefined(); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); expect(err.written).toBeUndefined(); expect(driver.rolledBack).toHaveLength(1); @@ -156,7 +157,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren const { engine, driver } = await makeEngine(false); const err: any = await engine.delete('acct', { where: { id: 'a1' } }).then(() => null, (e: unknown) => e); expect(err).toBeInstanceOf(Error); - expect(err.code).toBeUndefined(); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); expect(ids(driver.rowsOf('acct'))).toEqual(['a1']); // The dependent the walk reached first is gone: nothing can roll it back. @@ -167,7 +169,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren const { engine, driver } = await makeEngine(false); const err: any = await engine.delete('acct', { where: { name: 'A' }, multi: true }).then(() => null, (e: unknown) => e); expect(err).toBeInstanceOf(Error); - expect(err.code).toBeUndefined(); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); expect(ids(driver.rowsOf('acct'))).toEqual(['a1']); }); @@ -178,7 +181,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren .transaction(async (trx: any) => engine.delete('line', { where: { id: 'l1' }, context: trx })) .then(() => null, (e: unknown) => e); expect(err).toBeInstanceOf(Error); - expect(err.code).toBeUndefined(); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); expect(driver.rolledBack).toHaveLength(1); expect(ids(driver.rowsOf('line'))).toEqual(['l1', 'l2']); @@ -191,7 +195,8 @@ describe('a cascade dependent\'s ERR_SUMMARY_RECOMPUTE never answers for a paren .transaction(async (trx: any) => trx.object('line').deleteById('l1')) .then(() => null, (e: unknown) => e); expect(err).toBeInstanceOf(Error); - expect(err.code).toBeUndefined(); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); expect(driver.rolledBack).toHaveLength(1); expect(ids(driver.rowsOf('line'))).toEqual(['l1', 'l2']); diff --git a/packages/objectql/src/engine-hook-stale-summary-refusal.test.ts b/packages/objectql/src/engine-hook-stale-summary-refusal.test.ts new file mode 100644 index 00000000000..c4cd1e37c67 --- /dev/null +++ b/packages/objectql/src/engine-hook-stale-summary-refusal.test.ts @@ -0,0 +1,221 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A hook's own `ctx.api` write that raises the stale-roll-up refusal never + * answers for the write the hook is bound to. + * + * `SummaryRecomputeError` (`ERR_SUMMARY_RECOMPUTE`) means "this write is + * stored, only a parent's roll-up recompute failed". It carries the write's + * result as `written`, and every answering surface turns it into that write's + * own success. A hook that writes through `ctx.api` makes a write of its own. + * When that write raised the refusal and the handler let it escape, the refusal + * reached the OUTER write's caller unchanged. So the caller read the hook's + * record as the result of its own write: measured through the REST data door + * before this file, `POST /data/case` answered `201` with the id and record of + * the row the hook wrote. Under an `after*` hook's unit (`inAfterAbortUnit`) the + * unit then committed, because the refusal says "written". + * + * Pinned here, at the one seam a handler's error leaves through + * (`triggerHooks`): + * - an aborting `afterInsert` / `afterUpdate` / `afterDelete` hook: the outer + * write refuses with an error that is NOT the stale-roll-up refusal (a + * declared `500 INTERNAL_ERROR`, no `written`, the refusal kept as `cause`), + * leading with the outer write, and its unit rolls both rows back; + * - a code-registered hook (no unit opens): the same refusal, whose text says + * what stays written, and both rows are stored; + * - a `before*` hook: the same refusal, and the outer row was never written. + * + * Controls: a hook declared `onError: 'log'` does not fail the outer write; the + * outer write's OWN refusal still answers `ERR_SUMMARY_RECOMPUTE` carrying its + * own result (and its unit still commits); any other error a hook throws passes + * through unchanged. + * + * The driver rolls back for real (a snapshot per transaction), and refuses + * every write to `bucket` while `refuseBucket` is set: that is the roll-up + * recompute's parent update, refused without a transient message, so no retry + * runs. + */ + +import { describe, it, expect } from 'vitest'; +import type { Hook } from '@objectstack/spec/data'; +import { ObjectQL } from './engine.js'; +import { bindHooksToEngine } from './hook-binder.js'; + +type Row = Record; + +function makeDriver() { + let stores = new Map>(); + const storeFor = (o: string) => stores.get(o) ?? (stores.set(o, new Map()), stores.get(o)!); + const matches = (row: Row, where: any) => + !where || Object.entries(where).every(([k, v]) => k.startsWith('$') || row[k] === v); + const begun: unknown[] = []; + const committed: unknown[] = []; + const rolledBack: unknown[] = []; + const snapshots = new Map(); + const state = { refuseBucket: false }; + let n = 0; + const driver: any = { + name: 'primary', version: '0.0.0', supports: {}, begun, committed, rolledBack, state, + rowsOf: (o: string) => Array.from(storeFor(o).values()), + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async syncSchema() {}, + async find(o: string, ast: any) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + }, + async findOne(o: string, ast: any) { return (await this.find(o, ast))[0] ?? null; }, + async create(o: string, data: Row) { + const row = { ...data, id: (data.id as string) ?? `${o}_${++n}` }; + storeFor(o).set(String(row.id), row); + return row; + }, + async update(o: string, id: string, data: Row) { + if (state.refuseBucket && o === 'bucket') throw new Error('bucket store refused the write'); + const row = { ...storeFor(o).get(String(id)), ...data, id }; + storeFor(o).set(String(id), row); + return row; + }, + async delete(o: string, id: string) { return storeFor(o).delete(String(id)); }, + async count(o: string, ast: any) { return (await this.find(o, ast)).length; }, + async bulkCreate(o: string, rows: Row[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { + const handle = { trx: begun.length + 1 }; + snapshots.set(handle, new Map(Array.from(stores, ([o, s]) => [o, new Map(Array.from(s, ([k, v]) => [k, { ...v }]))]))); + begun.push(handle); + return handle; + }, + async commit(h: unknown) { snapshots.delete(h); committed.push(h); }, + async rollback(h: unknown) { stores = snapshots.get(h) ?? stores; rolledBack.push(h); }, + }; + return driver; +} + +const text = (name: string) => ({ name, type: 'text' }); +/** The hook's own write: a `line`, whose roll-up into `bucket` the driver refuses. */ +const writeLine = (ctx: any) => ctx.api.object('line').insert({ name: `from-${ctx.event}`, bucket: 'b1' }); +const hook = (events: string[], handler: (ctx: any) => unknown, extra: Partial = {}) => + ({ name: 'line_writer', object: 'case', events, handler, ...extra }) as unknown as Hook; + +async function makeEngine(hooks: Hook[]) { + const log = () => {}; + const engine = new ObjectQL({ logger: { debug: log, info: log, warn: log, error: log } } as any); + (engine as any).summaryRetryOptions = { sleep: async () => {}, backoffBaseMs: 0 }; + const driver = makeDriver(); + engine.registerDriver(driver, true); + await engine.init(); + engine.registry.registerObject({ name: 'case', label: 'case', fields: { id: { ...text('id'), primaryKey: true }, name: text('name') } } as any, '__test__'); + engine.registry.registerObject({ + name: 'bucket', label: 'bucket', + fields: { + id: { ...text('id'), primaryKey: true }, name: text('name'), + line_count: { name: 'line_count', type: 'summary', summaryOperations: { object: 'line', function: 'count' } }, + }, + } as any, '__test__'); + engine.registry.registerObject({ + name: 'line', label: 'line', + fields: { id: { ...text('id'), primaryKey: true }, name: text('name'), bucket: { name: 'bucket', type: 'master_detail', reference: 'bucket' } }, + } as any, '__test__'); + await engine.insert('bucket', { id: 'b1', name: 'B' }); + await engine.insert('case', { id: 'c0', name: 'existing' }); + bindHooksToEngine(engine, hooks, { packageId: 'app:t' }); + driver.begun.length = 0; + driver.committed.length = 0; + driver.rolledBack.length = 0; + driver.state.refuseBucket = true; + return { engine, driver }; +} + +const refusal = (p: Promise): Promise => p.then(() => null, (e: unknown) => e); +const names = (rows: Row[]) => rows.map((r) => r.name).sort(); + +/** The converted refusal: an internal error naming the OUTER write, the hook write's refusal kept as `cause`. */ +function expectHookStopped(err: any, lead: string) { + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); + expect(err.written).toBeUndefined(); + expect(err.failures).toBeUndefined(); + expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(err.cause?.written?.name).toMatch(/^from-/); + expect(err.message.startsWith(lead), err.message).toBe(true); + expect(err.message).toContain('bucket.line_count'); +} + +describe('a hook\'s own ctx.api write that raises ERR_SUMMARY_RECOMPUTE never answers for the write it is bound to', () => { + it('afterInsert (aborting, unit open): the insert refuses naming itself, and the unit rolls both rows back', async () => { + const { engine, driver } = await makeEngine([hook(['afterInsert'], writeLine)]); + const err = await refusal(engine.insert('case', { name: 'c-1' })); + expectHookStopped(err, 'The insert of "case" did not complete: its afterInsert hook "line_writer" stopped.'); + expect(driver.begun).toHaveLength(1); + expect(driver.committed).toHaveLength(0); + expect(driver.rolledBack).toHaveLength(1); + expect(names(driver.rowsOf('case'))).toEqual(['existing']); + expect(driver.rowsOf('line')).toHaveLength(0); + }); + + it('afterUpdate and afterDelete (aborting, unit open): refused naming the outer write, the stored row unchanged', async () => { + const upd = await makeEngine([hook(['afterUpdate'], writeLine)]); + const updErr = await refusal(upd.engine.update('case', { id: 'c0', name: 'renamed' })); + expectHookStopped(updErr, 'The update of "case" did not complete: its afterUpdate hook "line_writer" stopped.'); + expect(upd.driver.rolledBack).toHaveLength(1); + expect(names(upd.driver.rowsOf('case'))).toEqual(['existing']); + expect(upd.driver.rowsOf('line')).toHaveLength(0); + + const del = await makeEngine([hook(['afterDelete'], writeLine)]); + const delErr = await refusal(del.engine.delete('case', { where: { id: 'c0' } })); + expectHookStopped(delErr, 'The delete of "case" did not complete: its afterDelete hook "line_writer" stopped.'); + expect(del.driver.rolledBack).toHaveLength(1); + expect(names(del.driver.rowsOf('case'))).toEqual(['existing']); + expect(del.driver.rowsOf('line')).toHaveLength(0); + }); + + it('a code-registered afterInsert (no unit opens): the same refusal, saying what stays written, and both rows stored', async () => { + const { engine, driver } = await makeEngine([]); + engine.registerHook('afterInsert', writeLine, { object: 'case', hookName: 'code_line_writer' }); + const err = await refusal(engine.insert('case', { name: 'c-1' })); + expectHookStopped(err, 'The insert of "case" did not complete: its afterInsert hook "code_line_writer" stopped.'); + expect(err.message).toContain( + 'Under a transaction that rolls back nothing was written; otherwise what was written before that point stays written.', + ); + expect(driver.begun).toHaveLength(0); + expect(names(driver.rowsOf('case'))).toEqual(['c-1', 'existing']); + expect(names(driver.rowsOf('line'))).toEqual(['from-afterInsert']); + }); + + it('beforeInsert: the same refusal, and the outer row was never written', async () => { + const { engine, driver } = await makeEngine([hook(['beforeInsert'], writeLine)]); + const err = await refusal(engine.insert('case', { name: 'c-1' })); + expectHookStopped(err, 'The insert of "case" did not complete: its beforeInsert hook "line_writer" stopped.'); + expect(names(driver.rowsOf('case'))).toEqual(['existing']); + // No unit opens for a before* hook, so the hook's own write stays, as the text says. + expect(names(driver.rowsOf('line'))).toEqual(['from-beforeInsert']); + }); + + it('CONTROL onError: \'log\': the hook\'s refusal is logged, and the outer insert answers its own row', async () => { + const { engine, driver } = await makeEngine([hook(['afterInsert'], writeLine, { onError: 'log' })]); + const row: any = await engine.insert('case', { name: 'c-1' }); + expect(row?.name).toBe('c-1'); + expect(names(driver.rowsOf('case'))).toEqual(['c-1', 'existing']); + expect(names(driver.rowsOf('line'))).toEqual(['from-afterInsert']); + }); + + it('CONTROL the outer write\'s OWN refusal still answers ERR_SUMMARY_RECOMPUTE with its own result, and its unit commits', async () => { + const { engine, driver } = await makeEngine([ + { name: 'observe', object: 'line', events: ['afterInsert'], handler: () => {} } as unknown as Hook, + ]); + const err = await refusal(engine.insert('line', { name: 'direct', bucket: 'b1' })); + expect(err?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(err.written?.name).toBe('direct'); + expect(driver.begun).toHaveLength(1); + expect(driver.committed).toHaveLength(1); + expect(driver.rolledBack).toHaveLength(0); + expect(names(driver.rowsOf('line'))).toEqual(['direct']); + }); + + it('CONTROL any other error a hook throws passes through unchanged', async () => { + const boom = new Error('refused by the hook'); + const { engine } = await makeEngine([hook(['afterInsert'], () => { throw boom; })]); + expect(await refusal(engine.insert('case', { name: 'c-1' }))).toBe(boom); + }); +}); diff --git a/packages/objectql/src/engine-unlanded-refusal-lead.test.ts b/packages/objectql/src/engine-unlanded-refusal-lead.test.ts new file mode 100644 index 00000000000..6eba2c26fd9 --- /dev/null +++ b/packages/objectql/src/engine-unlanded-refusal-lead.test.ts @@ -0,0 +1,225 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The refusal a write answers when a nested write's stale-roll-up refusal + * stopped it (`summary-errors.ts`, #29291) leads with the write its caller + * asked for, and never claims a rollback that failed. + * + * 1. A nested cascade: deleting an account walks its lines, each line's + * delete walks its subs, and a sub's delete refuses on recompute (its + * roll-up into `bucket` cannot be recomputed). The line's walk converts + * first, naming the line; the account's walk then received a code-less + * error and passed it through, so the account's caller read + * `The delete of "line" did not complete`. Now each outer walk re-leads + * it, at every depth, and `cause` stays the sub's own refusal. + * 2. `ObjectQL.transaction` swallows a failing rollback so the original error + * surfaces. It still converted that error to "The transaction rolled back, + * so nothing written inside it was stored". Now, when the rollback threw, + * the lead says that nothing is known about what is stored. + * + * Controls: a delete of the sub itself still answers `ERR_SUMMARY_RECOMPUTE` + * with its own result; a transaction whose rollback succeeds still says it + * rolled back. + * + * The driver rolls back for real (a snapshot per transaction), refuses every + * write to `bucket` while `refuseBucket` is set (the roll-up recompute's parent + * update, refused without a transient message, so no retry runs), and makes + * `rollback()` throw while `failRollback` is set. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { SummaryRecomputeError, cascadeStoppedRefusal, hookStoppedRefusal } from './summary-errors.js'; + +type Row = Record; + +function matches(row: Row, where: any): boolean { + if (!where || typeof where !== 'object') return true; + return Object.entries(where).every(([k, v]) => { + if (k === '$and') return (v as any[]).every((w) => matches(row, w)); + if (k === '$or') return (v as any[]).some((w) => matches(row, w)); + if (k.startsWith('$')) return true; + if (v && typeof v === 'object' && !Array.isArray(v)) { + const ops = v as Record; + if ('$in' in ops) return (ops.$in as unknown[]).map(String).includes(String(row[k])); + if ('$eq' in ops) return String(row[k]) === String(ops.$eq); + if ('$ne' in ops) return String(row[k]) !== String(ops.$ne); + return true; + } + return row[k] === v || (row[k] != null && v != null && String(row[k]) === String(v)); + }); +} + +function makeDriver() { + let stores = new Map>(); + const storeFor = (o: string) => stores.get(o) ?? (stores.set(o, new Map()), stores.get(o)!); + const rolledBack: unknown[] = []; + const snapshots = new Map(); + const state = { refuseBucket: false, failRollback: false }; + let n = 0; + const driver: any = { + name: 'primary', version: '0.0.0', supports: {}, rolledBack, state, + rowsOf: (o: string) => Array.from(storeFor(o).values()), + async connect() {}, async disconnect() {}, async checkHealth() { return true; }, async execute() { return null; }, + async syncSchema() {}, + async find(o: string, ast: any) { + const rows = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + return typeof ast?.limit === 'number' ? rows.slice(0, ast.limit) : rows; + }, + async findOne(o: string, ast: any) { return (await this.find(o, ast))[0] ?? null; }, + async create(o: string, data: Row) { + const row = { ...data, id: (data.id as string) ?? `${o}_${++n}` }; + storeFor(o).set(String(row.id), row); + return row; + }, + async update(o: string, id: string, data: Row) { + if (state.refuseBucket && o === 'bucket') throw new Error('bucket store refused the write'); + const row = { ...storeFor(o).get(String(id)), ...data, id }; + storeFor(o).set(String(id), row); + return row; + }, + async delete(o: string, id: string) { return storeFor(o).delete(String(id)); }, + async deleteMany(o: string, ast: any) { + const doomed = Array.from(storeFor(o).values()).filter((r) => matches(r, ast?.where)); + for (const r of doomed) storeFor(o).delete(String(r.id)); + return doomed.length; + }, + async count(o: string, ast: any) { return (await this.find(o, ast)).length; }, + async bulkCreate(o: string, rows: Row[]) { return Promise.all(rows.map((r) => this.create(o, r))); }, + async bulkUpdate() { return []; }, async bulkDelete() {}, + async beginTransaction() { + const handle = { trx: snapshots.size + 1 }; + snapshots.set(handle, new Map(Array.from(stores, ([o, s]) => [o, new Map(Array.from(s, ([k, v]) => [k, { ...v }]))]))); + return handle; + }, + async commit(h: unknown) { snapshots.delete(h); }, + async rollback(h: unknown) { + if (state.failRollback) throw new Error('connection lost during rollback'); + stores = snapshots.get(h) ?? stores; + rolledBack.push(h); + }, + }; + return driver; +} + +async function makeEngine() { + const log = () => {}; + const engine = new ObjectQL({ logger: { debug: log, info: log, warn: log, error: log } } as any); + (engine as any).summaryRetryOptions = { sleep: async () => {}, backoffBaseMs: 0 }; + const driver = makeDriver(); + engine.registerDriver(driver, true); + await engine.init(); + const text = (name: string) => ({ name, type: 'text' }); + const id = { ...text('id'), primaryKey: true }; + engine.registry.registerObject({ name: 'acct', label: 'acct', fields: { id, name: text('name') } } as any, '__test__'); + engine.registry.registerObject({ + name: 'bucket', label: 'bucket', + fields: { id, name: text('name'), sub_count: { name: 'sub_count', type: 'summary', summaryOperations: { object: 'sub', function: 'count' } } }, + } as any, '__test__'); + engine.registry.registerObject({ + name: 'line', label: 'line', + fields: { id, name: text('name'), acct: { name: 'acct', type: 'master_detail', reference: 'acct', deleteBehavior: 'cascade' } }, + } as any, '__test__'); + engine.registry.registerObject({ + name: 'sub', label: 'sub', + fields: { + id, name: text('name'), + line: { name: 'line', type: 'master_detail', reference: 'line', deleteBehavior: 'cascade' }, + bucket: { name: 'bucket', type: 'lookup', reference: 'bucket' }, + }, + } as any, '__test__'); + await engine.insert('acct', { id: 'a1', name: 'A' }); + await engine.insert('bucket', { id: 'b1', name: 'B' }); + await engine.insert('line', { id: 'l1', name: 'L1', acct: 'a1' }); + await engine.insert('sub', { id: 's1', name: 'S1', line: 'l1', bucket: 'b1' }); + driver.rolledBack.length = 0; + driver.state.refuseBucket = true; + return { engine, driver }; +} + +const refusal = (p: Promise): Promise => p.then(() => null, (e: unknown) => e); +const ids = (rows: Row[]) => rows.map((r) => r.id).sort(); + +describe('the unlanded-write refusal leads with the write its caller asked for', () => { + it('a three-level cascade: the account\'s caller reads the account, and `cause` is the sub\'s own refusal', async () => { + const { engine, driver } = await makeEngine(); + const err = await refusal(engine.delete('acct', { where: { id: 'a1' } })); + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); + expect(err.message.startsWith('The delete of "acct" did not complete: its cascade stopped'), err.message).toBe(true); + expect(err.message).not.toContain('"line"'); + expect(err.message).toContain('bucket.sub_count'); + expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(err.cause?.failures?.[0]?.childObject).toBe('sub'); + expect(ids(driver.rowsOf('acct'))).toEqual(['a1']); + expect(ids(driver.rowsOf('line'))).toEqual(['l1']); + expect(ids(driver.rowsOf('sub'))).toEqual(['s1']); + }); + + it('the middle level\'s own caller still reads the line', async () => { + const { engine } = await makeEngine(); + const err = await refusal(engine.delete('line', { where: { id: 'l1' } })); + expect(err.message.startsWith('The delete of "line" did not complete'), err.message).toBe(true); + expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + }); + + it('every outer conversion re-leads, whatever the depth, and keeps the original refusal as cause', () => { + const original = new SummaryRecomputeError([{ childObject: 'sub', parentObject: 'bucket', parentId: 'b1', field: 'sub_count', error: null }], true); + const inner = cascadeStoppedRefusal(original, 'line') as any; + const outer = cascadeStoppedRefusal(inner, 'acct') as any; + const hooked = hookStoppedRefusal(outer, 'afterInsert', 'case', 'cleanup') as any; + expect(outer.message.startsWith('The delete of "acct" did not complete'), outer.message).toBe(true); + expect(hooked.message.startsWith('The insert of "case" did not complete: its afterInsert hook "cleanup" stopped.'), hooked.message).toBe(true); + for (const e of [inner, outer, hooked]) { + expect(e.cause).toBe(original); + expect(e.code).toBe('INTERNAL_ERROR'); + expect(e.status).toBe(500); + expect(e.message).toContain('bucket.sub_count'); + } + }); + + it('CONTROL a code-less error that is no conversion passes every conversion unchanged', () => { + const other = new Error('something else'); + expect(cascadeStoppedRefusal(other, 'acct')).toBe(other); + expect(hookStoppedRefusal(other, 'afterInsert', 'case', undefined)).toBe(other); + }); + + it('CONTROL the sub\'s own delete landed: ERR_SUMMARY_RECOMPUTE, carrying its result', async () => { + const { engine, driver } = await makeEngine(); + const err = await refusal(engine.delete('sub', { where: { id: 's1' } })); + expect(err?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(err.written).toBe(true); + expect(driver.rowsOf('sub')).toHaveLength(0); + }); +}); + +describe('ObjectQL.transaction never claims "rolled back" when its rollback threw', () => { + const escapeRefusal = (engine: ObjectQL) => + refusal(engine.transaction(async (trx: any) => engine.delete('sub', { where: { id: 's1' }, context: trx }))); + + it('the rollback throws: refused with a lead that says nothing is known, the refusal as cause', async () => { + const { engine, driver } = await makeEngine(); + driver.state.failRollback = true; + const err = await escapeRefusal(engine); + expect(err).toBeInstanceOf(Error); + expect(err.code).toBe('INTERNAL_ERROR'); + expect(err.status).toBe(500); + expect(err.written).toBeUndefined(); + expect(err.message.startsWith( + 'The transaction did not complete, and its rollback failed too, so what was written inside it may or may not be stored.', + ), err.message).toBe(true); + expect(err.message).not.toContain('rolled back'); + expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(driver.rolledBack).toHaveLength(0); + }); + + it('CONTROL the rollback succeeds: "The transaction rolled back", and nothing is stored', async () => { + const { engine, driver } = await makeEngine(); + const err = await escapeRefusal(engine); + expect(err.message.startsWith('The transaction rolled back, so nothing written inside it was stored.'), err.message).toBe(true); + expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE'); + expect(driver.rolledBack).toHaveLength(1); + expect(ids(driver.rowsOf('sub'))).toEqual(['s1']); + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 93b1fa9ba76..8ad5ac844ae 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -201,7 +201,7 @@ import { } from '@objectstack/core'; import { WriteEpoch, isWriteEpochOperation } from './write-epoch.js'; import { bridgeAuthzInvalidation } from './authz-invalidation-bridge.js'; -import { SummaryRecomputeError, cascadeStoppedRefusal, rolledBackRefusal, type SummaryRecomputeFailure } from './summary-errors.js'; +import { SummaryRecomputeError, cascadeStoppedRefusal, hookStoppedRefusal, rollbackFailedRefusal, rolledBackRefusal, type SummaryRecomputeFailure } from './summary-errors.js'; import { TransactionUnsupportedError } from './transaction-errors.js'; import { aggregateSummaryValue, @@ -6008,7 +6008,13 @@ export class ObjectQL implements IObjectQLEngine { this.logger.debug('Skipping metadata-bound hook (skipAutomations)', { event, hook: entry.hookName }); continue; } - await entry.handler(context); + // [#29422] A stale-roll-up refusal a hook's own `ctx.api` write raised + // describes THAT write, not this one: `hookStoppedRefusal`. + try { + await entry.handler(context); + } catch (e) { + throw hookStoppedRefusal(e, event, context.object, entry.hookName); + } } } @@ -6056,7 +6062,9 @@ export class ObjectQL implements IObjectQLEngine { * (`transaction()` covers that one alone and would refuse the write), or when * that driver has no transactions (ADR-0119 D1; asking would only warn). * `SummaryRecomputeError` means "the records WERE written": it is rethrown - * after the commit, never through the rollback. + * after the commit, never through the rollback. Only the write's OWN refusal + * arrives here: one a hook's `ctx.api` write raised was already converted + * where it left the handler (#29422, `hookStoppedRefusal`), so it rolls back. */ private opensAfterAbortUnit(event: 'afterInsert' | 'afterUpdate' | 'afterDelete', object: string, context: ExecutionContext | undefined): boolean { const threaded = context?.transaction; @@ -6322,7 +6330,12 @@ export class ObjectQL implements IObjectQLEngine { } as unknown as HookContext; for (const entry of flagged) { if (skipAutomations && entry.meta) continue; - await entry.handler(wholeOpCtx); + // [#29422] As in `triggerHooks`. + try { + await entry.handler(wholeOpCtx); + } catch (e) { + throw hookStoppedRefusal(e, event, object, entry.hookName); + } } // `input` is identity-shared with the batch context, so the observation // covers everything the handlers touched. @@ -21914,18 +21927,22 @@ export class ObjectQL implements IObjectQLEngine { else if (drv.commitTransaction) await drv.commitTransaction(trx); return result; } catch (err) { + let rollbackFailed = false; try { if (drv.rollback) await drv.rollback(trx); else if (drv.rollbackTransaction) await drv.rollbackTransaction(trx); } catch { // swallow rollback failures so the original error surfaces + rollbackFailed = true; } // [#21274] A boundary outside the middleware seam: a commit failure, or // a callback that reached the driver through the transaction handle. // An error that already crossed an engine boundary inside the callback // comes back as the same reference. A stale-roll-up refusal ("stored") // is no longer true once this rollback has run: `rolledBackRefusal`. - throw redactPropagatedDriverFault(rolledBackRefusal(err)); + // [#29421] Nor is "rolled back" once the rollback threw: + // `rollbackFailedRefusal`. + throw redactPropagatedDriverFault(rollbackFailed ? rollbackFailedRefusal(err) : rolledBackRefusal(err)); } } diff --git a/packages/objectql/src/summary-errors.ts b/packages/objectql/src/summary-errors.ts index 6634bd5480a..cc64c15be75 100644 --- a/packages/objectql/src/summary-errors.ts +++ b/packages/objectql/src/summary-errors.ts @@ -79,8 +79,8 @@ export class SummaryRecomputeError extends Error { * * That refusal says "this write is stored, only a roll-up recompute failed", * and every answering surface turns it into the write's own success. It is - * true for the write that raised it, and only while that write stands. Two - * engine units carry a nested write's refusal past the point where it stops + * true for the write that raised it, and only while that write stands. Three + * engine seams carry a nested write's refusal past the point where it stops * being true: * * - a delete's cascade walk deletes (or clears) each dependent through the @@ -88,31 +88,71 @@ export class SummaryRecomputeError extends Error { * refusal stops the walk, so the parent's row is never removed — and under * the `'atomic'` plan the transaction then rolls the dependent back too; * - a `transaction()` whose callback lets the refusal escape rolls back every - * write in it, the refusing one included. + * write in it, the refusing one included; + * - a hook that writes through `ctx.api` and lets that write's refusal escape + * stops the write it is hooked to, whose caller would otherwise read the + * hook's write as its own. * - * Either way the caller's own write did not land, so it is told so: an error - * with no `code` (an internal failure, which is what an unrecomputable roll-up - * is), the original refusal kept as `cause` for the operator. Every other error + * Either way the caller's own write did not land, so it is told so: an + * internal failure, which is what an unrecomputable roll-up is + * ({@link UnlandedWriteRefusal}), the original refusal kept as `cause` for the + * operator. Every other error * passes through unchanged, and so does the refusal anywhere it is still true: * the dependent's own delete, a write outside any unit, a transaction that * catches it and commits, a joined transaction whose owner decides. * * Matched by `code`, as every reader of the refusal matches it. + * + * [#29421] A conversion that meets an EARLIER conversion re-leads it when + * `relead` is set: a nested cascade (a grandchild refuses, the child's walk + * converts naming the child, then the parent's walk receives that) or a hook + * whose own write was such a delete. The caller of the outer write is told + * about the outer write, and `cause` stays the original refusal, so the + * summaries named are the same at every level. A transaction does not re-lead: + * a conversion that escapes it already names the write that did not complete. */ -function unlandedWriteRefusal(e: unknown, lead: string): unknown { - if (!(e instanceof Error) || (e as { code?: unknown }).code !== SUMMARY_RECOMPUTE_CODE) return e; - const failures = (e as { failures?: unknown }).failures; +function unlandedWriteRefusal(e: unknown, lead: string, relead = false): unknown { + const original = relead && e instanceof UnlandedWriteRefusal ? e.cause : e; + if (!(original instanceof Error) || (original as { code?: unknown }).code !== SUMMARY_RECOMPUTE_CODE) return e; + const failures = (original as { failures?: unknown }).failures; const named = Array.isArray(failures) ? [...new Set((failures as SummaryRecomputeFailure[]).map((f) => `${f?.parentObject}.${f?.field}`))] : []; const summaries = named.length > 0 ? `the roll-up summary ${named.join(', ')}` : 'a parent roll-up summary'; - const refusal = new Error( + return new UnlandedWriteRefusal( `${lead} A write inside it was stored, then ${summaries} could not be recomputed after it, and that ` + "refusal stopped it. Fix the summary field's `summaryOperations` (the engine's warning names why the " + 'recompute was refused), then retry.', + original, ); - (refusal as Error & { cause?: unknown }).cause = e; - return refusal; +} + +/** + * [#29421] What {@link unlandedWriteRefusal} answers: an internal failure, the + * original refusal as `cause`. + * + * It DECLARES that it is one, `status: 500` and the generic ADR-0112 + * `INTERNAL_ERROR`, so every door reads it structurally. Before, it declared + * nothing. The data door then reached its 500 only because the sentence + * missed every message heuristic in `classifyDataError` (the double quotes in + * the leads dodge the unknown-object one), and the action door answered it as + * a 400 refusal. Measured: the data door's body is byte-identical (`500 + * { error, code: 'INTERNAL_ERROR' }`), and the action door now answers `500 + * INTERNAL_ERROR` with the same sentence. `status` alone would not do: a + * declared 5xx with no code drops `code` from the data door's body. + * + * A class so a later conversion can tell an earlier one apart (the re-lead + * above). Module-private and never exported: no reader outside this file + * matches it, so the published surface does not move. `name` stays `'Error'`. + */ +class UnlandedWriteRefusal extends Error { + readonly status = 500; + readonly code = 'INTERNAL_ERROR'; + declare readonly cause: Error; + constructor(message: string, original: Error) { + super(message); + (this as { cause?: unknown }).cause = original; + } } /** {@link unlandedWriteRefusal} for a delete whose cascade walk the refusal stopped. */ @@ -124,6 +164,36 @@ export function cascadeStoppedRefusal(e: unknown, object: string): unknown { `The delete of "${object}" did not complete: its cascade stopped before the record was removed. ` + 'Under a transaction that rolls back nothing was written; otherwise the dependents written before ' + 'that point stay written.', + true, + ); +} + +/** + * {@link unlandedWriteRefusal} for a write whose hook a nested write's refusal + * stopped. + * + * A hook that writes through `ctx.api` makes a write of its own. When that + * write raises {@link SummaryRecomputeError}, the refusal leaves the handler + * carrying the HOOK's write as `written`. Let through, it reached the outer + * write's caller as that caller's own refusal, so a door answered the hook's + * record as the result of the write it was asked for. The outer write did not + * complete either: the rest of its chain, its own roll-ups and its events never + * ran. An `after*` hook's unit (`inAfterAbortUnit`) now rolls back both rows, + * as `HookSchema.onError` `abort` says. + * + * Only the refusal is converted; every other error a hook throws passes + * through unchanged. And only one a handler let escape: a hook whose `onError` + * is `log` swallows it before it gets here. + */ +export function hookStoppedRefusal(e: unknown, event: string, object: string | undefined, hook: string | undefined): unknown { + const verb = event.replace(/^(before|after)/, '').toLowerCase() || event; + return unlandedWriteRefusal( + e, + // Double quotes, as in `cascadeStoppedRefusal`. + `The ${verb} of "${object ?? '?'}" did not complete: its ${event} hook${hook ? ` "${hook}"` : ''} stopped. ` + + 'Under a transaction that rolls back nothing was written; otherwise what was written before that point ' + + 'stays written.', + true, ); } @@ -131,3 +201,16 @@ export function cascadeStoppedRefusal(e: unknown, object: string): unknown { export function rolledBackRefusal(e: unknown): unknown { return unlandedWriteRefusal(e, 'The transaction rolled back, so nothing written inside it was stored.'); } + +/** + * [#29421] {@link unlandedWriteRefusal} for a transaction whose rollback itself + * failed. Nothing is known about what is stored then, so the lead does not say + * "rolled back", and the refusal is still not answered as a success. + */ +export function rollbackFailedRefusal(e: unknown): unknown { + return unlandedWriteRefusal( + e, + 'The transaction did not complete, and its rollback failed too, so what was written inside it may or may ' + + 'not be stored.', + ); +} diff --git a/packages/rest/src/data-delete-cascade-summary-rollback.test.ts b/packages/rest/src/data-delete-cascade-summary-rollback.test.ts index e15c6277c20..a1acaa5f1cd 100644 --- a/packages/rest/src/data-delete-cascade-summary-rollback.test.ts +++ b/packages/rest/src/data-delete-cascade-summary-rollback.test.ts @@ -134,8 +134,10 @@ describe('a DELETE whose cascade a dependent\'s failing roll-up stops answers a it('DELETE of the account answers 500 INTERNAL_ERROR, and nothing is deleted', async () => { const res = await call('DELETE', '/:object/:id', { params: { object: ACCOUNT, id: 'a1' } }); expect(res._status, JSON.stringify(res._json)).toBe(500); - expect(res._json?.code, JSON.stringify(res._json)).toBe('INTERNAL_ERROR'); - expect(res._json.success).toBeUndefined(); + // [#29421] The whole body, byte for byte: the refusal now DECLARES `500 + // INTERNAL_ERROR`, and the door answers exactly what it answered when it + // reached the same 500 by falling through every message heuristic. + expect(res._json).toEqual({ error: 'Internal server error', code: 'INTERNAL_ERROR' }); expect(await rowIds(ACCOUNT)).toEqual(['a1']); expect(await rowIds(LINE)).toEqual(['l1', 'l2', 'l3']); }); diff --git a/packages/rest/src/data-write-hook-stale-summary-refusal.test.ts b/packages/rest/src/data-write-hook-stale-summary-refusal.test.ts new file mode 100644 index 00000000000..3fa8f448418 --- /dev/null +++ b/packages/rest/src/data-write-hook-stale-summary-refusal.test.ts @@ -0,0 +1,149 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * A POST whose hook made a write of its own, and that write's roll-up could + * not be recomputed, never answers the hook's record as its own result. + * + * ## The defect, measured before this file was written + * + * The data door answers a write's success over the engine's stale-roll-up + * refusal (`ERR_SUMMARY_RECOMPUTE`, "the triggering records WERE written"), + * using the refusal's `written` as the created record. An `afterInsert` hook on + * the case writes a line through `ctx.api`, and the line's roll-up into the + * bucket (`memo_sum`, a sum over a text field) refuses on every recompute. That + * refusal left the hook and reached the door unchanged, so `POST /data/case` + * answered `201 { object: case, id: , record: }`. + * That held for a metadata-bound hook (whose unit then committed both rows) + * and for a code-registered one alike. + * + * ## What is pinned + * + * - a metadata-bound hook (an aborting `afterInsert`, so the write opens its + * own unit): `500 INTERNAL_ERROR`, byte for byte, and neither row is stored; + * - a code-registered hook (no unit): the same answer, and both rows are stored, + * as the refusal's text says; + * - CONTROL: `POST` of a line itself (its own write landed, only its recompute + * failed) still answers `201` with its own row. + * + * Harness: the composed REST harness of `data-delete-cascade-summary-rollback.test.ts` + * (`RestServer` → `ObjectStackProtocolImplementation` → `ObjectQL` → a real + * `SqlDriver` on better-sqlite3, which rolls back for real). + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { ObjectQL, bindHooksToEngine } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import { RestServer } from './rest-server'; +import { installDefaultOrganization } from './default-organization.testkit.js'; + +const CASE = 'qhr_case'; +const CODE_CASE = 'qhr_code_case'; +const BUCKET = 'qhr_bucket'; +const LINE = 'qhr_line'; + +function createMockServer() { + const noop = () => {}; + return { get: noop, post: noop, put: noop, delete: noop, patch: noop, use: noop, listen: async () => {}, close: async () => {} }; +} + +function makeRes() { + const res: any = { + write: () => true, end: () => {}, + header: () => res, + status: (code: number) => { res._status = code; return res; }, + json: (body: any) => { res._json = body; return res; }, + }; + return res; +} + +const dir = mkdtempSync(join(tmpdir(), 'os-hook-rollup-')); +let engine: ObjectQL; +let call: (method: string, path: string, req: Record) => Promise; + +/** The hook's own write: a line, whose roll-up into the bucket every recompute refuses. */ +const writeLine = (name: string) => async (ctx: any) => { await ctx.api.object(LINE).insert({ name, bucket: 'b1' }); }; + +beforeAll(async () => { + engine = new ObjectQL(); + engine.registerDriver( + new SqlDriver({ client: 'better-sqlite3', connection: { filename: join(dir, 'hook.sqlite') }, useNullAsDefault: true }) as any, + true, + ); + await engine.init(); + engine.registry.registerObject({ name: CASE, label: 'Case', fields: { name: { name: 'name', type: 'text' } } } as any); + engine.registry.registerObject({ name: CODE_CASE, label: 'Code case', fields: { name: { name: 'name', type: 'text' } } } as any); + engine.registry.registerObject({ + name: BUCKET, label: 'Bucket', + fields: { + name: { name: 'name', type: 'text' }, + // A sum over a text field: every recompute of it is refused. + memo_sum: { name: 'memo_sum', type: 'summary', summaryOperations: { object: LINE, function: 'sum', field: 'memo' } }, + }, + } as any); + engine.registry.registerObject({ + name: LINE, label: 'Line', + fields: { + name: { name: 'name', type: 'text' }, + memo: { name: 'memo', type: 'text' }, + bucket: { name: 'bucket', type: 'master_detail', reference: BUCKET }, + }, + } as any); + await engine.syncSchemas(); + await installDefaultOrganization(engine); + await engine.insert(BUCKET, { id: 'b1', name: 'B' } as any); + bindHooksToEngine(engine as any, [ + { name: 'line_writer', object: CASE, events: ['afterInsert'], handler: writeLine('from-meta-hook') } as any, + ], { packageId: 'app:qhr' }); + engine.registerHook('afterInsert', writeLine('from-code-hook'), { object: CODE_CASE }); + + const protocol = new ObjectStackProtocolImplementation(engine as any); + const rest = new RestServer(createMockServer() as any, protocol as any, { api: { requireAuth: false } } as any); + (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' }); + rest.registerRoutes(); + const routes = rest.getRoutes(); + call = async (method, path, req) => { + const route = routes.find((r: any) => r.method === method && r.path === `/api/v1/data${path}`); + expect(route, `${method} /api/v1/data${path}`).toBeDefined(); + const res = makeRes(); + await route!.handler({ query: {}, headers: {}, ...req } as any, res); + return res; + }; +}); + +afterAll(async () => { + try { await engine.destroy(); } catch { /* noop */ } + rmSync(dir, { recursive: true, force: true }); +}); + +const names = async (object: string) => + ((await engine.find(object, { fields: ['name'] })) as Array<{ name: string }>).map((r) => r.name).sort(); + +describe('a POST whose hook\'s own write cannot recompute its roll-up never answers the hook\'s record', () => { + it('a metadata-bound afterInsert: 500 INTERNAL_ERROR, and neither row is stored', async () => { + const res = await call('POST', '/:object', { params: { object: CASE }, body: { name: 'c-1' } }); + expect(res._status, JSON.stringify(res._json)).toBe(500); + expect(res._json).toEqual({ error: 'Internal server error', code: 'INTERNAL_ERROR' }); + expect(await names(CASE)).toEqual([]); + expect(await names(LINE)).toEqual([]); + }); + + it('a code-registered afterInsert (no unit): the same answer, and both rows are stored', async () => { + const res = await call('POST', '/:object', { params: { object: CODE_CASE }, body: { name: 'cc-1' } }); + expect(res._status, JSON.stringify(res._json)).toBe(500); + expect(res._json).toEqual({ error: 'Internal server error', code: 'INTERNAL_ERROR' }); + expect(await names(CODE_CASE)).toEqual(['cc-1']); + expect(await names(LINE)).toEqual(['from-code-hook']); + }); + + it('CONTROL POST of a line itself landed: 201 with its own row', async () => { + const res = await call('POST', '/:object', { params: { object: LINE }, body: { name: 'direct', bucket: 'b1' } }); + expect(res._status ?? 201, JSON.stringify(res._json)).toBe(201); + expect(res._json?.object).toBe(LINE); + expect(res._json?.record?.name).toBe('direct'); + expect(res._json?.id).toBe(res._json?.record?.id); + }); +}); diff --git a/packages/runtime/src/action-delete-cascade-summary-rollback.test.ts b/packages/runtime/src/action-delete-cascade-summary-rollback.test.ts index e42ea892b16..71f8aee9e0b 100644 --- a/packages/runtime/src/action-delete-cascade-summary-rollback.test.ts +++ b/packages/runtime/src/action-delete-cascade-summary-rollback.test.ts @@ -13,14 +13,15 @@ * recompute refuses), so the first line's refusal stops the cascade and the * transaction rolls everything back. The engine now answers that delete with a * refusal that is not the stale-roll-up one, so the body does not run on and - * the action door answers it as it answers any other throw out of a body: a - * `400` refusal carrying the engine's message, `success: false`. + * the action door answers it as the internal failure it declares (#29421: `500 + * INTERNAL_ERROR`, before that the `400` any undeclared throw out of a body + * gets) carrying the engine's message, `success: false`. * * Pinned through the real stack: `HttpDispatcher` → the action door → * `ctx.engine` → `ObjectQL` → a real `SqlDriver` on better-sqlite3, which rolls * back for real (the harness of `dispatcher-unique-violation-wire-code.test.ts`). * - * - the account's delete: the action answers `400`, the body's next line never + * - the account's delete: the action answers `500`, the body's next line never * ran, and the account and its lines are all still stored; * - CONTROL a line's own delete landed: the action answers its body's own * result and the line is gone. @@ -133,10 +134,11 @@ const rowIds = async (object: string) => ((await engine.find(object, { fields: ['id'] })) as Array<{ id: string }>).map((r) => r.id).sort(); describe('an action\'s ctx.engine.delete whose cascade a dependent\'s failing roll-up stops is refused', () => { - it('the account\'s delete: the action answers a 400 refusal, the body stopped there, and nothing is deleted', async () => { + it('the account\'s delete: the action answers a 500 INTERNAL_ERROR refusal, the body stopped there, and nothing is deleted', async () => { const response = await postAction(`${ACCOUNT}/delete_account`); - expect(response.status, JSON.stringify(response.body)).toBe(400); + expect(response.status, JSON.stringify(response.body)).toBe(500); expect(response.body?.success, JSON.stringify(response.body)).toBe(false); + expect(response.body?.error?.code, JSON.stringify(response.body)).toBe('INTERNAL_ERROR'); expect(response.body?.error?.message).toContain('The delete of "qca_account" did not complete'); expect(bodyRanOn).not.toContain('delete_account'); expect(await rowIds(ACCOUNT)).toEqual(['a1']);