Skip to content

🐛 fix(limiter): distinguish TransactionConflict from ConditionalCheckFailed in _commit_initial (#332)#401

Merged
sodre merged 1 commit into
mainfrom
fix/332-transaction-conflict-retry
Feb 16, 2026
Merged

🐛 fix(limiter): distinguish TransactionConflict from ConditionalCheckFailed in _commit_initial (#332)#401
sodre merged 1 commit into
mainfrom
fix/332-transaction-conflict-retry

Conversation

@sodre

@sodre sodre commented Feb 16, 2026

Copy link
Copy Markdown
Member

Summary

  • Distinguish TransactionConflict from ConditionalCheckFailed in _is_condition_check_failure() by inspecting CancellationReasons codes instead of treating all TransactionCanceledException the same
  • Add _is_transaction_conflict() helper and retry loop with exponential backoff (25ms base, 3 retries) for transient contention errors in _commit_initial()
  • Fix _build_retry_failure_statuses() to compute retry_after_seconds from bucket state via calculate_retry_after() instead of hardcoding 0.0
  • Apply identical fixes to both async (lease.py) and sync (sync_lease.py) code paths

Test plan

  • Unit tests for _is_condition_check_failure(): pure ConditionalCheckFailed, pure TransactionConflict, mixed reasons, no response attribute
  • Unit tests for _is_transaction_conflict(): pure conflict, condition-check-only, unrelated exceptions, ClientError fallback
  • Unit test: TransactionConflict retries original transaction (not consumption-only path)
  • Unit test: TransactionConflict exhausts retries and propagates exception
  • Unit test: ConditionalCheckFailed still enters consumption-only retry path (regression)
  • Unit test: mixed reasons prefer ConditionalCheckFailed over TransactionConflict
  • Unit test: cascade TransactionConflict does not raise false RateLimitExceeded (issue scenario)
  • Unit test: _build_retry_failure_statuses() computes non-zero retry_after_seconds from deficit
  • Unit test: _build_retry_failure_statuses() returns 0.0 when no deficit
  • Run full unit test suite: uv run pytest tests/unit/ -v

Closes #332

🤖 Generated with Claude Code

…Failed in _commit_initial (#332)

TransactionConflict during cascade contention was incorrectly treated as
ConditionalCheckFailed, causing false RateLimitExceeded with
retry_after_seconds=0.0. The fix inspects CancellationReasons to
distinguish transient contention (retry original transaction with
exponential backoff) from optimistic lock failures (consumption-only
retry path).

- Add _get_cancellation_reason_codes() and _is_transaction_conflict()
- Fix _is_condition_check_failure() to inspect CancellationReasons
- Add TransactionConflict retry loop (3 retries, 25/50/100ms backoff)
- Fix _build_retry_failure_statuses() to compute retry_after_seconds
  from bucket state via calculate_retry_after()

Fixes #332

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@sodre sodre added this to the v0.10.0 milestone Feb 16, 2026
@sodre sodre added the area/limiter Core rate limiting logic label Feb 16, 2026
@codecov

codecov Bot commented Feb 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.93%. Comparing base (8e129fe) to head (25a50bb).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #401      +/-   ##
==========================================
+ Coverage   91.89%   91.93%   +0.03%     
==========================================
  Files          33       33              
  Lines        7437     7473      +36     
==========================================
+ Hits         6834     6870      +36     
  Misses        603      603              
Flag Coverage Δ
doctest 29.41% <27.27%> (-0.03%) ⬇️
e2e 42.31% <38.63%> (-0.06%) ⬇️
integration 51.27% <38.63%> (-0.11%) ⬇️
unit 91.66% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sodre sodre marked this pull request as ready for review February 16, 2026 17:10
@sodre sodre merged commit f8582a0 into main Feb 16, 2026
25 checks passed
@sodre sodre deleted the fix/332-transaction-conflict-retry branch February 16, 2026 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/limiter Core rate limiting logic

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 TransactionConflict incorrectly treated as ConditionalCheckFailed in _commit_initial retry path

1 participant