Repository navigation
Conversation
…-After value The backoff wait generator read the retry information from the first failed response only, so later retries ignored the Retry-After or X-Ratelimit-Reset sent with the most recent 429, and kept retrying after a non-429 error.
|
@yoarajota is attempting to deploy a commit to the Flagsmith Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. Walkthrough
Merge Risk:
 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 2 files
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval skipped because this PR is from an external contributor.
View guided diff | Re-trigger cubic
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8965 +/- ##
==========================================
- Coverage 98.91% 98.04% -0.88%
==========================================
Files 1708 1708
Lines 71826 71838 +12
==========================================
- Hits 71046 70431 -615
- Misses 780 1407 +627 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
docs/if required so people know about the feature.Changes
launch_darkly_backoffhandsbackoffa wait generator.backoffcallswait.send(exc)after every failed attempt, but_wait_genassignedexconce, before its loop, and never from the value returned byyield retry_after. Every retry therefore recomputed its wait from the first failed response.On
main(910356d), with a mocked endpoint that answers 429 withRetry-After: 1, then 5, then 20, then 200, the client sleeps for 1, 1 and 1 seconds (plus up to one second of jitter) instead of 1, 5 and 20. It sends two requests earlier than LaunchDarkly asked. Each can hit the limit again and use up one of theBACKOFF_MAX_RETRIESattempts. The comment above the function says the client must wait the time given inRetry-AfterorX-Ratelimit-Reset.The same stale value decides whether to retry at all. A 429 followed by a 500 keeps retrying the 500 for as long as the first response said 429.
_wait_gennow reads each sent exception:exc = yield retry_after. A response that is not a 429 ends the generator, sobackoffgives up and_handle_giveupre-raises that error. A 500 after a 429 now fails immediately instead of being retried.I did not touch
Retry-After: 0, which is falsy and is not retried. It is a separate case.Review effort: 1/5
How did you test this code?
Added two tests to
tests/unit/integrations/launch_darkly/test_client.py, usingrequests_mockand a patchedbackoff._sync.time.sleep:test_launch_darkly_client__rate_limit_changing_retry_after__waits_latest_value: 429 withRetry-After1, 5, 20, then 200. Asserts the waits are 1, 5 and 20 seconds before jitter.test_launch_darkly_client__rate_limit_then_server_error__raises_without_retry: 429, 500, 200. Asserts anHTTPErrorwith status 500 after two calls.With
client.pychecked out frommain, both fail ([1, 1, 1] == [1, 5, 20], andDID NOT RAISE) and the other 14 client tests pass. With the fix, all 16 pass.Also ran, against a local Postgres 15.5:
pytest tests/unit/integrations/launch_darkly/: 67 passed.mypy integrations/launch_darkly tests/unit/integrations/launch_darkly/test_client.py: no issues. The newyieldcarries atype: ignore[assignment]like the existing one, becausebackoff.on_exceptiontypeswait_genasGenerator[float, None, None].ruff checkandruff format --checkon both paths: clean.The full
make testsuite andpre-commitwere not run.