fix(retry): distinguish durable quota exhaustion from hourly limits

[karlwaldman]

Fixes #77.

Measured problem

Over the cited 30-day cohort, free Python traffic saw 429s on 26.2% of requests versus 15.6% for free Node traffic. Paid cohorts were broadly comparable (Python 5.2%, Node 2.6%), so this PR addresses one concrete Python amplification path without claiming the cohort delta has a single cause.

The client includes 429 in its default retry codes. max_retries=3 is implemented as three total request attempts, so one durable quota refusal could become three identical refusals before the caller received RateLimitError.

Contract correction

X-RateLimit-State is not sufficient by itself. The API deliberately emits all of the following for its recoverable hourly circuit breaker:

• X-RateLimit-State: exhausted
• X-RateLimit-Remaining: 0
• X-RateLimit-Window: hourly_circuit_breaker
• Retry-After: <hourly reset>

That behavior is pinned in the API contract test. The API's durable usage windows are counter-backed: daily_counter, monthly_counter, and trial_counter (source).

The earlier PR head incorrectly stopped every state=exhausted 429 and also fell back to remaining=0; that would have suppressed the existing retry path for the hourly circuit breaker.

Change

• should_retry() keeps its two-argument compatibility and accepts optional response headers.
• A 429 is treated as durable quota exhaustion only when state=exhausted is paired with daily_counter, monthly_counter, or trial_counter.
• Hourly circuit-breaker, enforcement-unavailable, missing, unknown, and future metadata fail open to the existing bounded retry behavior.
• Sync request(), sync request_with_headers(), and async request() pass response headers into the decision.
• Sync and async boundary tests prove durable quota exhaustion makes exactly one HTTP request.
• Removed invented state aliases, the ambiguous remaining=0 fallback, and an unused retry_after_seconds() helper from the prior head.

This PR does not change or overclaim the existing wait policy: callers still cap an individual Retry-After sleep at 60 seconds. Full automatic recovery from a longer hourly reset is outside this narrow fix.

Red / green evidence

Adversarial tests added on top of prior head d65c942a739052d4924102c89b7864c12370a7a9 failed 4/23: hourly circuit breaker, state-only exhaustion, remaining-only exhaustion, and unknown future windows were all incorrectly suppressed.

This PR is stacked directly on exact #81 head 81fe122fbc4fbc27e986cc3cb0743381ecbf5a90. Its merge base and parent are that exact commit; the PR adds one retry-only commit across six files:

• current head: 7e58ef9d26665b377677e4dda0024df67b5c66bd

• current tree: 7d74a83b16aad2231b99087ad6849d3ea8bb2fee

• Targeted strategy and sync/async client boundary tests: 29 passed.

• Full non-live suite: 408 passed, 13 skipped.

• Ruff: clean for all source plus the new test module.

• mypy: no issues in 46 source files.

• Storefront validator: 3 surfaces validated.

• Hosted live workflow: success.

• Hosted Python 3.8, 3.9, 3.10, 3.11, and 3.12: all success.

The hosted runs were attached to pre-squash head 04c70d4d8a5d064d49ff6e59668634836a90b4c0; that commit and final one-commit head 7e58ef9 have the identical tree 7d74a83b16aad2231b99087ad6849d3ea8bb2fee. Squashing removed misleading historical commit messages and changed no files.

Release gate

This is not released. Merge and observe #81's hourly scheduled synthetic before landing this dependent PR or using it as release evidence. Also retain the version-attribution caveat in OilpriceAPI/oilpriceapi-api#6434: sdk_version coverage was only 0.61%, so post-release adoption cannot yet be measured reliably by SDK version.

[coderabbitai]

Warning

Review limit reached

@karlwaldman, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 59 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 45b6e11a-d93b-4ffb-bf56-8b6329b73185

📥 Commits

Reviewing files that changed from the base of the PR and between 47266b1 and faa072a.

📒 Files selected for processing (6)

• oilpriceapi/async_client.py
• oilpriceapi/client.py
• oilpriceapi/retry.py
• tests/test_client.py
• tests/test_retry_remedy.py
• tests/unit/test_async_client.py

📝 Walkthrough

Walkthrough

The retry strategy now receives response headers, detects exhausted rate-limit quotas, parses capped Retry-After values, and supports case-insensitive header names. Synchronous and asynchronous clients pass headers for 429 and 5xx retry decisions. Tests cover these behaviors.

Changes

Header-aware retry handling

Layer / File(s)	Summary
Retry strategy and validation oilpriceapi/retry.py, tests/test_retry_remedy.py	RetryStrategy.should_retry accepts optional headers. Quota exhaustion suppresses 429 retries. The strategy parses rate-limit states, remaining quota, and capped Retry-After values. Tests cover fallback behavior, compatibility, retry limits, and invalid headers.
Client retry wiring oilpriceapi/client.py, oilpriceapi/async_client.py	Synchronous and asynchronous retry paths pass response headers to should_retry for 429 and 5xx responses.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant HTTPResponse
  participant RetryStrategy
  Client->>HTTPResponse: receive 429 or 5xx response
  Client->>RetryStrategy: should_retry(attempt, status_code, headers)
  RetryStrategy->>RetryStrategy: evaluate quota headers
  RetryStrategy-->>Client: retry or stop

Loading

Possibly related PRs

• OilpriceAPI/python-sdk#72: Extends the same retry call sites to use response headers and supports related retry metadata.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The changes satisfy the coding objectives in issue #77, including header-aware retries, quota detection, Retry-After handling, and compatibility.
Out of Scope Changes check	✅ Passed	All changes are limited to retry handling, client header propagation, and related tests described in issue #77.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: distinguishing durable quota exhaustion from temporary hourly rate limits during retry handling.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-retry-remedy

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/test_retry_remedy.py (1)

81-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the client retry-delay paths.

These tests call RetryStrategy.retry_after_seconds directly. Add synchronous and asynchronous client tests that verify an invalid Retry-After value falls back to calculate_wait_time instead of using the raw header value.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_retry_remedy.py` around lines 81 - 102, Add synchronous and
asynchronous client tests covering invalid Retry-After handling in the client
retry flow. Verify that when RetryStrategy.retry_after_seconds receives an
invalid header, the clients call calculate_wait_time and use its result instead
of the raw Retry-After value.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@oilpriceapi/retry.py`:
- Around line 115-144: Use one safe Retry-After parser across all retry paths:
update RetryStrategy.retry_after_seconds in oilpriceapi/retry.py:115-144 to
return None for non-finite or negative values; update both retry paths in
oilpriceapi/client.py:277-281 and 391-395 and AsyncOilPriceAPI.request in
oilpriceapi/async_client.py:241-245 to call
retry_after_seconds(response.headers) and fall back to exponential backoff only
when it returns None; add sync and async coverage for invalid -5 and NaN headers
in tests/test_retry_remedy.py:81-102.

---

Nitpick comments:
In `@tests/test_retry_remedy.py`:
- Around line 81-102: Add synchronous and asynchronous client tests covering
invalid Retry-After handling in the client retry flow. Verify that when
RetryStrategy.retry_after_seconds receives an invalid header, the clients call
calculate_wait_time and use its result instead of the raw Retry-After value.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f11dd41-c8cb-400c-85f8-c3209f673df9

📥 Commits

Reviewing files that changed from the base of the PR and between f7b9263 and 47266b1.

📒 Files selected for processing (4)

• oilpriceapi/async_client.py
• oilpriceapi/client.py
• oilpriceapi/retry.py
• tests/test_retry_remedy.py