-
Notifications
You must be signed in to change notification settings - Fork 179
Capture messenger exceptions #326
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
e6c1faa
348d629
d24bb25
df4c3e5
9c235a1
23f4d6b
a311892
9c2f4d2
d969165
d6f2db3
e049ea0
f6a4a5f
479941a
684409b
6e86c23
a90f2e9
3f7e66f
c19a54d
a2e5391
bf834b2
ae09a50
61d60ad
5b90e23
d999e47
88f5437
08938a0
4956311
048dae9
c0d061e
41dbc12
c2e1306
b1ddb7c
ae91616
fc29626
e8d1dd5
10036b0
eb3b6bd
fd8f386
d8124b6
ce3781e
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 |
---|---|---|
|
@@ -80,6 +80,9 @@ Keep in mind that leaving the `dsn` value empty (or undeclared) in other environ | |
```yaml | ||
sentry: | ||
dsn: "https://public:[email protected]/1" | ||
messenger: | ||
enabled: true # flushes Sentry messages at the end of each message handling | ||
capture_soft_fails: true # captures exceptions marked for retry too | ||
options: | ||
environment: '%kernel.environment%' | ||
release: '%env(VERSION)%' #your app version | ||
|
@@ -165,7 +168,7 @@ The 3.0 version of the bundle uses the newest version (2.x) of the underlying Se | |
|
||
The Sentry 2.0 SDK uses the Unified API, hence it uses the concept of `Scope`s to hold information about the current | ||
state of the app, and attach it to any event that is reported. This bundle has three listeners (`RequestListener`, | ||
`SubRequestListener` and `ConsoleListener`) that adds some easy default information. | ||
`SubRequestListener` and `ConsoleListener`) that adds some easy default information. Since 3.5, a fourth listener has been added to handle the case of Messanger Workers: `MessengerListener`. | ||
|
||
Those listeners normally are executed with a priority of `1` to allow easier customization with custom listener, that by | ||
default run with a lower priority of `0`. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
<?php | ||
|
||
namespace Sentry\SentryBundle\EventListener; | ||
|
||
use Sentry\FlushableClientInterface; | ||
use Symfony\Component\Messenger\Event\WorkerMessageFailedEvent; | ||
use Symfony\Component\Messenger\Event\WorkerMessageHandledEvent; | ||
use Symfony\Component\Messenger\Exception\HandlerFailedException; | ||
|
||
final class MessengerListener | ||
{ | ||
/** | ||
* @var FlushableClientInterface | ||
*/ | ||
private $client; | ||
|
||
/** | ||
* @var bool | ||
*/ | ||
private $captureSoftFails; | ||
|
||
/** | ||
* @param FlushableClientInterface $client | ||
* @param bool $captureSoftFails | ||
*/ | ||
public function __construct(FlushableClientInterface $client, bool $captureSoftFails = true) | ||
{ | ||
$this->client = $client; | ||
$this->captureSoftFails = $captureSoftFails; | ||
} | ||
|
||
/** | ||
* @param WorkerMessageFailedEvent $event | ||
*/ | ||
public function onWorkerMessageFailed(WorkerMessageFailedEvent $event): void | ||
{ | ||
if (! $this->captureSoftFails && $event->willRetry()) { | ||
// Don't capture soft fails. I.e. those that will be scheduled for retry. | ||
return; | ||
} | ||
|
||
$error = $event->getThrowable(); | ||
|
||
if ($error instanceof HandlerFailedException && null !== $error->getPrevious()) { | ||
// Unwrap the messenger exception to get the original error | ||
$error = $error->getPrevious(); | ||
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.
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. What's the use case there? What's the situation where the handler will report multiple exceptions? 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. https://github.com/symfony/messenger/blob/master/Middleware/HandleMessageMiddleware.php#L56-L69 Looks like when a command has multiple handlers, they will all run. Any throwable is caught and the next handler is continued. So here, we should iterate over 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. Or just capture 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. Since Just to be sure that I understood this correctly: multiple handlers will run together only if they are all sync and not async, right? If that's the case, at least it should be a non-so-common use case, so we don't have to rush the patch. Opening #338 to track this issue. 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.
No, I don't think so by reading the code and from what I understand about Messenger. Sync or Async does not matter. The only thing that matter is if there are multiple handlers for a Message or not. 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.
Correct. Each handler for a command will be run either to completion or to an exception. Subsequent handlers will be executed and previous exceptions aggregated and added to the HandlerFailedException. So even if one handler succeeds, another handler may fail and raise the exception. 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. Sorry, maybe I did not explain myself completely. For sync and async I was referring to the transport: if it's sync, the message is handled in the same request/process, and so it requires special handling on Sentry's side. If it's async, it should go through a queue (Rabbit? SQS?) so it will be executed inside a different process, and it's isolated as far as Sentry is concerned. Does this still hold? 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. Yeah if a command has multiple handlers, sync or async won’t matter. The handle middleware will execute them all in the same process, catching and aggregating errors as they’re thrown, to be later raised in the handler failed exception. |
||
} | ||
|
||
$this->client->captureException($error); | ||
$this->client->flush(); | ||
} | ||
|
||
/** | ||
* @param WorkerMessageHandledEvent $event | ||
*/ | ||
public function onWorkerMessageHandled(WorkerMessageHandledEvent $event): void | ||
{ | ||
// Flush normally happens at shutdown... which only happens in the worker if it is run with a lifecycle limit | ||
// such as --time=X or --limit=Y. Flush immediately in a background worker. | ||
$this->client->flush(); | ||
Jean85 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.