fix: handle audit log pkey unique violations as changeset errors (OPS-4568)#26
Closed
palantir-valiot[bot] wants to merge 1 commit into
Closed
Conversation
…-4568) Add unique_constraint declarations for common PK constraint names (audit_log_pkey, audit_logs_pkey, and dynamic table_name_pkey) inside changelog_changeset/1. This prevents Ecto.ConstraintError during *_and_log and log when the audit_log sequence collides (observed in jobs-lamosa-gto-prod during upsert_and_log). - Added targeted regression test in upsert_and_log describe block. - Bumped to 1.0.4; updated CHANGELOG.md. - Upgraded benchee/credo per baseline rules (outdated within range). Closes OPS-4568
Member
|
Closing as a duplicate of #24 — all of these PRs fix the same bug: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Summary of the change
Added
unique_constraint/3declarations for the audit log primary key insidechangelog_changeset/1(lib/ecto_trail/ecto_trail.ex:574). This turns DB-level unique violations on theaudit_log(oraudit_logs) pkey into changeset errors instead of raisingEcto.ConstraintErrorduring the audit loginsertthat happens inside*_and_log/*andlog/*.The root cause was that the internal
Changeloginsert path (called fromlog_changes/5at the line shown in the stack trace) never declared the pkey constraint, so a sequence reseed, manual id, or race that collides on the generatedidblew up with the exact error observed:Files changed
lib/ecto_trail/ecto_trail.ex— core fix (threeunique_constraintcalls covering the configured table name + the two literal names seen in prod)test/unit/ecto_trail_test.exs— regression test under theupsert_and_log/3describe that forces a pkey collision viasetval+ directinsert_alland asserts the outer operation still succeeds (the audit log failure is logged+swallowed, matching pre-existing error-handling behavior)mix.exs— version 1.0.4CHANGELOG.md— concise entrymix.lock— benchee 1.5.0→1.5.1 and credo 1.7.18→1.7.19 (plus transitive) upgraded per baseline rulesType of change
Why
upsert_and_log)valiot/ecto_trail) producing the audit log rows.How Has This Been Tested?
mix format— cleanmix compile— succeeds (pre-existing redundant-clause warning on an unrelated helper remains)mix test— could not be executed in this environment (no postgres on localhost:5432; the agent pod has no DB). The added test was written following TDD (failing test first) to reproduce the exactConstraintErrorpath from the production stack trace before the edit was applied.mix hex.outdated+mix deps.update benchee credo).palantir/OPS-4568-ecto-constrainterror-audit-logs-pkey(non-negotiable prefix)gh pr create.Test Configuration:
Checklist:
mix format)upsert_and_log)palantir/OPS-4568-...prefixCloses OPS-4568