-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Messenger] catch all exceptions while deserializing a message #46996
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
[Messenger] catch all exceptions while deserializing a message #46996
Conversation
Did you see #39622 ? But indeed we can merge this one too |
Hi @lyrixx I wasn't aware of this other PR - but I think they don't conflict with each other |
Shouldn't exceptions thrown by your custom normalizers implement |
yes @dunglas but if any error that the developer didn't think about occurs in this normalizer, it will result in the message indefinitely requeued, and the worker will be in error until the message has not been manually removed from the queue. |
I think that ensuring that the proper exception is thrown is the responsibility of the normalizer implementation and I fear that doing this in core would lead to confusing situations more than it would help. |
typically some broken json/xml where some fields are missing that are passed to rabbit, that will cause a |
Something "classic":
You have to do some weird things to deal with that! (I really need to finish my PR BTW) |
Can't the failing queue be used for that? https://symfonycasts.com/screencast/messenger/show-retry-failures |
No because the exception breaks everything and is out of the scope. But yes! it should be. I asked to my employer some time to work on this topic ASAP. |
You fixed it @lyrixx right? Only in 6.2 though? |
I'm not sure I covered this case. @nikophil Can you revise this PR and check if you still need it? |
Hello,
In Messenger, while deserializing a message, if an error is encountered in a custom denormalizer, the error is not caught, and then
MessageDecodingFailedException
is not thrown. Then, if RabbitMQ is used, it results in a cyclic error where the message is always requeued without any way to kick it out from the queue.My guess would be to catch any error that would happen while deserializing, even if the general policy is to never catch
\Throwable
. There is high amount of risk that if a given message have created an error once, it will create an error on every attempt to deserialize it.Or maybe we can at least catch
\Error|Symfony\Component\Serializer\Exception\ExceptionInterface
in order to cover all cases related to type error an so on...