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 4 commits into
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
…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.
Copy link
Copy Markdown
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.py are almost identical; consider parameterizing over the API base (official/Azure) to reduce duplication while still asserting client.max_retries == 0 for both.
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>Sourcery is free for open source - if you like our reviews please consider sharing them ✨ Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Sorry, something went wrong.
Comment thread
tests/test_openai_source.py
Outdated
…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
Reviewers
At least 1 approving review is required to merge this pull request.
1 participant
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_retries with the OpenAI SDK's built-in retry, which caused more HTTP requests than configured. Passing max_retries=0 to both AsyncOpenAI and AsyncAzureOpenAI makes AstrBot's retry_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。两层重试相乘：
The clients were created with only timeout, inheriting the SDK default max_retries=2, while _query()/_query_stream() wrap chat.completions.create in retry_provider_request() (tenacity, stop_after_attempt(request_max_retries)). The two retry layers multiply:
改动 / 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: pass max_retries=0 to both the AsyncAzureOpenAI and AsyncOpenAI constructors.
tests/test_openai_source.py: add construction tests for the official and Azure branches asserting the client type and client.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.py and tests/test_request_retry.py pass.
ruff check passes.
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: