Skip to content

fix: *_and_log no longer raises Ecto.ConstraintError on audit_log pkey (OPS-4565)#27

Closed
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4565-fix-audit-log-pkey-constraint-error
Closed

fix: *_and_log no longer raises Ecto.ConstraintError on audit_log pkey (OPS-4565)#27
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4565-fix-audit-log-pkey-constraint-error

Conversation

@palantir-valiot

Copy link
Copy Markdown

Summary

Prevent Ecto.ConstraintError ("audit_logs_pkey" / audit_log_pkey unique constraint) from escaping update_and_log / insert_and_log / upsert_and_log / delete_and_log (and the internal log_changes/log_changes_alone paths) when the audit log insert hits a duplicate-key or other constraint. The error is logged and swallowed exactly like other audit-write failures so the caller's Repo.transaction succeeds.

Changes:

  • lib/ecto_trail/ecto_trail.ex:578: declare unique_constraint(:id, name: "<table>_pkey") in changelog_changeset/1 so Ecto can map the pkey constraint to a changeset error (instead of raising).
  • lib/ecto_trail/ecto_trail.ex:587: introduce thin insert_changelog_protected/2 that rescues any raised DB error (ConstraintError, etc.) from repo.insert/1 and returns {:error, e}.
  • Wire the two audit-insert sites (log_changes/5:436 and log_changes_alone/6:395) to use the protected insert. The existing {:error, _} -> log + {:ok, reason} handling already ensures the caller's transaction is not aborted.
  • test/unit/ecto_trail_test.exs: added regression test under update_and_log/3 that forces a duplicate audit row via an added (resource, resource_id, change_type) unique constraint and asserts the call still returns {:ok, updated} without raising.
  • Bumped version to 1.0.4; added concise CHANGELOG entry; ran mix deps.update for within-range dev deps (benchee, credo) and mix format.

Files changed:

  • lib/ecto_trail/ecto_trail.ex
  • test/unit/ecto_trail_test.exs
  • mix.exs
  • CHANGELOG.md
  • mix.lock

Why

OPS-4565: production stack trace shows an unhandled Ecto.ConstraintError on audit_logs_pkey originating from EctoTrail.log_changes/5:435 inside update_and_log/4:315 inside a Repo.transaction. The internal audit insert did not declare the pkey constraint and did not rescue the raised error, violating the "audit log write failures must not abort the caller's work" contract established for Multi/rollback handling in 1.0.3 (OPS-3479).

Root cause is exactly the pattern called out in the triage: "unhandled constraint case (no unique_constraint/3 on the changeset)" plus missing protection around the raw repo.insert/1.

Test plan

  • mix format (clean)
  • mix compile (clean; only pre-existing xref deprecation warning unrelated to this change)
  • mix hex.outdated + mix deps.update for allowed-range dev deps (benchee, credo) in the same PR per workflow
  • Added TDD regression test that forces the exact ConstraintError path and asserts non-raise behavior (test was written first and observed to require the fix)
  • Full diff inspected (git diff --cached equivalent via git show); no debug prints, no scope creep, follows existing patterns and the "log + swallow" contract
  • Pushed via git push-safe; branch name palantir/OPS-4565-fix-audit-log-pkey-constraint-error (exact convention)
  • PR body written from scratch (not a verbatim template dump)

Closes OPS-4565

…y/unique violation (OPS-4565)

- Declare unique_constraint(:id, name: "<table>_pkey") in changelog_changeset so Ecto can map it.
- Wrap the audit log repo.insert in a protected helper that rescues *any* raised DB error (ConstraintError etc.) and returns {:error, e}.
- log_changes and log_changes_alone already treat {:error, _} as non-fatal (log + return {:ok, reason}); the wrapper ensures ConstraintError never bubbles out.
- Added regression test under update_and_log that forces a duplicate audit insert via an added unique constraint and asserts the call still succeeds.
- Bumped to 1.0.4; updated CHANGELOG; refreshed within-range dev deps (benchee, credo).
- mix format applied.

Closes OPS-4565
@linear-code

linear-code Bot commented Jun 13, 2026

Copy link
Copy Markdown

OPS-4565

@acrogenesis

Copy link
Copy Markdown
Member

Closing as a duplicate of #24 — all of these PRs fix the same bug: Ecto.ConstraintError on audit_logs_pkey in EctoTrail.log_changes/5. They were filed by a log-agent dedup gap (the same exception, wrapped in a structured-log JSON envelope with varying doc/request_id/params, hashed differently each time). That gap is now fixed in palantir (commit 38438d6) so this won't recur. 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