Skip to content

fix(core): 解除 call_event_hook 中 functools.partial 包装#9178

Open
Aryunr wants to merge 2 commits into
AstrBotDevs:masterfrom
Aryunr:fix/call_event_hook-partial-unwrap
Open

fix(core): 解除 call_event_hook 中 functools.partial 包装#9178
Aryunr wants to merge 2 commits into
AstrBotDevs:masterfrom
Aryunr:fix/call_event_hook-partial-unwrap

Conversation

@Aryunr

@Aryunr Aryunr commented Jul 7, 2026

Copy link
Copy Markdown

改动

修复了 call_event_hook 中调用插件事件处理器时,handler.handler 可能被 star_manager 绑定为 functools.partial 对象,导致参数传递异常的问题。

修改文件:

  • astrbot/core/utils/context_utils.py — 在 call_event_hook 函数中,调用前通过 while isinstance(_handler_fn, functools.partial) 解开所有 partial 包装,再执行原始函数。

测试结果

已在 AstrBot v4.26.5 + OneBotV11 环境下测试通过:

  1. 安装 astrbot_plugin_ai_draw 插件
  2. 修复前:on_llm_requestself 为 None,调用 self.provider 报错
  3. 修复后:插件正常运行,/画图/识图 功能正常,self 正确绑定到插件实例
  4. 控制台无红色错误日志

运行日志

[2026-07-07 14:40:11.082] [Core][INFO][core.event_bus:74]: [11Qtest] [QQ_Costom(aiocqhttp)] Aryun: [引用消息(Sakura: 🎨 生成完成:)] [At:2705889261] 这是什么 ?
[2026-07-07 14:40:15.489] [Core][INFO][astrbot_plugin_ai_draw.main:438]: AiDraw on_agent_begin: 已拦截 1 张图片
[2026-07-07 14:40:32.150] [Core][INFO][pipeline.context_utils:118]: astrbot_plugin_ai_draw - on_agent_begin 终止了事件传播。
[2026-07-07 14:40:34.550] [Core][INFO][runners.tool_loop_agent_runner:1385]: Agent execution was requested to stop by user.
[2026-07-07 14:40:34.552] [Core][INFO][pipeline.context_utils:118]: astrbot_plugin_recall_cancel - on_llm_response 终止了事件传播。
[2026-07-07 14:40:34.579] [Core][INFO][respond.stage:206]: Prepare to send - Aryun: 📷 图片识别结果:
这张图片展示了一颗CPU处理器芯片的特写镜头,整体采用专业的产品摄影风格拍摄。以下是详细内容:
主体对象
芯片本体:一颗方形的处理器,以对角线角度放置在深色背景上。
基板颜色:PCB基板呈深绿色,边缘略微泛黄,显示出典型的电路板质感。

25b019bf-fe64-4cf9-9e2d-cdf76459b741

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.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Bug Fixes:

Summary by Sourcery

Bug Fixes:

  • Ensure plugin event handlers wrapped in functools.partial are unwrapped before coroutine checks and invocation to prevent incorrect parameter binding.

在 call_event_hook 中调用插件事件处理器前,解开 handler.handler 可能存在的 functools.partial 包装,避免因 star_manager 绑定导致的参数传递异常。
@dosubot dosubot Bot added size:XS This PR changes 0-9 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend labels Jul 7, 2026

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

  • Unwrapping the functools.partial and then calling _handler_fn(event, *args, **kwargs) drops any arguments bound in the partial (e.g., self), so consider using the unwrapped function only for the inspect.iscoroutinefunction check and still invoking handler.handler to preserve its bound arguments.
  • If the goal is just to handle inspect.iscoroutinefunction returning False for functools.partial, you can keep the original behavior by doing base_fn = handler.handler.func if isinstance(handler.handler, functools.partial) else handler.handler for the assertion, but still await handler.handler(event, *args, **kwargs) for execution.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Unwrapping the `functools.partial` and then calling `_handler_fn(event, *args, **kwargs)` drops any arguments bound in the partial (e.g., `self`), so consider using the unwrapped function only for the `inspect.iscoroutinefunction` check and still invoking `handler.handler` to preserve its bound arguments.
- If the goal is just to handle `inspect.iscoroutinefunction` returning `False` for `functools.partial`, you can keep the original behavior by doing `base_fn = handler.handler.func if isinstance(handler.handler, functools.partial) else handler.handler` for the assertion, but still `await handler.handler(event, *args, **kwargs)` for execution.

## Individual Comments

### Comment 1
<location path="astrbot/core/pipeline/context_utils.py" line_range="95-102" />
<code_context>
     for handler in handlers:
         try:
-            assert inspect.iscoroutinefunction(handler.handler)
+            _handler_fn = handler.handler
+            while isinstance(_handler_fn, functools.partial):
+                _handler_fn = _handler_fn.func
+            assert inspect.iscoroutinefunction(_handler_fn)
             logger.debug(
                 f"hook({hook_type.name}) -> {star_map[handler.handler_module_path].name} - {handler.handler_name}",
             )
-            await handler.handler(event, *args, **kwargs)
+            await _handler_fn(event, *args, **kwargs)
         except BaseException:
             logger.error(traceback.format_exc())
</code_context>
<issue_to_address>
**issue (bug_risk):** Unwrapping `functools.partial` for the call drops pre-configured args/kwargs from the handler.

Using `_handler_fn` for invocation drops any args/kwargs bound by `functools.partial`. For instance, if `handler.handler` is `functools.partial(async_fn, fixed_arg=1)`, calling `_handler_fn(event, *args, **kwargs)` skips `fixed_arg`. Consider using `_handler_fn` only to walk through `.func` for the `iscoroutinefunction` check, but still invoking `await handler.handler(event, *args, **kwargs)` to preserve the original partial binding behavior.
</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.

Comment thread astrbot/core/pipeline/context_utils.py Outdated
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the event hook calling logic in astrbot/core/pipeline/context_utils.py to support event handlers wrapped with functools.partial by unwrapping them before checking if they are coroutine functions. The reviewer suggested a valuable improvement to also handle decorators by using inspect.unwrap in a loop, which prevents assertion failures when handlers are wrapped with custom decorators using functools.wraps.

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.

Comment thread astrbot/core/pipeline/context_utils.py
@Aryunr

Aryunr commented Jul 8, 2026

Copy link
Copy Markdown
Author

@sourcery-ai

@sourcery-ai

sourcery-ai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Sure! I'm generating a new review now.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey - I've left some high level feedback:

  • Since you now resolve the underlying function into _handler_fn, consider using _handler_fn rather than handler.handler in the debug log so the log reflects the actual coroutine being invoked (and avoids confusion if additional wrapper layers are introduced later).
  • If a handler ends up not being a coroutine after unwrapping the functools.partial layers, using assert will raise an AssertionError that may be harder to trace in production; consider raising a more explicit exception or logging a clear error instead.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Since you now resolve the underlying function into `_handler_fn`, consider using `_handler_fn` rather than `handler.handler` in the debug log so the log reflects the actual coroutine being invoked (and avoids confusion if additional wrapper layers are introduced later).
- If a handler ends up not being a coroutine after unwrapping the `functools.partial` layers, using `assert` will raise an `AssertionError` that may be harder to trace in production; consider raising a more explicit exception or logging a clear error instead.

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.

@sourcery-ai

sourcery-ai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Hey @Aryunr, I've posted a new review for you!

@Aryunr

Aryunr commented Jul 8, 2026

Copy link
Copy Markdown
Author

sourcery-ai评测澄清

debug log 用_handler_fn代替handler.handler不采纳
handler.handler_module_pathhandler.handler_name 是 handler 注册时存的元数据字符串,跟 functools.partial 包装无关。它们指向的是插件 handler 的模块路径和函数名,这才是日志该记录的信息。改用_handler_fn.__module__ / _handler_fn.__qualname__反而可能指向 partial 内部嵌套的函数引用,不符合 log 目的。

Gemini-code-assist评测澄清

inspect.unwrap 解装饰器包装 —— __wrapped__ 属性可能包裹异步函数不采纳

对 AstrBot 当前运行实例中所有已注册 handler 逐一验证:星标 handler 由 star_manager 内部通过 get_handler_or_create 直接存储原始函数引用,中间不存在 @functools.wraps 装饰器包装层。register_on_llm_request / register_on_agent_begin 等注册函数将用户函数原样存入 handler.handler,唯一的二次包装是 star_manager 在绑定插件实例时创建的 functools.partial——这正是当前代码 while isinstance(_handler_fn, functools.partial) 已经处理的场景。

inspect.unwrap 解决的是同步装饰器包裹异步函数导致 iscoroutinefunction 误判的问题,这要求插件开发者主动在注册装饰器之上再叠一层自定义装饰器。AstrBot 插件生态中不存在此模式:注册装饰器本身就是入口,没有开发者会在外部再包一层。即使未来有人这样做,该装饰器在语义上就是 handler 的一部分,静默 unwrap 跳过它在行为上也是错误的——装饰器存在本身就代表开发者意图。

该建议防御的场景在当前 AstrBot 体系下不存在。此条不采纳.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

感谢你的详细解释。我理解了在 AstrBot 当前的插件注册和运行机制下,functools.partial 是主要的包装形式,且不存在开发者主动叠层装饰器导致 inspect.unwrap 产生副作用的场景。既然该防御性建议在当前架构下并不适用,我完全接受你的澄清。感谢你对系统内部实现细节的深入分析。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend size:XS This PR changes 0-9 lines, ignoring generated files.

Projects

None yet

1 participant