fix(message_tools): 路径不存在时抛出异常并阻止消息发送#8149
Conversation
- _resolve_path_from_sandbox 在所有解析路径均失败时改为抛出 FileNotFoundError,而非静默返回原始路径,避免将无效路径传递给下游组件 - 新增 component_type 关键字参数,使错误信息能明确指出是 image/record/video/file 哪类资源路径缺失 - 在 call 方法中捕获 FileNotFoundError 并提前返回错误字符串,确保路径无效时不会继续构建或发送任何消息组件 - 补充单元测试,验证缺失图片路径场景下 send_message 不会被调用
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
_resolve_path_from_sandboxlogic now treats any sandbox access failure (e.g.,get_bootererrors or download failures) asFileNotFoundError, which may produce misleading"path does not exist"messages; consider distinguishing between "not found" and "sandbox unavailable/IO error" in the raised error. - The
component_typestring is used for error messages but is free-form; consider constraining it via an enum orLiteralconstants to avoid typos and keep error wording consistent across call sites.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `_resolve_path_from_sandbox` logic now treats any sandbox access failure (e.g., `get_booter` errors or download failures) as `FileNotFoundError`, which may produce misleading `"path does not exist"` messages; consider distinguishing between "not found" and "sandbox unavailable/IO error" in the raised error.
- The `component_type` string is used for error messages but is free-form; consider constraining it via an enum or `Literal` constants to avoid typos and keep error wording consistent across call sites.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request enhances the SendMessageToUserTool by improving file path resolution and error reporting, specifically by raising FileNotFoundError with component-specific context and adding a corresponding unit test. Feedback suggests re-raising original exceptions in _resolve_path_from_sandbox to prevent masking underlying system errors (like sandbox connectivity issues) and updating the test assertions to reflect these more accurate error messages.
| except Exception as exc: | ||
| logger.warning(f"Failed to check/download file from sandbox: {exc}") | ||
|
|
||
| return path, False | ||
| raise FileNotFoundError(f"{component_type} path does not exist: {path}") |
There was a problem hiding this comment.
捕获所有异常并统一抛出 FileNotFoundError 会掩盖真实的失败原因(例如沙箱连接问题或权限错误)。建议让系统级异常向上抛出,以便由 call 方法中的通用 Exception 块处理,从而为用户或 LLM 提供更准确的错误信息。FileNotFoundError 应该仅在明确确认文件不存在时抛出。
| except Exception as exc: | |
| logger.warning(f"Failed to check/download file from sandbox: {exc}") | |
| return path, False | |
| raise FileNotFoundError(f"{component_type} path does not exist: {path}") | |
| except Exception as exc: | |
| logger.warning(f"Failed to check/download file from sandbox: {exc}") | |
| raise | |
| raise FileNotFoundError(f"{component_type} path does not exist: {path}") |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Ref #8107
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
Ensure message sending aborts when media file paths cannot be resolved from the sandbox.
Bug Fixes:
Tests: