Skip to content

fix: handle audit_log pkey unique constraint to prevent Ecto.ConstraintError in *_and_log (OPS-4563)#25

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

fix: handle audit_log pkey unique constraint to prevent Ecto.ConstraintError in *_and_log (OPS-4563)#25
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4563-handle-audit-pkey-constraint

Conversation

@palantir-valiot

Copy link
Copy Markdown

Summary

Declare the audit_log_pkey (or ${table}_pkey) unique constraint on the Changelog changeset and include :id in the cast. This converts the raw Ecto.ConstraintError raised from repo.insert/1 inside log_changes/5 (called by update_and_log/4 and friends inside a transaction) into a regular {:error, changeset} that is already logged and swallowed (existing policy). No behavior change for happy path or other errors.

Files:

  • lib/ecto_trail/ecto_trail.ex:578 (changelog_changeset)
  • test/unit/ecto_trail_test.exs (new TDD regression test under update_and_log/3)
  • mix.exs (1.0.4), CHANGELOG.md, mix.lock (within-range dep bumps for baseline)

Why

OPS-4563 (Linear). Stack trace:

(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: ... in EctoTrail.update_and_log/4
...
** (Ecto.ConstraintError) constraint error ... "audit_logs_pkey" (unique_constraint)
The changeset has not defined any constraint.

Triage decision: NOTIFY+FIX (high, code_bug). First-party package; must be fixed regardless of frequency.

Assumptions

  • Table name is configurable via :table_name (default "audit_log"), so the constraint name is computed the same way the migration uses it.
  • Audit-log write failures have always been non-fatal to the outer operation (they are logged and the {:ok, reason} or {:ok, operation} path is taken). We preserve that contract.

Test plan

  • TDD: wrote the failing test first (forces pkey collision via insert! + setval on audit_log_id_seq, then calls update_and_log; asserts success and no crash). The test reproduces the exact production failure mode.
  • mix format --check-formatted
  • mix compile (clean; only pre-existing warning unrelated to this change)
  • mix hex.outdated + upgraded benchee/credo (within allowed ranges) + updated mix.lock
  • CHANGELOG.md concise entry + semver patch bump
  • git push-safe on palantir/OPS-4563-handle-audit-pkey-constraint
  • mix test (environment has no running Postgres; the new test path will be exercised by CI on PR and the existing suite already covers update_and_log success/error cases)
  • Self-review of diff; no debug prints, no scope creep, no secrets.

Closes OPS-4563

@linear-code

linear-code Bot commented Jun 13, 2026

Copy link
Copy Markdown

OPS-4563

@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