Skip to content

fix: handle audit_logs_pkey ConstraintError in *_and_log / log (OPS-4612)#73

Closed
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4612-handle-audit-logs-pkey-constraint
Closed

fix: handle audit_logs_pkey ConstraintError in *_and_log / log (OPS-4612)#73
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4612-handle-audit-logs-pkey-constraint

Conversation

@palantir-valiot

Copy link
Copy Markdown

Summary

Prevent Ecto.ConstraintError ("audit_logs_pkey" unique_constraint) during the internal audit log insert in EctoTrail.update_and_log (and sibling helpers insert_and_log, upsert_and_log, delete_and_log, log, log_bulk).

The root cause was two-fold:

  • changelog_changeset/1 never declared unique_constraint(:id, name: "<table>_pkey"), so Ecto raised instead of returning {:error, changeset}.
  • The insert sites in log_changes/5 and log_changes_alone/6 (ecto_trail.ex:435, 394) did not rescue ConstraintError (or any other exception) — they let it propagate out of the repo.transaction in the *_and_log wrappers, rolling back the caller's business work.

Fix:

  • Expose Changelog.table_name/0 (respects the :table_name app env override).
  • Add the pkey unique_constraint (table-aware) in changelog_changeset.
  • Extract insert_audit_safely/3 that treats any error or raised exception from the audit insert exactly like the pre-existing "best effort" paths: log and return {:ok, reason} so the outer transaction commits the real operation.
  • Added a regression test that forces a pkey collision on the audit_log sequence and asserts the business update_and_log still succeeds and commits.

Files changed: lib/ecto_trail/ecto_trail.ex, lib/ecto_trail/changelog.ex, test/unit/ecto_trail_test.exs, mix.exs, CHANGELOG.md.

Why

Stack trace from OPS-4612 (eliot-lamosa-gto-prod):

(ecto_trail 1.0.3) lib/ecto_trail/ecto_trail.ex:435: EctoTrail.log_changes/5
(ecto_trail 1.0.3) lib/ecto_trail/ecto_trail.ex:315: anonymous fn/4 in EctoTrail.update_and_log/4
...
(Ecto.ConstraintError) constraint error when attempting to insert struct: * "audit_logs_pkey" (unique_constraint)

This is a clear code bug in the first-party package valiot/ecto_trail. The caller's GraphQL mutation (and any other writer using *_and_log) must not be rolled back or crash because of a duplicate audit log pkey.

Closes OPS-4612

Test plan

  • mix format (per repo baseline; no AGENTS.md/CLAUDE.md present)
  • mix compile (clean except one pre-existing redundant-clause warning on map_custom_ecto_type)
  • Added TDD regression test exercising the exact update_and_log + pkey collision path (the test seeds a conflicting row via the sequence and asserts the business update commits). Full mix test requires a live Postgres (test/test_helper.exs + data_case.ex + migrations); the container had no DB server reachable, so integration tests could not be executed here. The new test follows the same patterns as the rest of the suite and will be exercised by CI.
  • Manual review of git diff (see below) — scoped only to the audit log error handling + constraint declaration + version/changelog. No debug prints, no unrelated changes.
  • Branch name follows the non-negotiable convention: palantir/OPS-4612-handle-audit-logs-pkey-constraint
  • All pushes performed via git push-safe (plain push used only once as a bootstrap to create the remote ref so the wrapper could run its fetch/rebase/force-with-lease path).

Diff stats (for reviewer sanity)

 CHANGELOG.md                  |  6 ++++++
 lib/ecto_trail/changelog.ex   |  3 +++
 lib/ecto_trail/ecto_trail.ex  | 47 +++++++++++++++++++++++++------------------
 mix.exs                       |  2 +-
 test/unit/ecto_trail_test.exs | 45 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 82 insertions(+), 21 deletions(-)

…t (OPS-4612)

- Add table-aware unique_constraint(:id, name: "..._pkey") on Changelog changeset.
- Wrap audit log insert in insert_audit_safely/3 that rescues ConstraintError (and any other) and returns {:ok, reason} instead of letting it propagate and rollback the caller's transaction.
- Refactor the two insert sites (log_changes and log_changes_alone) to use the shared helper.
- Expose Changelog.table_name/0 for constraint name computation (supports :table_name app env override).
- Add regression test for update_and_log that seeds a colliding pkey row and asserts the business update still commits.
- Bump 1.0.3 -> 1.0.4 and concise CHANGELOG entry.

Closes OPS-4612
@linear-code

linear-code Bot commented Jun 13, 2026

Copy link
Copy Markdown

OPS-4612

@acrogenesis

Copy link
Copy Markdown
Member

Closing as a duplicate of #24 — same bug: Ecto.ConstraintError on audit_logs_pkey in EctoTrail.log_changes/5. These were generated from a backlog of duplicate Linear issues created by a log-agent dedup gap (now fixed in palantir 38438d6; no new duplicates are being filed). Consolidating on #24.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant