fix(LaunchDarkly): Rate limited requests retry using the latest Retry-After value

YO
1d ago
+62-5
  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

launch_darkly_backoff hands backoff a wait generator. backoff calls wait.send(exc) after every failed attempt, but _wait_gen assigned exc once, before its loop, and never from the value returned by yield retry_after. Every retry therefore recomputed its wait from the first failed response.

On main (910356dea), with a mocked endpoint that answers 429 with Retry-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 the BACKOFF_MAX_RETRIES attempts. The comment above the function says the client must wait the time given in Retry-After or X-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_gen now reads each sent exception: exc = yield retry_after. A response that is not a 429 ends the generator, so backoff gives up and _handle_giveup re-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, using requests_mock and a patched backoff._sync.time.sleep:

  • test_launch_darkly_client__rate_limit_changing_retry_after__waits_latest_value: 429 with Retry-After 1, 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 an HTTPError with status 500 after two calls.

With client.py checked out from main, both fail ([1, 1, 1] == [1, 5, 20], and DID 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 new yield carries a type: ignore[assignment] like the existing one, because backoff.on_exception types wait_gen as Generator[float, None, None].
  • ruff check and ruff format --check on both paths: clean.

The full make test suite and pre-commit were not run.