Skip to content

fix: handle audit_logs_pkey unique constraint in log_changes (OPS-4574)#32

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

fix: handle audit_logs_pkey unique constraint in log_changes (OPS-4574)#32
palantir-valiot[bot] wants to merge 1 commit into
mainfrom
palantir/OPS-4574-handle-audit-log-pkey-constraint

Conversation

@palantir-valiot

Copy link
Copy Markdown

Summary

Root cause: update_and_log / insert_and_log (and friends) run inside an explicit repo.transaction, then call log_changes which does a raw repo.insert(changelog_changeset(...)). The Changelog schema had no unique_constraint declared on its pkey, so a DB unique violation on audit_log_pkey (e.g. sequence rewind in prod) produced an unhandled Ecto.ConstraintError instead of {:error, changeset}. The surrounding code already treats {:error, reason} as a soft failure (it logs and returns {:ok, reason}), allowing the caller's write to succeed. The error surfaced in valiot_app during a GraphQL updateDevice mutation.

Fix: pipe the cast through Changeset.unique_constraint(:id, name: pkey_constraint_name()) inside changelog_changeset/1. The pkey name is computed dynamically from the (configurable) Changelog table source so it works for custom table_name configs. Added a TDD test that seeds a Changelog row, rewinds the sequence, then calls update_and_log and asserts the outer operation still succeeds.

Changed files:

  • lib/ecto_trail/ecto_trail.ex:574 (the constraint declaration + helper)
  • test/unit/ecto_trail_test.exs (new collision test in the update_and_log describe)
  • mix.exs, CHANGELOG.md (version + release note)
  • mix.lock (freshened dev/test deps per baseline rules)

Why

This is the exact failure described in OPS-4574 (Ecto.ConstraintError on audit_logs_pkey during ecto_trail.log_changes called from update_and_log inside a transaction). See stack trace in the issue for lib/ecto_trail/ecto_trail.ex:435.

Closes OPS-4574

Test plan

  • mix format (clean)
  • mix compile (clean; only pre-existing redundant-clause warning in map_custom_ecto_type)
  • mix hex.outdated + upgraded benchee 1.5.0→1.5.1 and credo 1.7.18→1.7.19 (and transitive) within allowed ranges in the same commit
  • Added/edited a regression test that forces a pkey collision on the audit log table (via setval on the sequence) during update_and_log and asserts success instead of ConstraintError
  • mix test — could not execute in the agent pod (Postgres is not available on localhost:5432; test/test_helper.exs requires a real DB and runs migrations). The new test follows the exact style and helpers of the surrounding tests and will be exercised by CI.
  • Read git diff; no debug prints, no unrelated changes, no empty diff
  • Version bump 1.0.3 → 1.0.4 and concise CHANGELOG entry (one line describing the visible behavior shift)
  • Branch name follows the non-negotiable convention: palantir/OPS-4574-handle-audit-log-pkey-constraint
  • PR body written from scratch (not the raw template)

Screenshots

N/A — no UI change.

…tx no longer raise Ecto.ConstraintError on collisions

- TDD: added test that seeds a Changelog row then rewinds the id sequence so the next update_and_log hits pkey unique violation; asserts the outer write still succeeds (no crash).
- changelog_changeset now pipes through unique_constraint(:id, name: "#{table}_pkey").
- log_changes already treats {:error, reason} as soft-fail (logs + returns {:ok, reason}); the constraint declaration converts the DB exception into that path.
- Bumped 1.0.3 -> 1.0.4 and updated CHANGELOG.md per release rules.
- Upgraded benchee 1.5.0->1.5.1 and credo 1.7.18->1.7.19 (plus transitive) within allowed ranges; ran mix hex.outdated.
- Ran mix format.

Closes OPS-4574
@linear-code

linear-code Bot commented Jun 13, 2026

Copy link
Copy Markdown

OPS-4574

@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