Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .changeset/batch-row-driver-text-withhold.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
---
"@objectstack/metadata-protocol": patch
---

Withhold undeclared driver text from a bulk write's per-row `errors[].message` (#8502)

`toRowApiError` interpolated whatever it caught into a batch row's message, so a
driver fault under `deleteManyData` answered
`{ code: "INTERNAL_ERROR", message: "SQLITE_ERROR: no such table: leave_request" }`
on response DATA riding a 200 — where no HTTP boundary's 5xx withhold can reach
it. Driven against a real driver the leaked text is worse than the tidy example:
a delete's raw message carries the failing statement's `WHERE` clause and its
bound record id, and a create's carries the whole `INSERT` with its values. The
causal row's message is also copied onto every `NOT_ATTEMPTED` / `ROLLED_BACK`
sibling, so one leaked sentence was repeated across the batch.

A caught sentence now reaches a caller only when its producer declared a
client-facing refusal, asked through `resolveThrownHttpError` — the same
resolver the HTTP doors answer with — so all three declarations this sink
actually receives are honoured: a 4xx `status`, a 4xx `statusCode`, and the
`VALIDATION_FAILED` shape that carries neither. Per-field authoring feedback
from the engine's validator, `RECORD_NOT_FOUND`, `VALIDATION_FAILED` and
`plugin-approvals`' `RECORD_LOCKED` are unchanged, byte for byte. Anything
undeclared — a driver fault, or a hook that throws a bare `Error` — gets a
stable sentence naming the operation, and the original goes to the server log.

The `code` limb is untouched (#8441 already gates it on catalog membership), and
no `httpStatus` is minted where the wire did not carry one.

**Behaviour change for hook authors**: a hook that refuses by throwing an
undeclared `Error` no longer has its sentence echoed on the row. Declare the
refusal — a 4xx `status` or `statusCode`, or `validationFailure(message, fields)`
from `@objectstack/types` — and the message is served verbatim, as it now is on
the single-record path.
22 changes: 18 additions & 4 deletions packages/metadata-protocol/src/protocol.batch-atomic.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -104,8 +104,15 @@ describe('batchData atomic — rollback is real and the response admits it (ADR-
// "Attempted, undone" vs "never ran" is a CODE, not a message-prefix
// regex (#4793) — the message keeps the human-readable cause.
expect(res.results[0].errors?.[0]?.code).toBe('ROLLED_BACK');
expect(res.results[0].errors?.[0]?.message).toContain('insert exploded'); // carries the cause
expect(res.results[1].errors?.[0]?.message).toBe('insert exploded'); // the causal row, verbatim
// [#8502] `insert exploded` is a BARE `Error` — it declares no client
// refusal, so its sentence is withheld and the row says the stable
// operation-named line instead. The claim under test is unchanged and
// is about PROPAGATION: whatever the causal row says, the rolled-back
// row quotes it, so a caller reading row 0 learns why row 1 stopped
// the batch. Asserted against the causal row's own message rather than
// a literal, so the two cannot drift apart.
expect(res.results[1].errors?.[0]?.message).toBe('The create of this record failed. The reason is in the server log.');
expect(res.results[0].errors?.[0]?.message).toContain(res.results[1].errors?.[0]?.message); // carries the cause
expect(res.results[2].errors?.[0]?.code).toBe('NOT_ATTEMPTED');
// Rows correlate to the request array by `index` (#4793).
expect(res.results.map((r: any) => r.index)).toEqual([0, 1, 2]);
Expand DownExpand Up@@ -162,7 +169,8 @@ describe('batchData atomic — rollback is real and the response admits it (ADR-
expect(res.succeeded).toBe(0);
expect(res.results[0].errors?.[0]?.code).toBe('ROLLED_BACK');
expect(res.results[0].id).toBe('rec-1'); // ids survive so a caller can reconcile
expect(res.results[1].errors?.[0]?.message).toBe('update exploded');
// [#8502] withheld: a bare `Error` declares no client refusal.
expect(res.results[1].errors?.[0]?.message).toBe('The update of this record failed. The reason is in the server log.');
});
});

Expand DownExpand Up@@ -236,7 +244,13 @@ describe('batchData atomic — precedence and opt-in (ADR-0119 D4)', () => {

expect(t.rollbacks).toHaveLength(1);
expect(t.insert).not.toHaveBeenCalled(); // no blind fallback
expect(res.results[0].errors?.[0]?.message).toBe('update exploded'); // the real cause survives
// [#8502] The cause is withheld from the RESPONSE (bare `Error`), so
// "the real cause survives" is now carried by the two structural
// assertions above — the update was attempted and no fallback insert
// ran — plus the row naming the UPSERT it was doing. What must never
// appear is the fallback insert's duplicate-key text.
expect(res.results[0].errors?.[0]?.message).toBe('The upsert of this record failed. The reason is in the server log.');
expect(res.results[0].errors?.[0]?.message).not.toContain('duplicate key');
});
});

Expand Down
Original file line numberDiff line numberDiff line change
Expand Up@@ -129,7 +129,11 @@ describe('batchData rows conform to BatchOperationResultSchema (#4793)', () => {
expectConformantResponse(res, 3);
expect(res.results[0].data).toMatchObject({ title: 'A' });
// An unclassified engine throw is a 500 in row form.
expect(res.results[1].errors[0]).toMatchObject({ code: 'INTERNAL_ERROR', message: 'insert exploded' });
// [#8502] `code` is unchanged; the message is the withheld stable line.
expect(res.results[1].errors[0]).toMatchObject({
code: 'INTERNAL_ERROR',
message: 'The create of this record failed. The reason is in the server log.',
});
expect(res.results[1].data).toBeUndefined();
});

Expand DownExpand Up@@ -185,8 +189,9 @@ describe('batchData rows conform to BatchOperationResultSchema (#4793)', () => {

expectConformantResponse(res, 3);
expect(res.results[0].errors[0].code).toBe('ROLLED_BACK');
expect(res.results[0].errors[0].message).toContain('insert exploded'); // human-readable cause
expect(res.results[1].errors[0].message).toBe('insert exploded'); // causal row keeps its own error
// [#8502] Same propagation claim, against the causal row's own text.
expect(res.results[1].errors[0].message).toBe('The create of this record failed. The reason is in the server log.');
expect(res.results[0].errors[0].message).toContain(res.results[1].errors[0].message); // human-readable cause
expect(res.results[2].errors[0].code).toBe('NOT_ATTEMPTED');
// No reverted write may carry a record payload.
for (const row of res.results) expect(row.data).toBeUndefined();
Expand All@@ -210,7 +215,7 @@ describe('updateManyData rows conform to BatchOperationResultSchema (#4793)', ()

expectConformantResponse(res, 3);
expect(res.results[0].data).toMatchObject({ id: 'a', title: 'a-new' });
expect(res.results[1].errors[0].message).toBe('update exploded');
expect(res.results[1].errors[0].message).toBe('The update of this record failed. The reason is in the server log.'); // [#8502]
});

it('atomic rollback — all three row classes, as codes', async () => {
Expand Down
Loading
Loading