fix(provider): 关闭 OpenAI SDK 内建重试,避免与 AstrBot 重试层叠加导致请求数翻倍 | Disable OpenAI SDK built-in retries to prevent request multiplication from stacked retry layers - #9669
Open
SweetenedSuzuka wants to merge 3 commits into
Conversation
…y layer (AstrBotDevs#9663) AsyncOpenAI/AsyncAzureOpenAI inherit the SDK default max_retries=2, which nests underneath AstrBot's tenacity-based retry_provider_request(). As a result provider_settings.request_max_retries no longer reflects the true attempt count (e.g. request_max_retries=1 still sends up to 3 HTTP requests) and delays fallback-provider switchover. Pass max_retries=0 so AstrBot's retry layer is the single source of truth for retries.
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The two new async tests in
test_openai_source.pyare almost identical; consider parameterizing over the API base (official/Azure) to reduce duplication while still assertingclient.max_retries == 0for both.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The two new async tests in `test_openai_source.py` are almost identical; consider parameterizing over the API base (official/Azure) to reduce duplication while still asserting `client.max_retries == 0` for both.
## Individual Comments
### Comment 1
<location path="tests/test_openai_source.py" line_range="65-71" />
<code_context>
+@pytest.mark.asyncio
+async def test_openai_client_disables_sdk_builtin_retries():
+ provider = _make_provider()
+ try:
+ assert isinstance(provider.client, AsyncOpenAI)
+ assert provider.client.max_retries == 0
+ finally:
+ await provider.terminate()
+
+
</code_context>
<issue_to_address>
**issue (testing):** Add a test that demonstrates the retry behavior is now controlled solely by `retry_provider_request()` (no multiplicative retries).
This test only checks that `max_retries` is set to 0 on the client; it doesn’t verify the actual retry behavior. Because the bug involved stacked retries (up to 5×3 calls), we should add a test that asserts the number of attempts matches `request_max_retries`.
Concretely, configure a provider with `request_max_retries=2`, stub the underlying HTTP call or `client.chat.completions.create` to always raise a retryable error and count invocations, then call through `retry_provider_request()` and assert the call count is exactly 2. This will protect against future regressions where SDK retries or extra wrapping might be reintroduced.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…retry tests Address review feedback on AstrBotDevs#9669: merge the near-identical official/Azure client construction tests into a parametrized test, and add a regression test asserting _query() performs exactly request_max_retries attempts through retry_provider_request() when the call keeps failing, guarding against multiplicative retries being reintroduced.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
摘要 / Summary
修复 OpenAI provider 中
provider_settings.request_max_retries与 OpenAI SDK 内建重试相互嵌套、导致实际 HTTP 请求次数超出配置的问题。为AsyncOpenAI/AsyncAzureOpenAI显式传入max_retries=0,使 AstrBot 的retry_provider_request()成为唯一的重试控制源。Fixes the stacking of
provider_settings.request_max_retrieswith the OpenAI SDK's built-in retry, which caused more HTTP requests than configured. Passingmax_retries=0to bothAsyncOpenAIandAsyncAzureOpenAImakes AstrBot'sretry_provider_request()the single source of truth for retries.问题 / Problem
openai_source.py创建客户端时只设置了timeout,未覆盖 SDK 默认的max_retries=2;而_query()/_query_stream()外层又用retry_provider_request()(tenacity,stop_after_attempt(request_max_retries))包裹chat.completions.create。两层重试相乘:request_max_retries=1,单次 provider 请求仍可能发送 3 次 HTTP(1 次首次 + 2 次 SDK 重试);request_max_retries=5时最多可能产生 5×3=15 次请求。The clients were created with only
timeout, inheriting the SDK defaultmax_retries=2, while_query()/_query_stream()wrapchat.completions.createinretry_provider_request()(tenacity,stop_after_attempt(request_max_retries)). The two retry layers multiply:request_max_retries=1a single provider request could send 3 HTTP requests (1 initial + 2 SDK retries);request_max_retries=5up to 5×3=15 requests were possible.改动 / Changes
astrbot/core/provider/sources/openai_source.py:为AsyncAzureOpenAI与AsyncOpenAI构造各加max_retries=0。tests/test_openai_source.py:新增官方与 Azure 两个分支的构造测试,断言客户端类型正确且client.max_retries == 0。astrbot/core/provider/sources/openai_source.py: passmax_retries=0to both theAsyncAzureOpenAIandAsyncOpenAIconstructors.tests/test_openai_source.py: add construction tests for the official and Azure branches asserting the client type andclient.max_retries == 0.验证 / Verification
tests/test_openai_source.py、tests/test_request_retry.py:通过。ruff check:通过。端到端模拟(mock 500):
request_max_retries=1时,旧行为发出 3 次 HTTP,新行为仅 1 次。tests/test_openai_source.pyandtests/test_request_retry.pypass.ruff checkpasses.End-to-end simulation (mocked 500): with
request_max_retries=1, the old behavior issues 3 HTTP requests, the new behavior only 1.范围说明 / Scope
TTS / Whisper / Embedding 客户端同样继承 SDK 默认重试,但它们不经过
retry_provider_request,不存在嵌套问题,故不在本次修改范围内,保持不变。The TTS / Whisper / Embedding clients also inherit the SDK default retries, but they do not go through
retry_provider_request, so there is no nesting there; they are intentionally left untouched.Closes #9663
Summary by Sourcery
Disable OpenAI SDK built-in retries for chat providers so AstrBot’s retry logic solely controls request attempts.
Bug Fixes:
Tests: