Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions astrbot/core/config/default.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
"default_provider_id": "",
"fallback_chat_models": [],
"request_max_retries": 5,
"provider_error_retries": 1,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

⚠️ Critical Reliability & WebUI Configuration Issues

  1. Default Value of 1 Disables All Recovery Paths:
    Setting the default value of provider_error_retries to 1 means the outer retry loop runs exactly once (i.e., 0 retries). As a result, all provider-level recovery paths (such as context length trimming, image fallback, key rotation, and tool removal) are completely disabled by default. If any of these errors occur on the first attempt, the request will immediately fail without attempting recovery.

    To preserve these robust recovery features out-of-the-box, consider keeping the default value higher (e.g., 3 or 5), while still allowing proxy/aggregator users to configure it down to 1 if they want to avoid nested retries.

  2. Missing from CONFIG_METADATA_3:
    This setting is not added to CONFIG_METADATA_3 in default.py. Since CONFIG_METADATA_3 is used to render the settings in the WebUI, users will not be able to see or configure this setting from the admin panel. Please add it to CONFIG_METADATA_3 under ai_group -> metadata -> ai -> items and update the corresponding i18n locale files (config-metadata.json).

"default_image_caption_provider_id": "",
"image_caption_prompt": "Please describe the image using Chinese.",
"provider_pool": ["*"], # "*" 表示使用所有可用的提供者
Expand Down Expand Up @@ -2803,6 +2804,9 @@
"request_max_retries": {
"type": "int",
},
"provider_error_retries": {
"type": "int",
},
"wake_prefix": {
"type": "string",
},
Expand Down
22 changes: 20 additions & 2 deletions astrbot/core/provider/sources/openai_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,24 @@ def __init__(self, provider_config, provider_settings) -> None:

self.reasoning_key = "reasoning_content"

def _provider_error_retries(self) -> int:
"""Return retry attempts for provider-level recovery paths.

This outer retry loop handles payload mutation and key rotation (for
example context trimming, tool removal, image fallback, or switching to
another configured API key). Transport/status-code retries are handled
separately by ``request_max_retries`` in ``retry_provider_request``.
Keeping this value configurable prevents nested retry loops from
multiplying latency for proxy/aggregator providers that already perform
their own upstream retry and fallback.
"""
raw = self.provider_settings.get("provider_error_retries", 1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Guard against self.provider_settings being None to prevent an AttributeError when calling .get().

        settings = self.provider_settings or {}
        raw = settings.get("provider_error_retries", 1)

try:
retries = int(raw)
except (TypeError, ValueError):
retries = 1
return max(1, retries)

def _ollama_disable_thinking_enabled(self) -> bool:
value = self.provider_config.get("ollama_disable_thinking", False)
if isinstance(value, str):
Expand Down Expand Up @@ -1188,7 +1206,7 @@ async def text_chat(
payloads["tool_choice"] = tool_choice

llm_response = None
max_retries = 10
max_retries = self._provider_error_retries()
available_api_keys = self.api_keys.copy()
chosen_key = random.choice(available_api_keys)
image_fallback_used = False
Expand Down Expand Up @@ -1264,7 +1282,7 @@ async def text_chat_stream(
if func_tool and not func_tool.empty():
payloads["tool_choice"] = tool_choice

max_retries = 10
max_retries = self._provider_error_retries()
available_api_keys = self.api_keys.copy()
chosen_key = random.choice(available_api_keys)
image_fallback_used = False
Expand Down
16 changes: 16 additions & 0 deletions tests/test_openai_source.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,22 @@ def fake_import(name, globals=None, locals=None, fromlist=(), level=0):
assert captured["httpx_module"] is openai_source_module.httpx


def test_provider_error_retries_defaults_and_coerces_values():
provider = ProviderOpenAIOfficial.__new__(ProviderOpenAIOfficial)

provider.provider_settings = {}
assert provider._provider_error_retries() == 1
Comment on lines +128 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add a test case to verify that _provider_error_retries handles provider_settings being None gracefully.

Suggested change
provider.provider_settings = {}
assert provider._provider_error_retries() == 1
provider.provider_settings = None
assert provider._provider_error_retries() == 1
provider.provider_settings = {}
assert provider._provider_error_retries() == 1


provider.provider_settings = {"provider_error_retries": "3"}
assert provider._provider_error_retries() == 3

provider.provider_settings = {"provider_error_retries": 0}
assert provider._provider_error_retries() == 1

provider.provider_settings = {"provider_error_retries": "invalid"}
assert provider._provider_error_retries() == 1


@pytest.mark.asyncio
async def test_get_models_retries_transient_request_error(monkeypatch):
monkeypatch.setattr(request_retry, "REQUEST_RETRY_WAIT_MIN_S", 0)
Expand Down
Loading