fix: cannot resolve voice when using aiocqhttp adapter#8523
Conversation
Refactor file handling to resolve sources more effectively, including decoding file URIs and handling base64 data. Update methods to ensure compatibility with different file formats and paths.
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
_resolve_file_source, theself.urlbranch has redundant and partially unreachable logic (self.url.startswith("file:///")is checked twice and theand os.path.exists(self._decode_file_uri(self.url))clause will never be hit), so consider restructuring this block to clearly handlefile:///(decode + exists),http, and direct filesystem paths separately. _resolve_file_sourceis declaredasyncbut does not perform anyawaitor I/O, which adds unnecessary complexity to call sites; consider making it a synchronous helper (or adding a comment if you anticipate future async operations there).- Both
convert_to_file_pathandconvert_to_base64now rely onfile_sourcebut still report errors usingself.file(e.g.,raise Exception(f"not a valid file: {self.file}")), which can be misleading when the URL/path is actually coming fromurlorpath; it may be clearer to include the resolved source or all candidate fields in the error message.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_resolve_file_source`, the `self.url` branch has redundant and partially unreachable logic (`self.url.startswith("file:///")` is checked twice and the `and os.path.exists(self._decode_file_uri(self.url))` clause will never be hit), so consider restructuring this block to clearly handle `file:///` (decode + exists), `http`, and direct filesystem paths separately.
- `_resolve_file_source` is declared `async` but does not perform any `await` or I/O, which adds unnecessary complexity to call sites; consider making it a synchronous helper (or adding a comment if you anticipate future async operations there).
- Both `convert_to_file_path` and `convert_to_base64` now rely on `file_source` but still report errors using `self.file` (e.g., `raise Exception(f"not a valid file: {self.file}")`), which can be misleading when the URL/path is actually coming from `url` or `path`; it may be clearer to include the resolved source or all candidate fields in the error message.
## Individual Comments
### Comment 1
<location path="astrbot/core/message/components.py" line_range="174-179" />
<code_context>
+ return self.file
+
+ # 2) 尝试 url(可能是 file:/// 或 http 链接)
+ if self.url:
+ if (
+ self.url.startswith("file:///")
+ or self.url.startswith("http")
+ or os.path.exists(self.url)
+ or (
+ self.url.startswith("file:///")
+ and os.path.exists(self._decode_file_uri(self.url))
</code_context>
<issue_to_address>
**issue (bug_risk):** The `os.path.exists(self._decode_file_uri(self.url))` branch is effectively unreachable due to the preceding `self.url.startswith("file:///")` condition.
Because the `self.url.startswith("file:///")` check appears earlier in the `or` chain, Python short-circuits when it’s true and never evaluates the `and os.path.exists(self._decode_file_uri(self.url))` part.
If you want to only accept `file:///` URLs when the decoded local path exists, consider restructuring as:
```python
if self.url.startswith("file:///"):
decoded = self._decode_file_uri(self.url)
if os.path.exists(decoded):
return self.url # or decoded
elif self.url.startswith("http") or os.path.exists(self.url):
return self.url
```
This ensures the existence check is actually applied and removes dead logic.
</issue_to_address>
### Comment 2
<location path="astrbot/core/message/components.py" line_range="156" />
<code_context>
+ path = urllib.parse.unquote(path)
+ return path
+
+ async def _resolve_file_source(self) -> str:
+ """选择可用的文件源。
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new file resolution logic by making `_resolve_file_source` synchronous, extracting reusable helpers for source validation/classification, and removing unreachable branches so both conversion methods share the same streamlined flow.
The new `_resolve_file_source` / `_decode_file_uri` layer does centralize logic, but there are a few concrete spots where you can reduce complexity and fix a subtle bug without changing behavior.
### 1. `async` helper without `await`
`_resolve_file_source` is `async` but never awaits, which forces `convert_to_file_path` / `convert_to_base64` into async even for pure in‑memory decisions. You can make it synchronous and keep the public API unchanged:
```python
@staticmethod
def _decode_file_uri(uri: str) -> str:
path = uri.removeprefix("file:///")
return urllib.parse.unquote(path)
def _resolve_file_source(self) -> str:
# 1) Prefer file if it is already usable
if self.file and self._is_usable_source(self.file):
return self.file
# 2) Fallback to url
if self.url and self._is_usable_source(self.url):
return self.url
# 3) Fallback to path
if self.path and os.path.exists(self.path):
return self.path
# 4) Last resort: raw value for error messages
return self.file or self.url or ""
```
Then call it synchronously:
```python
async def convert_to_file_path(self) -> str:
file_source = self._resolve_file_source()
...
```
### 2. Redundant / unreachable condition
In the current `_resolve_file_source`:
```python
if self.url:
if (
self.url.startswith("file:///")
or self.url.startswith("http")
or os.path.exists(self.url)
or (
self.url.startswith("file:///")
and os.path.exists(self._decode_file_uri(self.url))
)
):
return self.url
```
The last branch `self.url.startswith("file:///") and ...` is never reached if `url.startswith("file:///")` is true, because the first `self.url.startswith("file:///")` already short‑circuits the `or`. This can be simplified and fixed by handling the decoded `file:///` case explicitly:
```python
def _is_usable_source(self, value: str) -> bool:
if value.startswith("file:///"):
decoded = self._decode_file_uri(value)
return os.path.exists(decoded)
if value.startswith(("http", "base64://")):
return True
return os.path.exists(value)
```
Then `_resolve_file_source` just delegates to `_is_usable_source` (as in the previous snippet), avoiding nested conditionals and unreachable code.
### 3. Reduce duplication between `convert_to_file_path` and `convert_to_base64`
Both methods still repeat the same “what kind of source is this?” branching. You can unify that into a small helper that returns a normalized “kind + value”, while keeping the async download intact:
```python
def _classify_source(self, source: str) -> tuple[str, str]:
if source.startswith("file:///"):
return "path", self._decode_file_uri(source)
if source.startswith("http"):
return "http", source
if source.startswith("base64://"):
return "base64", source.removeprefix("base64://")
return "path", source # bare filesystem path
```
Then both conversions become simpler and consistent:
```python
async def convert_to_file_path(self) -> str:
source = self._resolve_file_source()
if not source:
raise Exception(f"not a valid file: {self.file}")
kind, value = self._classify_source(source)
if kind == "http":
file_path = await download_image_by_url(value)
return os.path.abspath(file_path)
if kind == "base64":
image_bytes = base64.b64decode(value)
file_path = os.path.join(
get_astrbot_temp_path(), f"recordseg_{uuid.uuid4()}.wav"
)
with open(file_path, "wb") as f:
f.write(image_bytes)
return os.path.abspath(file_path)
# kind == "path"
if os.path.exists(value):
return os.path.abspath(value)
raise Exception(f"not a valid file: {self.file}")
```
```python
async def convert_to_base64(self) -> str:
source = self._resolve_file_source()
if not source:
raise Exception(f"not a valid file: {self.file}")
kind, value = self._classify_source(source)
if kind == "http":
file_path = await download_image_by_url(value)
bs64_data = file_to_base64(file_path)
elif kind == "base64":
bs64_data = value
else: # kind == "path"
if not os.path.exists(value):
raise Exception(f"not a valid file: {self.file}")
bs64_data = file_to_base64(value)
return bs64_data.removeprefix("base64://")
```
This keeps all existing behaviors (file/url/path precedence, `file:///` decoding, HTTP download, base64 handling) while:
- making `_resolve_file_source` single‑responsibility and synchronous,
- removing the unreachable `file:///` branch,
- and centralizing the “what kind of source is this?” logic so both call sites stay in sync.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if self.url: | ||
| if ( | ||
| self.url.startswith("file:///") | ||
| or self.url.startswith("http") | ||
| or os.path.exists(self.url) | ||
| or ( |
There was a problem hiding this comment.
issue (bug_risk): The os.path.exists(self._decode_file_uri(self.url)) branch is effectively unreachable due to the preceding self.url.startswith("file:///") condition.
Because the self.url.startswith("file:///") check appears earlier in the or chain, Python short-circuits when it’s true and never evaluates the and os.path.exists(self._decode_file_uri(self.url)) part.
If you want to only accept file:/// URLs when the decoded local path exists, consider restructuring as:
if self.url.startswith("file:///"):
decoded = self._decode_file_uri(self.url)
if os.path.exists(decoded):
return self.url # or decoded
elif self.url.startswith("http") or os.path.exists(self.url):
return self.urlThis ensures the existence check is actually applied and removes dead logic.
There was a problem hiding this comment.
Code Review
This pull request improves file path resolution and decoding for audio records in astrbot/core/message/components.py by introducing _decode_file_uri and _resolve_file_source helpers. However, several improvements are recommended: first, avoid using download_image_by_url for audio files as it incorrectly saves them with a .jpg extension; second, simplify a redundant condition in the _resolve_file_source method; and third, refactor convert_to_base64 to reuse convert_to_file_path to eliminate duplicated logic.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Refactor file handling to resolve sources more effectively, including decoding file URIs and handling base64 data. Update methods to ensure compatibility with different file formats and paths.
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
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
Improve audio record file handling by unifying source resolution across file, URL, path, and base64 inputs.
New Features:
Enhancements: