feat(sentry): improve coroutine context handling and exception capture #967
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit refactors the CoroutineAspect classes to improve context propagation and exception handling in coroutines: **Core Changes:** - Replace manual context copying with `Context::copy()` for cleaner context propagation - Add `printLog` method interception to capture exceptions thrown in coroutines - Extract context keys to a shared constant `CONTEXT_KEYS` - Simplify coroutine callable wrapper logic by removing try-catch blocks **Benefits:** - More reliable context restoration in new coroutines - Automatic exception capture from coroutine errors via `printLog` hook - Improved code maintainability with centralized context key management - Better integration with Hyperf's Context system **Technical Details:** - Use `Context::copy($cid, self::CONTEXT_KEYS)` instead of manual foreach loop - Defer exception capture in `printLog` to ensure proper timing - Remove redundant exception handling as exceptions are now captured via aspect
总体描述两个 CoroutineAspect 文件进行重构，引入 变更
序列图sequenceDiagram
participant Client as 调用方
participant Aspect as CoroutineAspect
participant Process as process()
participant HandleCreate as handleCreate()
participant Context as Context
participant Coroutine as Coroutine
participant Defer as Co::defer()
participant Integration as Integration
Client->>Aspect: process(proceedingJoinPoint)
Aspect->>Process: 路由分发(methodName)
alt 方法为 create
Process->>HandleCreate: 执行处理
HandleCreate->>Context: Context::copy(CONTEXT_KEYS)
Context-->>HandleCreate: 复制指定上下文
HandleCreate->>Coroutine: 创建新协程
HandleCreate->>Defer: Co::defer(flushEvents)
Defer-->>Coroutine: 注册延迟操作
Coroutine-->>Integration: 协程完成时刷新事件
else 方法为 printLog
Process->>HandleCreate: handlePrintLog处理
end
代码审查工作量估算🎯 3 (中等) | ⏱️ ~25 分钟 变更涉及两个相关文件的协调修改，引入了新的上下文复制机制和延迟操作模式。需要理解 Context 的复制语义、Co::defer 的调用时机，以及新旧上下文传递逻辑的对应关系。变更具有一定的逻辑密度，但改动相对集中且具有一致性。 可能相关的 PR
建议审查者
诗意颂歌
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 PHPStan (2.1.31)At least one path must be specified to analyse. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
huangdijia
added a commit
that referenced
this pull request
Oct 21, 2025
#967) This commit refactors the CoroutineAspect classes to improve context propagation and exception handling in coroutines: **Core Changes:** - Replace manual context copying with `Context::copy()` for cleaner context propagation - Add `printLog` method interception to capture exceptions thrown in coroutines - Extract context keys to a shared constant `CONTEXT_KEYS` - Simplify coroutine callable wrapper logic by removing try-catch blocks **Benefits:** - More reliable context restoration in new coroutines - Automatic exception capture from coroutine errors via `printLog` hook - Improved code maintainability with centralized context key management - Better integration with Hyperf's Context system **Technical Details:** - Use `Context::copy($cid, self::CONTEXT_KEYS)` instead of manual foreach loop - Defer exception capture in `printLog` to ensure proper timing - Remove redundant exception handling as exceptions are now captured via aspect Co-authored-by: Deeka Wong <8337659+huangdijia@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
56-66: 【严重】未转发 Coroutine::create 的可变参数，目标回调将收不到参数。当前仅调用 $callable()，会破坏原有行为；同时仅改了 arguments['keys']['callable']，未同步 arguments['values'][0] 也可能导致 AOP 调用仍使用旧回调。请按下述方式健壮转发参数并同步 values：
- $cid = Co::id(); - $callable = $proceedingJoinPoint->arguments['keys']['callable']; + $cid = Co::id(); + $arguments = $proceedingJoinPoint->arguments; + $callable = $arguments['keys']['callable'] ?? ($arguments['values'][0] ?? null); + $args = $arguments['keys']['args'] + ?? $arguments['keys']['params'] + ?? array_slice($arguments['values'] ?? [], 1); - $proceedingJoinPoint->arguments['keys']['callable'] = function () use ($callable, $span, $callingOnFunction, $cid) { + $proceedingJoinPoint->arguments['keys']['callable'] = function () use ($callable, $args, $span, $callingOnFunction, $cid) { SentrySdk::init(); // Ensure Sentry is initialized in the new coroutine. @@ - return trace( - fn () => $callable(), + return trace( + fn () => $callable(...$args), SpanContext::make() ->setOp('coroutine.execute') ->setDescription($callingOnFunction) ->setOrigin('auto.coroutine') ); }; + // 保持 keys/values 同步，避免 AOP 使用旧 callable + $proceedingJoinPoint->arguments['values'][0] = $proceedingJoinPoint->arguments['keys']['callable'];Also applies to: 84-91
🧹 Nitpick comments (4)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (2)
32-34: 【建议】将 CONTEXT_KEYS 抽到共享位置（例如 trait 或常量类）供两个 Aspect 复用，避免双处维护出现漂移。命名建议：FriendsOfHyperf\Sentry\Context\ContextKeys::HTTP.
78-83: 【重复 flush 风险】此处 defer 内调用 Integration::flushEvents()，而非追踪版的 Aspect（src/sentry/src/Aspect/CoroutineAspect.php 的 handleCreate）也会在协程结束时 Co::defer flush 一次。两处同时 flush 没必要，浪费资源。建议只保留一处（更建议保留在非追踪 Aspect 中）。本文件可仅负责 finish 事务，移除 flush：
- defer(function () use ($transaction) { - $transaction->finish(); - Integration::flushEvents(); - }); + defer(function () use ($transaction) { + $transaction->finish(); + });src/sentry/src/Aspect/CoroutineAspect.php (2)
24-26: 【建议】CONTEXT_KEYS 与 Tracing 版本重复定义，建议抽到共享常量/trait 统一维护，避免将来增减键遗漏。
58-60: 【重复 flush 风险】此处已 Co::defer flush，但 Tracing/Aspect 里也会 defer flush。建议仅保留一处（更推荐保留在本文件非追踪 Aspect），并在 Tracing 版本移除 flush（参见对应评论中的 diff）。
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/sentry/src/Aspect/CoroutineAspect.php(3 hunks)src/sentry/src/Tracing/Aspect/CoroutineAspect.php(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/*/src/**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All component PHP code must use the namespace pattern FriendsOfHyperf{ComponentName}
Files:
src/sentry/src/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Aspect/CoroutineAspect.php
**/*.php
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.php: Code style must follow PSR-12 and be formatted by PHP-CS-Fixer per .php-cs-fixer.php
Run PHPStan and keep the codebase passing per phpstan.neon.dist
Files:
src/sentry/src/Tracing/Aspect/CoroutineAspect.phpsrc/sentry/src/Aspect/CoroutineAspect.php
🧬 Code graph analysis (2)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)init(42-45)
src/sentry/src/Aspect/CoroutineAspect.php (3)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
process(45-101)src/sentry/src/Integration.php (2)
Integration(28-158)flushEvents(106-115)src/sentry/class_map/SentrySdk.php (2)
SentrySdk(24-65)getCurrentHub(51-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.1 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.2 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 6.0.2
- GitHub Check: Test on PHP 8.1 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.2 with Swoole 5.1.7
- GitHub Check: Test on PHP 8.3 with Swoole 5.1.7
🔇 Additional comments (2)
src/sentry/src/Tracing/Aspect/CoroutineAspect.php (1)
69-77: 【请确认事务/Span 结构】在 trace('coroutine.create') 内又 startTransaction('coroutine.prepare')，随后再 trace('coroutine.execute')。这会产生“一个事务 + 两个 span”的层次。请确认这是否符合采样与可视化预期，避免出现孤立事务或重复根节点。可在本地开启 Sentry debug 检查事件结构与 traceparent 继承是否正确。src/sentry/src/Aspect/CoroutineAspect.php (1)
66-75: 【LGTM】printLog 拦截与延迟上报实现简洁有效；类型检查合理，不会误报。
Comment on lines
+54
to
63
| $proceedingJoinPoint->arguments['keys']['callable'] = function () use ($callable, $cid) { | ||
| // Restore the Context in the new Coroutine. | ||
| Context::copy($cid, self::CONTEXT_KEYS); | ||
|
|
||
| foreach ($keys as $key) { | ||
| if (isset($from[$key]) && ! isset($current[$key])) { | ||
| $current[$key] = $from[$key]; | ||
| } | ||
| } | ||
| // Defer the flushing of events until the coroutine completes. | ||
| Co::defer(fn () => Integration::flushEvents()); | ||
|
|
||
| try { | ||
| $callable(); | ||
| } catch (Throwable $throwable) { | ||
| SentrySdk::getCurrentHub()->captureException($throwable); | ||
| throw $throwable; | ||
| } finally { | ||
| Integration::flushEvents(); | ||
| } | ||
| // Continue the callable in the new Coroutine. | ||
| $callable(); | ||
| }; |
There was a problem hiding this comment.
【严重】丢失 Coroutine::create 的可变参数，且未同步 arguments['values'][0]。这会导致回调收不到原始参数，功能性回归。
按下述方式修复，并可选在协程内初始化 Hub 以保证非追踪路径也具备 Sentry 上下文：
- $callable = $proceedingJoinPoint->arguments['keys']['callable'];
- $cid = Co::id();
-
- $proceedingJoinPoint->arguments['keys']['callable'] = function () use ($callable, $cid) {
- // Restore the Context in the new Coroutine.
- Context::copy($cid, self::CONTEXT_KEYS);
-
- // Defer the flushing of events until the coroutine completes.
- Co::defer(fn () => Integration::flushEvents());
-
- // Continue the callable in the new Coroutine.
- $callable();
- };
+ $arguments = $proceedingJoinPoint->arguments;
+ $callable = $arguments['keys']['callable'] ?? ($arguments['values'][0] ?? null);
+ $args = $arguments['keys']['args']
+ ?? $arguments['keys']['params']
+ ?? array_slice($arguments['values'] ?? [], 1);
+ $cid = Co::id();
+
+ $proceedingJoinPoint->arguments['keys']['callable'] = function () use ($callable, $args, $cid) {
+ // 恢复父协程上下文
+ Context::copy($cid, self::CONTEXT_KEYS);
+ // 可选：确保新协程具备已配置的 Hub（在未启用 tracing 的情况下仍然生效）
+ SentrySdk::init();
+ // 协程结束后再 flush
+ Co::defer(fn () => Integration::flushEvents());
+ // 继续执行并正确转发参数
+ $callable(...$args);
+ };
+ // 保持 keys/values 同步
+ $proceedingJoinPoint->arguments['values'][0] = $proceedingJoinPoint->arguments['keys']['callable'];Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/sentry/src/Aspect/CoroutineAspect.php around lines 54 to 63, the closure
assigned to $proceedingJoinPoint->arguments['keys']['callable'] drops the
original variadic arguments passed to Coroutine::create and fails to synchronize
$proceedingJoinPoint->arguments['values'][0], causing the callback to not
receive its original params; fix by capturing the original argument list
(preserve variadic args) when building the closure and forward them to $callable
(and update arguments['values'][0] to the preserved args) so the coroutine
receives the same parameters as the original call; optionally inside the
coroutine reinitialize or bind a Hub/context (e.g., create or copy Hub) to
ensure Sentry context exists for non-traced paths.
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR refactors the Sentry
CoroutineAspectclasses to improve context propagation and exception handling in coroutines:Context::copy()utilityprintLogmethod interception to capture exceptions thrown in coroutinesCONTEXT_KEYSChanges
CoroutineAspect (src/sentry/src/Aspect/CoroutineAspect.php)
CONTEXT_KEYSconstant for centralized context key managementprintLogmethod to intercepted classes for exception captureprocess()to use pattern matching for method dispatchhandleCreate()andhandlePrintLog()methodsContext::copy()printLoghook using deferred executionTracing\CoroutineAspect (src/sentry/src/Tracing/Aspect/CoroutineAspect.php)
CONTEXT_KEYSconstant for consistencyContext::copy()Benefits
Context::copy()for proper context propagationprintLogaspect, ensuring all coroutine errors are reportedTest plan
Summary by CodeRabbit
发布说明