Skip to content
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

MailerInterface: failed exception contract when enabling messenger #44839

Merged
merged 1 commit into from Feb 25, 2022

Conversation

giosh94mhz
Copy link
Contributor

@giosh94mhz giosh94mhz commented Dec 29, 2021

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets no
License MIT
Doc PR likely not required...

The interface for MailerInterface::send states that exception for
transport failures are sent as TransportExceptionInterface, which
make perfectly sense and allows to create solid UI.

If at later stage the Messenger component is enabled, the mail will
be rewired into a dispatched message, which by default is synchronous.
This cause all exception to be wrapped into HandlerFailedException
which doesn't the interface contract.

In the end the UI is broken, and the exception HandlerFailedException,
even if caught, is totally unrelated to the usage context as it is only
related to configuration details.

This patch will unwrap the underlying MailTransportInterface exception
if available for the handler exception.

PS: don't know if make sense to add a phpunit test for this cross-component scenario, if so I'll try. Also, this will restore compatibility with interface, but I don't if it's to be considered a BC (strictly speaking, it may be)

The interface for MailerInterface::send states that exception for
transport failures are sent as TransportExceptionInterface, which
make perfectly sense and allows to create solid UI.

If at later stage the Messenger component is enabled, the mail will
be rewired into a dispatched message, which by default is synchronous.
This cause all exception to be wrapped into HandlerFailedException
which doesn't the interface contract.

In the end the UI is broken, and the exception HandlerFailedException,
even if caught, is totally unrelated to the usage context as it is only
related to configuration details.

This patch will unwrap the underlying MailTransportInterface exception
if available for the handler exception.
fabpot
fabpot approved these changes Feb 25, 2022
@fabpot
Copy link
Member

@fabpot fabpot commented Feb 25, 2022

Thank you @giosh94mhz.

@fabpot fabpot merged commit 989175a into symfony:4.4 Feb 25, 2022
9 of 10 checks passed
@fabpot fabpot mentioned this pull request Feb 28, 2022
@fabpot fabpot mentioned this pull request Feb 28, 2022
@fabpot fabpot mentioned this pull request Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants