Skip to content

Commit fbd45d0

Browse files
committed
docs(adr): log AmbiguousCommitException consumer wiring as known gap
The in-service half of the ambiguous-commit protection is delivered (no retry once the real COMMIT has been attempted), but the caller-side half the exception's contract directs - queue jobs catching it and failing without retry - is not wired anywhere: no job or service in app/ catches it, Laravel's queue retries on any uncaught exception while tries remain (21 jobs declare tries of 2-5; ~35 top-level jobs inherit the worker default), and the one catch that does see it today (processTicketData's per-row log-and-skip) swallows it as a generic warning and still deletes the import file. Recorded in ADR-003 Known Gaps with the recommended follow-up: catch and fail() without retry in payment/order-critical jobs (error-level reconciliation event), and in log-and-skip loops handle it separately from the generic catch - record the row as unknown outcome and preserve the source artifact for reconciliation.
1 parent 757d3b8 commit fbd45d0

1 file changed

Lines changed: 23 additions & 0 deletions

File tree

adr/003-nested-transaction-rollback-safety.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,3 +364,26 @@ attempt starts a fresh root transaction whose `getByExternalIdExclusiveLock()` r
364364
member the winning worker committed, and ingestion succeeds — making the recovery the comment
365365
promises actually happen for the first time, without catching around a nested `transaction()`
366366
call (the exact shape this ADR concludes is never safe).
367+
368+
### `AmbiguousCommitException` has no consumers yet — queue jobs still blind-retry ambiguous commits
369+
370+
The in-service half of the ambiguous-commit protection is delivered: `runRootTransaction()`
371+
never re-executes the callback once the real COMMIT has been attempted (hardening item 1).
372+
The caller-side half is not wired up anywhere. The exception's contract directs callers —
373+
queue jobs especially — to catch it and fail without retry (`$this->fail($e)`), then
374+
reconcile; as of this branch, **no job or service in `app/` catches it**. Laravel's queue
375+
does not inspect exception types: any uncaught exception triggers a retry while `tries`
376+
remain, so for any job configured with `tries > 1` (21 jobs declare `$tries` of 2-5, and the
377+
~35 top-level jobs in `app/Jobs/` without a `$tries` property inherit the worker's `--tries`
378+
default) an ambiguous commit still re-runs the whole job — the exact duplicate-writes
379+
scenario the marker type exists to prevent. The one `catch` that does see it today —
380+
`SummitOrderService::processTicketData()`'s per-row log-and-skip (`:4673-4675`) — swallows
381+
it as a generic warning and proceeds to delete the import file, treating an
382+
unknown-durability row as cleanly processed.
383+
384+
**Recommended fix (follow-up):** wire the consumer side in two places. (1) In
385+
payment/order-critical queue jobs, catch `AmbiguousCommitException` and fail the job without
386+
retry, logging it at error level as a reconciliation-required event. (2) In log-and-skip
387+
loops (the CSV import row loop being the known case), handle it separately from the generic
388+
`catch (Exception)`: record the row as *unknown outcome* rather than *failed*, and preserve
389+
the source artifact instead of deleting it, so the row can be reconciled against the DB.

0 commit comments

Comments
 (0)