Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 34 additions & 22 deletions src/sentry/src/Aspect/CoroutineAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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()
Expand All @@ -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();
};
Comment on lines +54 to 63
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

【严重】丢失 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.

}

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));
}
}
24 changes: 10 additions & 14 deletions src/sentry/src/Tracing/Aspect/CoroutineAspect.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)
Expand All @@ -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')
Expand All @@ -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();
Expand Down