|
| 1 | +"""Retry must distinguish "you are bursting" from "you are out of quota". |
| 2 | +
|
| 3 | +Measured against production over 30 days: free accounts on this SDK were |
| 4 | +rate-limited on 26.2% of requests against 15.6% for the Node SDK on the same |
| 5 | +tier. `should_retry` took the status code alone, so a quota-exhausted 429 -- |
| 6 | +which cannot succeed until the billing period resets -- was tried three times, |
| 7 | +turning one refusal into three. |
| 8 | +
|
| 9 | +The API distinguishes the two cases with the combination of |
| 10 | +`X-RateLimit-State` and `X-RateLimit-Window` (oilpriceapi-api#5664). The state |
| 11 | +alone is ambiguous: both a durable quota wall and the recoverable hourly |
| 12 | +circuit breaker use `exhausted`. |
| 13 | +""" |
| 14 | + |
| 15 | +import pytest |
| 16 | + |
| 17 | +from oilpriceapi.retry import RetryStrategy |
| 18 | + |
| 19 | + |
| 20 | +@pytest.fixture |
| 21 | +def strategy(): |
| 22 | + return RetryStrategy(max_retries=3, retry_on=[429, 500, 502, 503, 504]) |
| 23 | + |
| 24 | + |
| 25 | +class TestQuotaExhaustedIsNotRetried: |
| 26 | + @pytest.mark.parametrize("window", ["daily_counter", "monthly_counter", "trial_counter"]) |
| 27 | + def test_durable_quota_windows_stop_the_retry(self, strategy, window): |
| 28 | + headers = { |
| 29 | + "X-RateLimit-State": "exhausted", |
| 30 | + "X-RateLimit-Window": window, |
| 31 | + "X-RateLimit-Remaining": "0", |
| 32 | + } |
| 33 | + assert strategy.should_retry(0, 429, headers) is False |
| 34 | + |
| 35 | + def test_header_name_is_matched_case_insensitively(self, strategy): |
| 36 | + # HTTP header names are case-insensitive and clients normalise them |
| 37 | + # differently. Matching on exact case would silently disable this. |
| 38 | + headers = { |
| 39 | + "x-ratelimit-state": "EXHAUSTED", |
| 40 | + "x-ratelimit-window": "MONTHLY_COUNTER", |
| 41 | + } |
| 42 | + assert strategy.should_retry(0, 429, headers) is False |
| 43 | + |
| 44 | + |
| 45 | +class TestBurstingIsStillRetried: |
| 46 | + def test_hourly_circuit_breaker_preserves_retry_behavior(self, strategy): |
| 47 | + # The API deliberately emits `state=exhausted` for this recoverable |
| 48 | + # safety limit. Looking at state or remaining alone would suppress the |
| 49 | + # existing bounded retry path even though this is not a durable quota. |
| 50 | + headers = { |
| 51 | + "X-RateLimit-State": "exhausted", |
| 52 | + "X-RateLimit-Window": "hourly_circuit_breaker", |
| 53 | + "X-RateLimit-Remaining": "0", |
| 54 | + "Retry-After": "1050", |
| 55 | + } |
| 56 | + assert strategy.should_retry(0, 429, headers) is True |
| 57 | + |
| 58 | + @pytest.mark.parametrize( |
| 59 | + "headers", |
| 60 | + [ |
| 61 | + {"X-RateLimit-State": "exhausted"}, |
| 62 | + {"X-RateLimit-Remaining": "0"}, |
| 63 | + { |
| 64 | + "X-RateLimit-State": "unavailable", |
| 65 | + "X-RateLimit-Window": "enforcement_check", |
| 66 | + }, |
| 67 | + { |
| 68 | + "X-RateLimit-State": "exhausted", |
| 69 | + "X-RateLimit-Window": "future_counter_contract", |
| 70 | + }, |
| 71 | + ], |
| 72 | + ) |
| 73 | + def test_ambiguous_or_recoverable_metadata_fails_open(self, strategy, headers): |
| 74 | + assert strategy.should_retry(0, 429, headers) is True |
| 75 | + |
| 76 | + def test_retries_when_headers_are_absent(self, strategy): |
| 77 | + # The critical safety property. An unknown state must behave exactly as |
| 78 | + # it did before this change, so a missing header can never convert a |
| 79 | + # retryable burst into a hard failure. |
| 80 | + assert strategy.should_retry(0, 429, None) is True |
| 81 | + assert strategy.should_retry(0, 429, {}) is True |
| 82 | + |
| 83 | + def test_server_errors_retry_regardless_of_rate_limit_headers(self, strategy): |
| 84 | + # A 500 carries no remedy. Exhausted allowance must not suppress it. |
| 85 | + headers = {"X-RateLimit-State": "exhausted"} |
| 86 | + for code in (500, 502, 503, 504): |
| 87 | + assert strategy.should_retry(0, code, headers) is True |
| 88 | + |
| 89 | + |
| 90 | +class TestExistingBehaviourUnchanged: |
| 91 | + def test_attempt_budget_still_respected(self, strategy): |
| 92 | + assert strategy.should_retry(2, 429, None) is False |
| 93 | + |
| 94 | + def test_non_retryable_status_still_not_retried(self, strategy): |
| 95 | + # 402 must never be retried: it is a payment problem, not a timing one. |
| 96 | + assert strategy.should_retry(0, 402, None) is False |
| 97 | + assert strategy.should_retry(0, 404, None) is False |
| 98 | + |
| 99 | + def test_two_argument_calls_still_work(self, strategy): |
| 100 | + # `headers` is optional so third-party callers of this public method do |
| 101 | + # not break on upgrade. |
| 102 | + assert strategy.should_retry(0, 500) is True |
| 103 | + assert strategy.should_retry(0, 404) is False |
| 104 | + |
| 105 | + |
| 106 | +class TestTheProductionScenario: |
| 107 | + def test_a_free_account_out_of_quota_makes_exactly_one_request(self, strategy): |
| 108 | + """The defect, stated as a test. |
| 109 | +
|
| 110 | + A free account that has spent its 200 daily requests previously issued |
| 111 | + 1 request + 2 retries = 3 refusals per call. It must now issue 1. |
| 112 | + """ |
| 113 | + headers = { |
| 114 | + "X-RateLimit-Limit": "200", |
| 115 | + "X-RateLimit-Remaining": "0", |
| 116 | + "X-RateLimit-State": "exhausted", |
| 117 | + "X-RateLimit-Window": "daily_counter", |
| 118 | + } |
| 119 | + attempts = sum( |
| 120 | + 1 |
| 121 | + for attempt in range(strategy.max_retries) |
| 122 | + if strategy.should_retry(attempt, 429, headers) |
| 123 | + ) |
| 124 | + assert attempts == 0, "a quota-exhausted 429 must not be retried at all" |
0 commit comments