Skip to content

[Mailer][Transport] Allow exception logging for RoundRobinTransport mailer #60110

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

Open
wants to merge 3 commits into
base: 7.4
Choose a base branch
from

Conversation

jnoordsij
Copy link
Contributor

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues N/A
License MIT

The RoundRobinTransport class (and by extension the FailoverTransport class) allow one to mail using different mail transports, with automatic failover in case a mailer fails. In case of such a failure, exceptions are caught (and grouped in case of multiple failures) and the next transport is attempted. This ensures that as long as one transport succeeds, the mail will be delivered.

However, the current way of exception handling also means that exceptions on failing transports will never bubble up, unless all of them fail at once. This means that in case of a prolonged failure of a transport, e.g. due to misconfiguration, the only way to detect such a failure is by noting that a intended mailer is never sending mail, rather then be warned by actual exceptions of your application. If this is not detected, then one will only discover once all senders are failing, that one of them might have been failing all of the time already.

To allow one to be notified of all exceptions happening within the handling of sending a mail, I propose to add an $exceptionHandler property to the class, that would allow one to manually add a callback for handling exceptions in an additional way. For example, this would allow one to manually send the exception to some service in a structured manner, so one can address the failing transport, while sending would still continue properly as long as there's still a succeeding transport left.

@carsonbot

This comment was marked as outdated.

@jnoordsij jnoordsij marked this pull request as ready for review April 1, 2025 12:37
@carsonbot carsonbot added this to the 7.3 milestone Apr 1, 2025
@carsonbot carsonbot changed the title [Mailer][Transport] Allow custom exception handling for RoundRobinTransport mailer [Mailer] [Transport] Allow custom exception handling for RoundRobinTransport mailer Apr 1, 2025
@stof
Copy link
Member

stof commented Apr 1, 2025

Injecting an exception handler callback looks uncommon to me. This looks like a use case for injecting a logger and performing some logging.

@jnoordsij jnoordsij force-pushed the roundrobin-mailer-error-handler branch from 40b9b2c to 4de5ed8 Compare April 1, 2025 13:29
@jnoordsij
Copy link
Contributor Author

Thanks for your suggestion! While I figured a callback would allow for more flexibility, I do agree that a logger is probably a more standard way of approaching this. I've adjusted to inject a LoggerInterface instead. Let me know what you think!

@jnoordsij jnoordsij changed the title [Mailer] [Transport] Allow custom exception handling for RoundRobinTransport mailer [Mailer] [Transport] Allow custom exception logging for RoundRobinTransport mailer Apr 1, 2025
@jnoordsij jnoordsij changed the title [Mailer] [Transport] Allow custom exception logging for RoundRobinTransport mailer [Mailer] [Transport] Allow exception logging for RoundRobinTransport mailer Apr 1, 2025
@OskarStark OskarStark changed the title [Mailer] [Transport] Allow exception logging for RoundRobinTransport mailer [Mailer][Transport] Allow exception logging for RoundRobinTransport mailer Apr 2, 2025
@fabpot fabpot modified the milestones: 7.3, 7.4 May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants