fix: add LLM metadata fallback endpoint - #9725
Open
wcqqq1214 wants to merge 2 commits into
Open
Conversation
wcqqq1214
marked this pull request as ready for review
August 18, 2026 05:27
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The nested try/except around the session and then each URL currently catches bare
Exceptiontwice and only logs once at the end; consider collapsing to a single try/except per-URL with more specific exception types and per-URL logging to make failures easier to diagnose. - In the fallback loop,
last_erroris always overwritten on each failure; if multiple URLs fail it may be more useful to either accumulate errors or log them as they occur so the final log message retains more context than just the last failure.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The nested try/except around the session and then each URL currently catches bare `Exception` twice and only logs once at the end; consider collapsing to a single try/except per-URL with more specific exception types and per-URL logging to make failures easier to diagnose.
- In the fallback loop, `last_error` is always overwritten on each failure; if multiple URLs fail it may be more useful to either accumulate errors or log them as they occur so the final log message retains more context than just the last failure.
## Individual Comments
### Comment 1
<location path="astrbot/core/utils/llm_metadata.py" line_range="44-53" />
<code_context>
+ for url in LLM_METADATA_URLS:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use more specific exception handling and add URL context to errors.
Catching bare `Exception` both in the loop and outer `try` obscures which failures you actually expect and can mask real bugs. Prefer catching the specific network/JSON exceptions you anticipate (e.g. `aiohttp.ClientError`, `asyncio.TimeoutError`, `ValueError` for JSON), and include the URL in the log message so intermittent endpoint issues are diagnosable (e.g. `logger.warning(f"Failed to fetch LLM metadata from {url}: {e}")`). Reserve the outer handler for truly unexpected errors.
Suggested implementation:
```python
for url in LLM_METADATA_URLS:
try:
async with session.get(url) as response:
response.raise_for_status()
data = await response.json()
except (aiohttp.ClientError, asyncio.TimeoutError, ValueError) as e:
logger.warning(f"Failed to fetch LLM metadata from {url}: {e}")
continue
models = {}
for info in data.values():
for model in info.get("models", {}).values():
model_id = model.get("id")
if not model_id:
```
1. Ensure `aiohttp`, `asyncio`, and the `logger` used here are imported in this module (e.g. `import asyncio`, `import aiohttp`, and a module-level `logger = logging.getLogger(__name__)` if not already present).
2. Update the outer `try/except` (not shown in the snippet) to avoid catching bare `Exception`; either remove it or have it catch only truly unexpected errors, and log them without duplicating the per-URL warning.
</issue_to_address>
### Comment 2
<location path="astrbot/core/utils/llm_metadata.py" line_range="39-48" />
<code_context>
+ async with session.get(url) as response:
</code_context>
<issue_to_address>
**suggestion:** Avoid clearing the global cache on each successful URL without indicating which source won.
Because the first successful URL populates `LLM_METADATAS` and returns, future changes like preferred ordering or per-source semantics could be confusing. Adding a log of which URL succeeded (e.g. `logger.info(f"Successfully fetched metadata for {len(models)} LLMs from {url}.")`) will make it clear in production which metadata source is actually in use and simplifies debugging primary vs fallback behavior.
</issue_to_address>
### Comment 3
<location path="astrbot/core/utils/llm_metadata.py" line_range="37" />
<code_context>
+)
async def update_llm_metadata() -> None:
- url = "https://models.dev/api.json"
+ global LLM_METADATAS
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the HTTP fetch and JSON parsing into a helper so `update_llm_metadata` only orchestrates URL retries and cache updates, simplifying its control flow.
You can keep the multiple-URL fallback while simplifying control flow and separating concerns by extracting the HTTP+JSON fetching into a helper and making `update_llm_metadata` primarily orchestrate URLs and cache updates.
For example:
```python
LLM_METADATA_URLS = (
"https://models.dev/api.json",
"https://models.opencode.ai/api.json",
)
async def _fetch_models_from_url(session: aiohttp.ClientSession, url: str) -> dict[str, LLMMetadata]:
async with session.get(url) as response:
response.raise_for_status()
data = await response.json()
models: dict[str, LLMMetadata] = {}
for info in data.values():
for model in info.get("models", {}).values():
model_id = model.get("id")
if not model_id:
continue
models[model_id] = LLMMetadata(
id=model_id,
reasoning=model.get("reasoning", False),
tool_call=model.get("tool_call", False),
knowledge=model.get("knowledge", "none"),
release_date=model.get("release_date", ""),
modalities=model.get("modalities", {"input": [], "output": []}),
open_weights=model.get("open_weights", False),
limit=model.get("limit", {"context": 0, "output": 0}),
)
return models
```
Then `update_llm_metadata` becomes:
```python
async def update_llm_metadata() -> None:
global LLM_METADATAS
last_error: Exception | None = None
async with aiohttp.ClientSession(
trust_env=True, connector=build_tls_connector()
) as session:
for url in LLM_METADATA_URLS:
try:
models = await _fetch_models_from_url(session, url)
except Exception as e:
last_error = e
continue
LLM_METADATAS.clear()
LLM_METADATAS.update(models)
logger.info(f"Successfully fetched metadata for {len(models)} LLMs.")
return
logger.error(f"Failed to fetch LLM metadata: {last_error}")
```
Benefits:
- Single `try` per URL; no nested `try` layers.
- `last_error` is only managed in one place (inside the loop).
- Parsing logic is isolated in `_fetch_models_from_url`, reducing cognitive load in `update_llm_metadata`.
- The main function now clearly reads as: create session → iterate URLs → on first success, update cache and return → after loop, log last error.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| ) | ||
|
|
||
|
|
||
| async def update_llm_metadata() -> None: |
Contributor
There was a problem hiding this comment.
issue (complexity): Consider extracting the HTTP fetch and JSON parsing into a helper so update_llm_metadata only orchestrates URL retries and cache updates, simplifying its control flow.
You can keep the multiple-URL fallback while simplifying control flow and separating concerns by extracting the HTTP+JSON fetching into a helper and making update_llm_metadata primarily orchestrate URLs and cache updates.
For example:
LLM_METADATA_URLS = (
"https://models.dev/api.json",
"https://models.opencode.ai/api.json",
)
async def _fetch_models_from_url(session: aiohttp.ClientSession, url: str) -> dict[str, LLMMetadata]:
async with session.get(url) as response:
response.raise_for_status()
data = await response.json()
models: dict[str, LLMMetadata] = {}
for info in data.values():
for model in info.get("models", {}).values():
model_id = model.get("id")
if not model_id:
continue
models[model_id] = LLMMetadata(
id=model_id,
reasoning=model.get("reasoning", False),
tool_call=model.get("tool_call", False),
knowledge=model.get("knowledge", "none"),
release_date=model.get("release_date", ""),
modalities=model.get("modalities", {"input": [], "output": []}),
open_weights=model.get("open_weights", False),
limit=model.get("limit", {"context": 0, "output": 0}),
)
return modelsThen update_llm_metadata becomes:
async def update_llm_metadata() -> None:
global LLM_METADATAS
last_error: Exception | None = None
async with aiohttp.ClientSession(
trust_env=True, connector=build_tls_connector()
) as session:
for url in LLM_METADATA_URLS:
try:
models = await _fetch_models_from_url(session, url)
except Exception as e:
last_error = e
continue
LLM_METADATAS.clear()
LLM_METADATAS.update(models)
logger.info(f"Successfully fetched metadata for {len(models)} LLMs.")
return
logger.error(f"Failed to fetch LLM metadata: {last_error}")Benefits:
- Single
tryper URL; no nestedtrylayers. last_erroris only managed in one place (inside the loop).- Parsing logic is isolated in
_fetch_models_from_url, reducing cognitive load inupdate_llm_metadata. - The main function now clearly reads as: create session → iterate URLs → on first success, update cache and return → after loop, log last error.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Motivation
Fixes #9719.
AstrBot fetches LLM metadata from
models.dev, which is not reachable in some network environments. The officialmodels.opencode.aiendpoint serves the same catalog and can be used as a fallback.Modifications / 改动点
Add
https://models.opencode.ai/api.jsonas the fallback endpoint aftermodels.dev.Retry on request errors, non-success HTTP responses, and JSON parsing errors.
Keep the existing in-place metadata cache update behavior.
Add unit tests for the primary endpoint and fallback path.
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图和测试结果
Backend-only change; screenshots are not applicable.
Verification:
The fallback endpoint
https://models.opencode.ai/api.jsonwas manually verified to be reachable without a proxy and to return HTTP 200 with valid JSON containing 191 providers.Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Make LLM metadata updates resilient by falling back to the OpenCode catalog when the primary endpoint cannot be reached.
Bug Fixes:
Tests: