Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Mailer] [Transport] Allow exception logging forRoundRobinTransport 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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@jnoordsij
Copy link
Contributor

@jnoordsijjnoordsij commentedApr 1, 2025
edited
Loading

QA
Branch?7.4
Bug fix?no
New feature?yes
Deprecations?no
IssuesN/A
LicenseMIT

TheRoundRobinTransport class (and by extension theFailoverTransport 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$logger property to the class that implementsLoggerInterface, that would allow one log exceptions from individual mailers used. For example, this would allow one to manually send the exception to some logging 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.

@jnoordsijjnoordsij marked this pull request as ready for reviewApril 1, 2025 12:37
@carsonbotcarsonbot added this to the7.3 milestoneApr 1, 2025
@carsonbotcarsonbot changed the title[Mailer][Transport] Allow custom exception handling for RoundRobinTransport mailer[Mailer] [Transport] Allow custom exception handling for RoundRobinTransport mailerApr 1, 2025
@stof
Copy link
Member

stof commentedApr 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.

@jnoordsijjnoordsijforce-pushed theroundrobin-mailer-error-handler branch from40b9b2c to4de5ed8CompareApril 1, 2025 13:29
@jnoordsij
Copy link
ContributorAuthor

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 aLoggerInterface instead. Let me know what you think!

@jnoordsijjnoordsij changed the title[Mailer] [Transport] Allow custom exception handling for RoundRobinTransport mailer[Mailer] [Transport] Allow custom exception logging for RoundRobinTransport mailerApr 1, 2025
@jnoordsijjnoordsij changed the title[Mailer] [Transport] Allow custom exception logging for RoundRobinTransport mailer[Mailer] [Transport] Allow exception logging for RoundRobinTransport mailerApr 1, 2025
@OskarStarkOskarStark changed the title[Mailer] [Transport] Allow exception logging for RoundRobinTransport mailer[Mailer][Transport] Allow exception logging forRoundRobinTransport mailerApr 2, 2025
@fabpotfabpot modified the milestones:7.3,7.4May 26, 2025
@jnoordsijjnoordsijforce-pushed theroundrobin-mailer-error-handler branch 2 times, most recently from3c28f9e to8f6473eCompareJune 12, 2025 13:18
@jnoordsijjnoordsijforce-pushed theroundrobin-mailer-error-handler branch from8f6473e to13cfa6aCompareJune 21, 2025 11:30
@jnoordsij
Copy link
ContributorAuthor

Rebased to resolve merge conflicts. Let me know if there is anything I can do to help get this merged.

@carsonbotcarsonbot changed the title[Mailer][Transport] Allow exception logging forRoundRobinTransport mailer[Mailer] [Transport] Allow exception logging forRoundRobinTransport mailerJun 21, 2025
@jnoordsijjnoordsijforce-pushed theroundrobin-mailer-error-handler branch froma324570 to4e62b59CompareJune 21, 2025 15:36
@jnoordsij
Copy link
ContributorAuthor

Thanks for the review suggestions! Changes have been applied; this is ready for review once more.

@fabpotfabpotforce-pushed theroundrobin-mailer-error-handler branch from4e62b59 to834f876CompareJuly 8, 2025 06:03
@fabpot
Copy link
Member

Thank you@jnoordsij.

jnoordsij reacted with thumbs up emoji

@fabpotfabpot merged commitdc81e07 intosymfony:7.4Jul 8, 2025
11 checks passed
@jnoordsijjnoordsij deleted the roundrobin-mailer-error-handler branchJuly 8, 2025 06:35
@holtkamp
Copy link
Contributor

@jnoordsij would it be an idea to also be able to provide the preferred LogLevel (which defaults to "error") in the constructor and use$this->logger->log($this->logLevel, $message)?

I can imagine scenarios / systems in which not being able to send e-mails is a bit more urgent than the currently hard-coded LogLevel "error"...

@jnoordsij
Copy link
ContributorAuthor

@holtkamp This definitely can be a viable user scenario I'd say, although I think defaulting toerror makes a lot of sense (we have captured an exception after all).

Given that this PR is now merged, I'd say if you have any ideas about how to achieve such a thing and why this is important, you can create a new PR targeting7.4 to introduce this.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

7.4

Development

Successfully merging this pull request may close these issues.

5 participants

@jnoordsij@carsonbot@stof@fabpot@holtkamp

[8]ページ先頭

©2009-2025 Movatter.jp