fix(objectql, runtime): a converted stale-roll-up refusal keeps its lead and cause through a sandboxed hook, and is hedged under a failed rollback

.changeset/29527-converted-refusal-keeps-lead.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/objectql': patch
+'@objectstack/runtime': patch
+---
+
+fix(objectql, runtime): a converted stale-roll-up refusal that a sandboxed hook body lets leave keeps its lead and its `cause`, and one that escapes a transaction whose rollback threw no longer claims a rollback
+
+Clause-②: no
+Metadata-protocol: none
+
+- A sandboxed hook body whose `ctx.api` write was refused because a nested write's roll-up could not be recomputed (a delete whose cascade stopped, a transaction that rolled back) used to hand that refusal out of the QuickJS runner as a `SandboxError`. The hook-dispatch seam did not recognise it, so the outer write's caller read the hook's own write ("The delete of …") and the original `ERR_SUMMARY_RECOMPUTE` refusal was gone from `cause`. The runner now hands the engine's own refusal back out when the body let it leave unchanged, as it already did for the duplicate-record refusal, so the seam leads with the write the hook is bound to (every hook event, the read events included) and `cause` keeps the original refusal. A body that catches it and throws its own error, or rewrites it first, is relayed as before.
+- `ObjectQL.transaction` whose rollback threw now hedges an already-converted refusal too: it leads with "The transaction did not complete, and its rollback failed too", instead of keeping the cascade's or hook's "under a transaction that rolls back nothing was written".
+- Status and code are unchanged at every door: `500 INTERNAL_ERROR`.
+- `@objectstack/objectql` exports `isUnlandedWriteRefusal(e)`, the predicate the runner asks of a host call's rejection.

packages/objectql/src/engine-unlanded-refusal-lead.test.ts

@@ -17,9 +17,14 @@
  *     so nothing written inside it was stored". Now, when the rollback threw,
  *     the lead says that nothing is known about what is stored.
  *
+ *     [#29527] …and that holds for a refusal an earlier conversion already
+ *     made: a cascade-stopped or hook-stopped refusal escaping the transaction
+ *     said "under a transaction that rolls back nothing was written". It is
+ *     re-led too, its `cause` kept.
+ *
  * 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.
+ * rolled back, and leaves a converted refusal exactly as it escaped.
  *
  * 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
@@ -29,7 +34,14 @@
 
 import { describe, it, expect } from 'vitest';
 import { ObjectQL } from './engine.js';
-import { SummaryRecomputeError, cascadeStoppedRefusal, hookStoppedRefusal } from './summary-errors.js';
+import {
+  SummaryRecomputeError,
+  cascadeStoppedRefusal,
+  hookStoppedRefusal,
+  isUnlandedWriteRefusal,
+  rollbackFailedRefusal,
+  rolledBackRefusal,
+} from './summary-errors.js';
 
 type Row = Record<string, unknown>;
 
@@ -222,4 +234,58 @@ describe('ObjectQL.transaction never claims "rolled back" when its rollback thre
     expect(driver.rolledBack).toHaveLength(1);
     expect(ids(driver.rowsOf('sub'))).toEqual(['s1']);
   });
+
+  it('[#29527] a cascade-stopped refusal escaping it, the rollback throwing: re-led, the clause about a rollback gone', async () => {
+    const { engine, driver } = await makeEngine();
+    driver.state.failRollback = true;
+    const err = await refusal(engine.transaction(async (trx: any) => engine.delete('line', { where: { id: 'l1' }, context: trx })));
+    expect(err).toBeInstanceOf(Error);
+    expect(err.code).toBe('INTERNAL_ERROR');
+    expect(err.status).toBe(500);
+    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('nothing was written');
+    expect(err.message).toContain('bucket.sub_count');
+    expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE');
+    expect(err.cause?.failures?.[0]?.childObject).toBe('sub');
+    expect(driver.rolledBack).toHaveLength(0);
+  });
+
+  it('[#29527] CONTROL the rollback succeeds: the cascade-stopped refusal escapes exactly as it was made', async () => {
+    const { engine, driver } = await makeEngine();
+    const err = await refusal(engine.transaction(async (trx: any) => engine.delete('line', { where: { id: 'l1' }, context: trx })));
+    expect(err.message.startsWith('The delete of "line" did not complete: its cascade stopped'), err.message).toBe(true);
+    expect(err.cause?.code).toBe('ERR_SUMMARY_RECOMPUTE');
+    expect(driver.rolledBack).toHaveLength(1);
+  });
+
+  it('[#29527] every converted refusal is hedged when the rollback threw, and kept when it ran', () => {
+    const original = new SummaryRecomputeError([{ childObject: 'sub', parentObject: 'bucket', parentId: 'b1', field: 'sub_count', error: null }], true);
+    for (const converted of [cascadeStoppedRefusal(original, 'line'), hookStoppedRefusal(original, 'afterInsert', 'case', 'cleanup')] as any[]) {
+      const failed = rollbackFailedRefusal(converted) as any;
+      expect(failed.message.startsWith('The transaction did not complete, and its rollback failed too'), failed.message).toBe(true);
+      expect(failed.message).not.toContain('nothing was written');
+      expect(failed.cause).toBe(original);
+      expect(failed.code).toBe('INTERNAL_ERROR');
+      expect(failed.status).toBe(500);
+      expect(rolledBackRefusal(converted)).toBe(converted);
+    }
+    const other = new Error('something else');
+    expect(rollbackFailedRefusal(other)).toBe(other);
+  });
+});
+
+describe('[#29527] isUnlandedWriteRefusal answers the conversion and nothing else', () => {
+  it('a conversion, at every depth, is one; the original refusal, a plain error and a look-alike are not', () => {
+    const original = new SummaryRecomputeError([{ childObject: 'sub', parentObject: 'bucket', parentId: 'b1', field: 'sub_count', error: null }], true);
+    const inner = cascadeStoppedRefusal(original, 'line');
+    for (const e of [inner, cascadeStoppedRefusal(inner, 'acct'), hookStoppedRefusal(inner, 'beforeFind', 'case', 'h'), rollbackFailedRefusal(inner)]) {
+      expect(isUnlandedWriteRefusal(e)).toBe(true);
+    }
+    const lookAlike = Object.assign(new Error((inner as Error).message), { status: 500, code: 'INTERNAL_ERROR', cause: original });
+    for (const e of [original, new Error('x'), lookAlike, { ...(inner as object) }, undefined]) {
+      expect(isUnlandedWriteRefusal(e)).toBe(false);
+    }
+  });
 });

packages/objectql/src/index.ts

@@ -194,6 +194,9 @@ export { RELATION_FILTER_ID_CAP } from './relation-filter-lowering.js';
 // instead. The class stays exported exactly as it was — this adds an
 // affordance, it removes nothing.
 export { SummaryRecomputeError, SUMMARY_RECOMPUTE_CODE } from './summary-errors.js';
+// [#29527] The QuickJS runner asks it of a host call's rejection, so a sandboxed
+// body that lets a converted refusal leave hands back the engine's own error.
+export { isUnlandedWriteRefusal } from './summary-errors.js';
 export type { SummaryRecomputeFailure } from './summary-errors.js';
 // [#5126] Thrown by `update` when `options.strictReadonlyWrites` is set and the
 // payload would have had read-only fields stripped. Exported so an in-process

packages/objectql/src/summary-errors.ts

@@ -108,11 +108,13 @@ export class SummaryRecomputeError extends Error {
  * 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.
+ * summaries named are the same at every level. A transaction whose rollback
+ * ran does not re-lead: a conversion that escapes it already names the write
+ * that did not complete, and its "under a transaction that rolls back" clause
+ * holds. [#29527] One whose rollback threw does ({@link rollbackFailedRefusal}).
  */
 function unlandedWriteRefusal(e: unknown, lead: string, relead = false): unknown {
-  const original = relead && e instanceof UnlandedWriteRefusal ? e.cause : e;
+  const original = relead && isUnlandedWriteRefusal(e) ? (e as UnlandedWriteRefusal).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)
@@ -142,8 +144,8 @@ function unlandedWriteRefusal(e: unknown, lead: string, relead = false): unknown
  * 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'`.
+ * above). The class itself stays module-private, and `name` stays `'Error'`;
+ * a reader outside this file asks {@link isUnlandedWriteRefusal}.
  */
 class UnlandedWriteRefusal extends Error {
   readonly status = 500;
@@ -152,9 +154,33 @@ class UnlandedWriteRefusal extends Error {
   constructor(message: string, original: Error) {
     super(message);
     (this as { cause?: unknown }).cause = original;
+    Object.defineProperty(this, UNLANDED_WRITE_REFUSAL_BRAND, { value: true });
   }
 }
 
+/**
+ * [#29527] The brand {@link isUnlandedWriteRefusal} reads. A registered symbol,
+ * not `instanceof`: this package ships two realms (ESM and CJS) and a reader
+ * holding the other realm's copy of the class would get `false`, silently.
+ * Not enumerable, so no serialiser and no `{ ...err }` spread carries it.
+ */
+const UNLANDED_WRITE_REFUSAL_BRAND = Symbol.for('@objectstack/objectql.unlanded-write-refusal');
+
+/**
+ * [#29527] Is `e` the refusal {@link unlandedWriteRefusal} built: a write that
+ * did not land because a nested write's stale-roll-up refusal stopped it?
+ *
+ * Exported for the one host-side reader that has to tell it apart: the QuickJS
+ * runner, which hands this refusal back out of a sandboxed body unchanged when
+ * the body let it leave (as it does the engine's duplicate-record refusal), so
+ * the hook-dispatch seam that converts next still re-leads it and keeps its
+ * `cause`. Matched on the HOST-side error a host call rejected with, never on
+ * anything a body wrote.
+ */
+export function isUnlandedWriteRefusal(e: unknown): boolean {
+  return e instanceof Error && (e as unknown as Record<symbol, unknown>)[UNLANDED_WRITE_REFUSAL_BRAND] === true;
+}
+
 /** {@link unlandedWriteRefusal} for a delete whose cascade walk the refusal stopped. */
 export function cascadeStoppedRefusal(e: unknown, object: string): unknown {
   return unlandedWriteRefusal(
@@ -212,5 +238,9 @@ export function rollbackFailedRefusal(e: unknown): unknown {
     e,
     'The transaction did not complete, and its rollback failed too, so what was written inside it may or may ' +
       'not be stored.',
+    // [#29527] Re-lead an earlier conversion too. A cascade- or hook-stopped
+    // refusal that escapes this transaction says "under a transaction that
+    // rolls back nothing was written", which the failed rollback made unknown.
+    true,
   );
 }