Skip to content

Conversation

@ocetars
Copy link
Member

@ocetars ocetars commented Dec 22, 2025

Modifications / 改动点

  • This is NOT a breaking change. / 这不是一个破坏性变更。

Screenshots or Test Results / 运行截图或测试结果


Checklist / 检查清单

  • 😊 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。/ If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
  • 👀 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。/ My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
  • 🤓 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到了 requirements.txtpyproject.toml 文件相应位置。/ 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.txt and pyproject.toml.
  • 😮 我的更改没有引入恶意代码。/ My changes do not introduce malicious code.

Summary by Sourcery

在命令描述符收集和命令配置同步周围添加错误处理,以在命令解析和加载过程中提高健壮性。

Bug 修复:

  • 防止在解析命令描述符时的失败破坏整体命令收集流程,对有问题的处理器进行跳过并记录警告。
  • 避免由于命令配置同步过程中的错误导致加载失败,通过记录错误日志而不是抛出异常来处理。
Original summary in English

Summary by Sourcery

Add error handling around command descriptor collection and command config synchronization to improve robustness during command parsing and loading.

Bug Fixes:

  • Prevent failures in command descriptor parsing from breaking overall command collection by skipping problematic handlers with a warning.
  • Avoid load failures caused by errors during command configuration synchronization by logging errors instead of raising them.

@auto-assign auto-assign bot requested review from Fridemn and anka-afk December 22, 2025 11:52
@Soulter
Copy link
Member

Soulter commented Dec 22, 2025

LGTM

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题,并给出了一些总体反馈:

  • _collect_descriptors 中,包裹 _build_descriptor 的宽泛 except Exception 可能会掩盖真正的编程错误;建议收窄异常类型,或者至少记录完整的回溯信息(例如使用 exc_info=Truelogger.exception),以便更容易排查问题。
  • load 中,目前是先记录错误信息,再调用 traceback.format_exc();你可以通过使用 logger.exception("同步指令配置失败") 来简化并统一日志风格,这样会自动包含堆栈信息。
给 AI Agent 的提示词
Please address the comments from this code review:

## Overall Comments
- In `_collect_descriptors`, the broad `except Exception` around `_build_descriptor` could hide real programming errors; consider narrowing the exception type or at least logging the full traceback (e.g., with `exc_info=True` or `logger.exception`) so issues are easier to diagnose.
- In `load`, instead of logging the error message and then `traceback.format_exc()`, you can simplify and make the log more consistent by using `logger.exception("同步指令配置失败")`, which will automatically include the stack trace.

## Individual Comments

### Comment 1
<location> `astrbot/core/star/command_management.py:202-205` </location>
<code_context>
+            if not include_sub_commands and desc.is_sub_command:
+                continue
+            descriptors.append(desc)
+        except Exception as e:
+            logger.warning(
+                f"解析指令处理函数 {handler.handler_full_name} 失败,跳过该指令。原因: {e!s}"
+            )
</code_context>

<issue_to_address>
**suggestion:** Capture and log the full traceback when a handler descriptor fails to build.

Catching `Exception` to avoid one bad handler breaking collection makes sense, but logging only `str(e)` limits diagnosability. Prefer `logger.exception(...)` (or include `traceback.format_exc()`) so the full stack trace is captured while keeping behavior unchanged:

```python
        except Exception:
            logger.exception(
                "解析指令处理函数 %s 失败,跳过该指令。", handler.handler_full_name
            )
```

```suggestion
        except Exception:
            logger.exception(
                "解析指令处理函数 %s 失败,跳过该指令。", handler.handler_full_name
            )
```
</issue_to_address>

Sourcery 对开源项目免费使用 —— 如果你觉得这些代码评审有帮助,欢迎分享 ✨
帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • In _collect_descriptors, the broad except Exception around _build_descriptor could hide real programming errors; consider narrowing the exception type or at least logging the full traceback (e.g., with exc_info=True or logger.exception) so issues are easier to diagnose.
  • In load, instead of logging the error message and then traceback.format_exc(), you can simplify and make the log more consistent by using logger.exception("同步指令配置失败"), which will automatically include the stack trace.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `_collect_descriptors`, the broad `except Exception` around `_build_descriptor` could hide real programming errors; consider narrowing the exception type or at least logging the full traceback (e.g., with `exc_info=True` or `logger.exception`) so issues are easier to diagnose.
- In `load`, instead of logging the error message and then `traceback.format_exc()`, you can simplify and make the log more consistent by using `logger.exception("同步指令配置失败")`, which will automatically include the stack trace.

## Individual Comments

### Comment 1
<location> `astrbot/core/star/command_management.py:202-205` </location>
<code_context>
+            if not include_sub_commands and desc.is_sub_command:
+                continue
+            descriptors.append(desc)
+        except Exception as e:
+            logger.warning(
+                f"解析指令处理函数 {handler.handler_full_name} 失败,跳过该指令。原因: {e!s}"
+            )
</code_context>

<issue_to_address>
**suggestion:** Capture and log the full traceback when a handler descriptor fails to build.

Catching `Exception` to avoid one bad handler breaking collection makes sense, but logging only `str(e)` limits diagnosability. Prefer `logger.exception(...)` (or include `traceback.format_exc()`) so the full stack trace is captured while keeping behavior unchanged:

```python
        except Exception:
            logger.exception(
                "解析指令处理函数 %s 失败,跳过该指令。", handler.handler_full_name
            )
```

```suggestion
        except Exception:
            logger.exception(
                "解析指令处理函数 %s 失败,跳过该指令。", handler.handler_full_name
            )
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Soulter Soulter merged commit 8c94a00 into AstrBotDevs:master Dec 22, 2025
6 checks passed
@Soulter Soulter changed the title fix: 增加指令解析与同步过程中的异常处理 fix(core): improve error handling of command parser and sync Dec 22, 2025
@ocetars ocetars deleted the hotfix/command_error branch December 22, 2025 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants