feat: add hot reload when failed to load plugins#5334
feat: add hot reload when failed to load plugins#5334Soulter merged 2 commits intoAstrBotDevs:masterfrom
Conversation
Summary of ChangesHello @Waterwzy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where plugins that failed to load due to import errors could not be properly hot reloaded. It improves the robustness of the plugin management system by enhancing error handling on the backend to accurately track and report failed plugins, and by ensuring the frontend consistently updates its display after any reload operation. This provides a more reliable and informative experience for users managing extensions. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈:
- 在
reloadPlugin中,可以考虑把getExtensions()移动到finally代码块中(或者在成功路径和捕获异常路径中都调用),这样即使重载请求本身失败(例如网络错误或抛出异常),扩展列表也仍然会被刷新。 - 在
star_manager.load中,记录失败时你更新了failed_plugin_dict,但没有更新failed_plugin_info;如果后续代码依赖这两个结构保持同步,那么把它们一起更新,或者将失败插件的元数据整合到单一的“可信源”中,可能会更安全。
供 AI 代理使用的提示词
Please address the comments from this code review:
## Overall Comments
- In `reloadPlugin`, consider moving `getExtensions()` into a `finally` block (or calling it both in success and catch paths) so the extension list is refreshed even when the reload request itself fails (e.g., network error or thrown exception).
- In `star_manager.load`, when recording failures you update `failed_plugin_dict` but not `failed_plugin_info`; if the rest of the code relies on both structures being in sync, it may be safer to update them together or consolidate to a single source of truth for failed plugin metadata.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've left some high level feedback:
- In
reloadPlugin, consider movinggetExtensions()into afinallyblock (or calling it both in success and catch paths) so the extension list is refreshed even when the reload request itself fails (e.g., network error or thrown exception). - In
star_manager.load, when recording failures you updatefailed_plugin_dictbut notfailed_plugin_info; if the rest of the code relies on both structures being in sync, it may be safer to update them together or consolidate to a single source of truth for failed plugin metadata.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `reloadPlugin`, consider moving `getExtensions()` into a `finally` block (or calling it both in success and catch paths) so the extension list is refreshed even when the reload request itself fails (e.g., network error or thrown exception).
- In `star_manager.load`, when recording failures you update `failed_plugin_dict` but not `failed_plugin_info`; if the rest of the code relies on both structures being in sync, it may be safer to update them together or consolidate to a single source of truth for failed plugin metadata.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 aims to improve the hot reload mechanism for failed plugins, ensuring import failures are logged and failed plugins are cleaned from the active plugin map, with the frontend extension list refreshing after reload attempts. However, a potential information disclosure vulnerability is introduced by storing and exposing full Python stack traces to the frontend dashboard. Furthermore, the backend cleanup logic should also remove failed plugins from star_registry for consistency, and this robust cleanup approach should be extended to other error handling blocks.
astrbot/core/star/star_manager.py
Outdated
| if path in star_map: | ||
| logger.info("失败插件依旧在插件列表中") | ||
| star_map.pop(path) |
There was a problem hiding this comment.
当插件加载失败时,代码正确地将其从 star_map 中移除。但是,它仍然保留在 star_registry 中。这可能会导致数据不一致,因为 get_all_stars()(返回 star_registry)可能仍会将失败的插件报告为已加载。为确保彻底清理,插件的元数据也应从 star_registry 中移除。
此外,建议在第 784 行附近的 except BaseException 块中也应用类似的清理逻辑,以一致地处理所有插件加载失败的情况。
if path in star_map:
logger.info("失败插件依旧在插件列表中,正在清理...")
metadata = star_map.pop(path)
if metadata in star_registry:
star_registry.remove(metadata)| self.failed_plugin_dict[root_dir_name] = { | ||
| "error": str(e), | ||
| "traceback": error_trace, | ||
| } |
There was a problem hiding this comment.
The application captures and stores full Python stack traces in self.failed_plugin_dict when a plugin fails to load. This data is intended to be sent to the frontend dashboard. Stack traces can reveal sensitive information about the server's internal file structure, installed libraries, and code logic, which can be leveraged by an attacker to plan further attacks.
Recommendation: Avoid sending full stack traces to the frontend. Log the full traceback on the server for debugging purposes and return only a generic error message or a sanitized version of the error to the client.
self.failed_plugin_dict[root_dir_name] = {
"error": str(e),
}|
Generated docs update PR (pending manual review): AI change summary:
Experimental bot notice:
|
Modifications / 改动点
Close: #5333
当重载插件发生import失败时,这里的处理不够完善,导致因上述原因加载失败的插件无法热重载。
具体修复如下:
astrbot/core/star/star_manager.pyself.failed_plugin_dict和self.failed_plugin_info可以正确记录失败消息,保证前端拿到正确的数据。dashboard/src/views/ExtensionPage.vue完善
reloadPlugin重载插件时的渲染逻辑,无论重载是否成功,都调用getExtensions以正确刷新插件页面和响应卡片。This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
改进插件在导入失败时的重新加载处理方式,以便正确追踪失败的插件,并让 UI 反映最新状态。
Bug 修复:
Original summary in English
Summary by Sourcery
Improve plugin reload handling when plugin imports fail so that failed plugins are correctly tracked and the UI reflects the latest state.
Bug Fixes: