fix: absorb Ecto.ConstraintError on audit_logs_pkey / audit_log_pkey in log_changes/5 and *_and_log paths (OPS-4588)#49
Closed
palantir-valiot[bot] wants to merge 1 commit into
Conversation
…nd_log paths
- Declare unique_constraint(:id, name: <table>_pkey) variants (audit_log_pkey, audit_logs_pkey, and the configured table name) on the Changelog changeset.
- Introduce insert_changelog_or_swallow/4 that rescues any error (including Ecto.ConstraintError) on the bare audit insert and logs+returns {:ok, reason} instead of letting the exception escape the transaction.
- Add regression tests that force a pkey collision via direct INSERT at a high sequence value and assert update_and_log/log swallow it (no ConstraintError) and the original business op succeeds.
- Bump to 1.0.4; add concise changelog entry.
- Upgrade benchee/credo (within ranges) per baseline rules.
Closes OPS-4588
Member
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.
Summary
Add
unique_constraint(:id, name: <table>_pkey)declarations (coveringaudit_log_pkey,audit_logs_pkey, and the configured table name) to theChangelogaudit-log changeset. Refactor the two insert paths (log_changes/5andlog_changes_alone/6) to go through a newinsert_changelog_or_swallow/4helper that rescues any error (includingEcto.ConstraintError) on the barerepo.insert, logs it, and returns{:ok, reason}instead of letting the exception escape the caller's transaction.This makes
update_and_log/4,insert_and_log/4,upsert_and_log/4,delete_and_log/4, and the standalonelog/5resilient to transient/duplicate pkey collisions on the audit table (the exact failure mode in the OPS-4588 stack trace fromvaliot_appcallingupdate_and_loginside a transaction).Also upgraded
benchee/credo(and transitivedeep_merge/statistex) within allowed ranges per baseline rules, bumped to 1.0.4, and added a concise CHANGELOG entry.Files changed:
Why
Linear: OPS-4588 —
Ecto.ConstraintErroronaudit_logs_pkey(oraudit_log_pkey) duringecto_trail.log_changes/5called fromupdate_and_loginside a transaction in eliot-lamosa-gto-prod. The triage analysis classified this as a high-severity first-party code bug in valiot/ecto_trail.The root cause was a bare
repo.insert(changelog_changeset(...))with nounique_constrainton the synthetic:id(the PK is DB-generated) and no rescue around the insert, so any concurrent or replayed pkey collision blew up the caller's transaction.Test plan
mix format(clean; applied)mix credo --strict(clean on the three source files)mix compile(andmix compile --warnings-as-errorsafter fixing our own introduced compile-timeApplication.compile_envmisuse by moving the table name to a module attribute)update_and_logstill succeeds for the business resourcelog/5pathmix test(and the DB-dependent new tests) will run in CI (the pod environment has no reachable Postgres; see test helper and prior CI config using Travis + external postgres). The tests are written to fail for the right reason before the fix.benchee,credo) in the same PR as required.palantir/OPS-4588-...branch convention andgit push-safe.Closes OPS-4588