Repository navigation
fix(llms): don't stack the sdk's retries on rate-limit replays - #8333
simin-yuan wants to merge 3 commits into
Conversation
Strix Security ReviewWarning This pull request has 114 commits after the last Strix review ( No security issues found. Review summaryThe diff only adds a replay marker (a boolean per-attempt Updated for Reviewed by Strix |
|
Per CONTRIBUTING, this was created with AI assistance; could a maintainer please add the llm-generated label? (Contributors can't set labels.) |
run_with_rate_limit_retry replays a throttled call up to three times, and each replay went out with the provider SDK's own retries still enabled (max_retries defaults to 2), so one 429 turned into 3x3 requests on the wire and 3x3x3 once the agent-level retry is included: 27 byte-identical requests for a single call in the benchmark. Mark the attempts that replay an already-failed call and let the OpenAI provider build its client with max_retries=0 for those. The first attempt still uses the SDK's retries.
4983b3e to
5156ac0
Compare
|
Rebased onto current main (5bb6f23). The two commits are unchanged in content, only their parent moved. On local verification, being precise about what I could and could not run here:
CI is the honest judge for the rest. Could a maintainer approve the workflow run for this head? It has been sitting in |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5156ac0. Configure here.
The previous commit routed the plain chat completions through _client_for_attempt(), so a rate-limit replay no longer stacks the SDK's three retries on top of CrewAI's own retry budget. The structured-output branches reach the API through the beta helpers and were missed: * beta.chat.completions.with_raw_response.parse (sync) * beta.chat.completions.stream (sync, streaming) * beta.chat.completions.with_raw_response.parse (async) Against a local server that always answers 429, one replay of each of those calls issues 4 requests with the SDK retries in force and 1 with them disabled, so the stacking was real on all three paths. All three now go through _client_for_attempt() and keep the SDK's retries on the first attempt only. The tests record the retry budget that reaches the beta helpers and fail on the previous commit.

Related issue
Closes #8332
Summary
run_with_rate_limit_retry/arun_with_rate_limit_retrynow record whether the attempt they are aboutto run is a replay of a call that already failed (
_llm_retry_replaycontextvar, exposed asin_rate_limit_retry_replay()), and the OpenAI provider uses that to build its client:All nine provider call sites in
lib/crewai/src/crewai/llms/providers/openai/completion.pygo through_client_for_attempt(...): 4responses.create, 3chat.completions.create, and 2chat.completions.with_raw_response.create(in_handle_completionand_ahandle_completion, i.e. thenon-streaming path).
Why: the two layers multiply today.
max_retriesdefaults to2atcompletion.py:376, so the SDK makesup to 3 HTTP attempts per call; CrewAI then retries the same call up to 3 times (
retry.py:35, wrapperinstalled at
base_llm.py:201/:215). Measured oncrewai==1.15.28against a server that always answers429: 27 byte-identical requests for one call(https://github.com/simin-yuan/agent-double-exec-bench,
results/e1/crewai/429/server.jsonl; the SDK'sx-stainless-retry-countcycles 0→1→2 nine times).Measured, before and after
Same harness, same fault server, CrewAI default configuration,
Retry-After: 1on the 429 cell. "Before"is base commit
dec075b5, "after" is this branch head:Retry-After: 1)x-stainless-retry-count0 ×9, 1 ×9, 2 ×9)The 15 is counted in the server log, not derived: the first attempt still spends the SDK's three attempts
(that is the cheap way to ride out a one-off blip), and the two replays CrewAI makes now spend one request
each — 3 rounds × (3 + 1 + 1). The 500 cell is unchanged because CrewAI never replays a 500 at the model
layer, so there is no replay for the SDK's retries to stack on.
Logs:
results/e3/crewai-before/andresults/e3/crewai-after/inhttps://github.com/simin-yuan/agent-double-exec-bench (
429and500-neutralcells,server.jsonl;chat/completionslines only).Verification
lib/crewai/tests/llms/test_retry.py, six new tests (eight cases):test_replays_of_a_throttled_call_are_marked_as_replays[False, False]test_async_replays_of_a_throttled_call_are_marked_as_replays[False, False]test_the_replay_flag_is_off_outside_a_retrytest_openai_provider_turns_sdk_retries_off_on_replaytest_non_streaming_chat_keeps_sdk_retries_for_the_first_attempt_only[True]max_retries=2test_async_non_streaming_chat_keeps_sdk_retries_for_the_first_attempt_only[True]max_retries=2The non-streaming pair also runs with
[False](first-attempt case) and passes in both directions — thathalf is the control: the patch must not touch the first attempt. With only
completion.pyreverted to itsparent version, the run is
2 failed, 2 passed: the two replay cases fail, the two controls pass.The replay tests drive the real code path (
BaseLLM.__init_subclass__wrapper → retry loop) and assert thesequence
[False, True]; the provider test asserts the client is asked formax_retries=0on a replay andleft untouched otherwise.
Local runs (Windows, Python 3.12, venv with
uv sync --all-groupsplus the provider extras):The 18 and the 138 are environmental, and I checked that rather than assuming it: running those exact test
ids with
lib/checked out atdec075b5reproduces them 1:1 (18 failed, 14.95 s / 138 failed). The 11errors are collection errors for optional dependencies that are not installed locally (
a2a,qdrant).The whole-tree mypy run also needs the full dev extras; on the two files this PR touches it is clean under
the same strict configuration, unchanged from
dec075b5.On Windows the repo's
--block-networkaddopts blocks the local socket asyncio needs, which makes everyasync test — pre-existing ones included — error out at setup; the runs above pass
--allowed-hosts=127.0.0.1for that reason only. CI on Linux does not have this problem and is the number to trust.
Additional context
Deliberately not touched here:
Agent._times_executednever being reset (agent/core.py:242,:886-887) — real, but it is asemantics change for
max_retry_limit, so it is reported in the linked issue instead;_handle_execution_error→execute_task), which produces the 0.02 sre-sends;
generic (
in_rate_limit_retry_replay()), so they can opt in one at a time.Disclosure: I directed this work. The source reading, the patch, its tests and the measurements were
produced by AI assistants I operate. Every number above comes from the server logs in
agent-double-exec-bench, and I will answer review comments on it.
Per CONTRIBUTING this should carry the
llm-generatedlabel. Contributors can't set labels, so I asked forit in a comment below.
Related: #7771 handles Retry-After on SDK response objects; this PR changes a different part of the same
files (the SDK's own retries under CrewAI's rate-limit replays). The two do not touch the same lines.
Note
Medium Risk
Changes shared LLM rate-limit retry semantics and every OpenAI HTTP entry point, but the scope is narrow (replay-only SDK disable) and is heavily covered by new tests.
Overview
Fixes multiplied HTTP retries when CrewAI’s rate-limit wrapper replays a failed OpenAI call while the OpenAI SDK still applies its own
max_retries.CrewAI’s
run_with_rate_limit_retry/arun_with_rate_limit_retrynow expose a replay signal viain_rate_limit_retry_replay()(backed by a context var set on attempts after the first). The OpenAI provider adds_client_for_attempt, which keeps the normal client on the first attempt but usesclient.with_options(max_retries=0)on replays. Chat completions, structured-output, streaming, and Responses API paths all route through that helper.The first throttled attempt still gets the SDK’s full retry budget; replays issue one HTTP try each, cutting stacked 429 traffic (e.g. 27 → 15 in the PR’s harness) without changing behavior for non-throttling errors. New tests in
test_retry.pycover the replay flag and OpenAI retry budgeting.Reviewed by Cursor Bugbot for commit b581eb2. Bugbot is set up for automated code reviews on this repo. Configure here.