Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Messenger] improve logs#32036
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Messenger] improve logs#32036
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| } | ||
| $this->logger->info('No handler for message"{class}"',$context); | ||
| $this->logger->info('No handler for message {class}',$context); |
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.
Removed quotes for consistency. They are not useful in logs because in cli they are colored due to the context and the class is a very fixed string anyway without whitespace.
| } | ||
| }catch (\Throwable$e) { | ||
| $context['exception'] =$e; | ||
| $this->logger->warning('An exception occurred while handling message "{class}":'.$e->getMessage(),$context); |
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.
Just removing the try catch. It's usually not a good idea to both log and re-throw because it created duplicate logs.
| }else { | ||
| if (null !==$this->logger) { | ||
| $this->logger->critical('Rejecting{class} (removing from transport).',$context + ['error' =>$throwable]); | ||
| $this->logger->critical('Error thrown while handling message{class}. Removing from transport after {retryCount} retries. Error: "{error}"',$context + ['retryCount' =>$retryCount,'error' =>$throwable->getMessage(),'exception' =>$throwable]); |
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.
including the exception message makes sure you can see it without having to run in-vvv mode. the context otherwise only shows the exception class but not the message.
fabpot commentedJun 14, 2019
Thank you@Tobion. |
This PR was merged into the 4.3 branch.Discussion----------[Messenger] improve logs| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | no| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets || License | MIT| Doc PR |The logs are currently very confusing and duplicated:- When handled sync the uncaught error it logged and displayed by the console / http error handler anyway. Currently it the warning is duplicated and useless:```14:26:11 WARNING [messenger] An exception occurred while handling message "{class}": OUCH, THAT HURTS! GO TO MOM!14:26:11 ERROR [console] Error thrown while running command "{class}". Message: "OUCH, THAT HURTS! GO TO MOM!"In HandleMessageMiddleware.php line 82: [Symfony\Component\Messenger\Exception\HandlerFailedException] OUCH, THAT HURTS! GO TO MOM!```- When handling async is was even confusing because the actual error was logged as warning and the retry (which is a good thing) was the error.```13:48:15 WARNING [messenger] An exception occurred while handling message "{class}": OUCH, THAT HURTS! GO TO MOM!13:48:15 ERROR [messenger] Retrying {class} - retry#1.```Now it's must clearer and adds even context like the delay:```16:20:11 ERROR [messenger] Error thrown while handling message {class}. Dispatching for retry#3 using 4000 ms delay. Error: "OUCH, THAT HURTS! GO TO MOM!"...16:20:15 CRITICAL [messenger] Error thrown while handling message {class}. Removing from transport after 3 retries. Error: "OUCH, THAT HURTS! GO TO MOM!"```Commits-------2ac7027 [Messenger] improve logs
Uh oh!
There was an error while loading.Please reload this page.
The logs are currently very confusing and duplicated:
Now it's must clearer and adds even context like the delay: