-
-
Notifications
You must be signed in to change notification settings - Fork 27
feat(sentry): improve event flushing and expand listener coverage #908
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,12 +19,14 @@ public function listen(): array | |||||||||||||||||||||
| { | ||||||||||||||||||||||
| return [ | ||||||||||||||||||||||
| Event\BeforeHandle::class, | ||||||||||||||||||||||
| Event\AfterHandle::class, | ||||||||||||||||||||||
| Event\RetryHandle::class, | ||||||||||||||||||||||
| Event\FailedHandle::class, | ||||||||||||||||||||||
| ]; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| /** | ||||||||||||||||||||||
| * @param Event\FailedHandle $event | ||||||||||||||||||||||
| * @param Event\FailedHandle|Event\AfterHandle|Event\BeforeHandle|object $event | ||||||||||||||||||||||
| */ | ||||||||||||||||||||||
| public function process(object $event): void | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
|
|
@@ -33,8 +35,16 @@ public function process(object $event): void | |||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| match ($event::class) { | ||||||||||||||||||||||
| Event\RetryHandle::class, | ||||||||||||||||||||||
| Event\FailedHandle::class => $this->captureException($event->getThrowable()), | ||||||||||||||||||||||
| default => $this->setupSentrySdk(), | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
|
Comment on lines
+38
to
41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain确认 RetryHandle 是否恒定提供 getThrowable();建议以 instanceof + 守卫式调用更稳妥 若 - match ($event::class) {
- Event\RetryHandle::class,
- Event\FailedHandle::class => $this->captureException($event->getThrowable()),
- default => $this->setupSentrySdk(),
- };
+ match (true) {
+ $event instanceof Event\BeforeHandle => $this->setupSentrySdk(),
+ $event instanceof Event\RetryHandle => method_exists($event, 'getThrowable') ? $this->captureException($event->getThrowable()) : null,
+ $event instanceof Event\FailedHandle => $this->captureException($event->getThrowable()),
+ default => null,
+ };为稳妥,请核实框架事件定义: 🌐 Web query: 💡 Result: 短答:没有。RetryHandle 事件类并不带 getThrowable() 方法(文档/示例中只有 FailedHandle 使用 event->getThrowable(),RetryHandle 只是标记“重试前”事件)。[1][2] 来源:
必须修复:RetryHandle 不提供 getThrowable(),当前实现会抛致命错误 已确认:Hyperf 的 RetryHandle 事件类不包含 getThrowable()(文档/示例);在 Retry 事件上直接调用 $event->getThrowable() 会触发运行时错误。 文件:src/sentry/src/Listener/AsyncQueueExceptionListener.php 行 38-41 - match ($event::class) {
- Event\RetryHandle::class,
- Event\FailedHandle::class => $this->captureException($event->getThrowable()),
- default => $this->setupSentrySdk(),
- };
+ match (true) {
+ $event instanceof Event\BeforeHandle => $this->setupSentrySdk(),
+ $event instanceof Event\RetryHandle => method_exists($event, 'getThrowable') ? $this->captureException($event->getThrowable()) : null,
+ $event instanceof Event\FailedHandle => $this->captureException($event->getThrowable()),
+ default => null,
+ };📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| match ($event::class) { | ||||||||||||||||||||||
| Event\AfterHandle::class, | ||||||||||||||||||||||
| Event\RetryHandle::class, | ||||||||||||||||||||||
| Event\FailedHandle::class => $this->flushEvents(), | ||||||||||||||||||||||
| default => null, | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
需要验证 Logs 类的导入和使用是否正确
添加了
Logs::getInstance()->flush()调用,但需要确认:Sentry\Logs\Logs类是否在当前使用的 Sentry SDK 版本中存在getInstance()和flush()方法🏁 Script executed:
Length of output: 155
🌐 Web query:
💡 Result:
Here are the official docs for the Sentry PHP SDK logs API / Log class:
References
[1] https://docs.sentry.io/platforms/php/logs/
[2] https://develop.sentry.dev/sdk/telemetry/logs/
[3] https://github.com/getsentry/sentry-php
必须修复:确认并修正 Sentry Logs 的导入与用法(会导致运行时错误)
use Sentry\Logs\Logs;(第17行),并在第113行调用Logs::getInstance()->flush();。\Sentry\Logs\Log(单数),未见Logs::getInstance()或flush()这类单例 API,当前导入/调用很可能不存在且会在运行时报错。