From 45fcd3e8e1889268452a7584ee8ee6dbfd4ab47e Mon Sep 17 00:00:00 2001 From: Deeka Wong <8337659+huangdijia@users.noreply.github.com> Date: Tue, 21 Oct 2025 15:28:33 +0800 Subject: [PATCH] feat(sentry): improve coroutine context handling and exception capture 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 --- src/sentry/src/Aspect/CoroutineAspect.php | 56 +++++++++++-------- .../src/Tracing/Aspect/CoroutineAspect.php | 24 ++++---- 2 files changed, 44 insertions(+), 36 deletions(-) diff --git a/src/sentry/src/Aspect/CoroutineAspect.php b/src/sentry/src/Aspect/CoroutineAspect.php index d0cdc007f..351e283c5 100644 --- a/src/sentry/src/Aspect/CoroutineAspect.php +++ b/src/sentry/src/Aspect/CoroutineAspect.php @@ -12,6 +12,7 @@ namespace FriendsOfHyperf\Sentry\Aspect; use FriendsOfHyperf\Sentry\Integration; +use Hyperf\Context\Context; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Hyperf\Engine\Coroutine as Co; @@ -20,12 +21,13 @@ class CoroutineAspect extends AbstractAspect { - public array $classes = [ - 'Hyperf\Coroutine\Coroutine::create', + public const CONTEXT_KEYS = [ + \Psr\Http\Message\ServerRequestInterface::class, ]; - protected array $keys = [ - \Psr\Http\Message\ServerRequestInterface::class, + public array $classes = [ + 'Hyperf\Coroutine\Coroutine::create', + 'Hyperf\Coroutine\Coroutine::printLog', ]; public function __construct() @@ -34,31 +36,41 @@ public function __construct() } public function process(ProceedingJoinPoint $proceedingJoinPoint) + { + match ($proceedingJoinPoint->methodName) { + 'create' => $this->handleCreate($proceedingJoinPoint), + 'printLog' => $this->handlePrintLog($proceedingJoinPoint), + default => null, + }; + + return $proceedingJoinPoint->process(); + } + + protected function handleCreate(ProceedingJoinPoint $proceedingJoinPoint): void { $callable = $proceedingJoinPoint->arguments['keys']['callable']; - $keys = $this->keys; $cid = Co::id(); - $proceedingJoinPoint->arguments['keys']['callable'] = function () use ($callable, $cid, $keys) { - $from = Co::getContextFor($cid); - $current = Co::getContextFor(); + $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(); }; + } - return $proceedingJoinPoint->process(); + protected function handlePrintLog(ProceedingJoinPoint $proceedingJoinPoint): void + { + $throwable = $proceedingJoinPoint->arguments['keys']['throwable'] ?? null; + + if (! $throwable instanceof Throwable) { + return; + } + + Co::defer(fn () => SentrySdk::getCurrentHub()->captureException($throwable)); } } diff --git a/src/sentry/src/Tracing/Aspect/CoroutineAspect.php b/src/sentry/src/Tracing/Aspect/CoroutineAspect.php index 6f91429a5..512a71d4b 100644 --- a/src/sentry/src/Tracing/Aspect/CoroutineAspect.php +++ b/src/sentry/src/Tracing/Aspect/CoroutineAspect.php @@ -14,6 +14,7 @@ use FriendsOfHyperf\Sentry\Feature; use FriendsOfHyperf\Sentry\Integration; use FriendsOfHyperf\Sentry\Util\CoroutineBacktraceHelper; +use Hyperf\Context\Context; use Hyperf\Di\Aop\AbstractAspect; use Hyperf\Di\Aop\ProceedingJoinPoint; use Hyperf\Engine\Coroutine as Co; @@ -28,12 +29,12 @@ class CoroutineAspect extends AbstractAspect { - public array $classes = [ - 'Hyperf\Coroutine\Coroutine::create', + public const CONTEXT_KEYS = [ + \Psr\Http\Message\ServerRequestInterface::class, ]; - protected array $keys = [ - \Psr\Http\Message\ServerRequestInterface::class, + public array $classes = [ + 'Hyperf\Coroutine\Coroutine::create', ]; public function __construct(protected Feature $feature) @@ -56,22 +57,16 @@ public function process(ProceedingJoinPoint $proceedingJoinPoint) function (Scope $scope) use ($proceedingJoinPoint, $callingOnFunction) { if ($callingOnFunction && $span = $scope->getSpan()) { $cid = Co::id(); - $keys = $this->keys; $callable = $proceedingJoinPoint->arguments['keys']['callable']; // Transfer the Sentry context to the new coroutine. - $proceedingJoinPoint->arguments['keys']['callable'] = function () use ($callable, $span, $callingOnFunction, $cid, $keys) { + $proceedingJoinPoint->arguments['keys']['callable'] = function () use ($callable, $span, $callingOnFunction, $cid) { SentrySdk::init(); // Ensure Sentry is initialized in the new coroutine. - $from = Co::getContextFor($cid); - $current = Co::getContextFor(); - - foreach ($keys as $key) { - if (isset($from[$key]) && ! isset($current[$key])) { - $current[$key] = $from[$key]; - } - } + // Restore the Context in the new Coroutine. + Context::copy($cid, self::CONTEXT_KEYS); + // Start a new transaction for the coroutine preparation phase. $transaction = startTransaction( continueTrace($span->toTraceparent(), $span->toBaggage()) ->setName('coroutine') @@ -80,6 +75,7 @@ function (Scope $scope) use ($proceedingJoinPoint, $callingOnFunction) { ->setOrigin('auto.coroutine') ); + // Defer the finishing of the transaction and flushing of events until the coroutine completes. defer(function () use ($transaction) { $transaction->finish(); Integration::flushEvents();