You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Description
According to the docs, specifically the following part:
The mailer will start using the first transport. If the sending fails, the mailer won’t retry it with the other transports, but it will switch to the next transport automatically for the following deliveries.
If I understand it correctly, it would always pick the first defined transport and only when this transport fails would use the next defined transport in line.
However this is not the case. This is introduced with the following change 6ebe83c.
How to reproduce
Run the following (a bit of hacky) test multiple times to trigger the 'randomized' code.
public function testPickingFirstTransport()
{
$transport1 = Transport::fromDsn('smtp://mailserver-01:1025');
$transport2 = Transport::fromDsn('smtp://mailserver-02:1026');
$failoverTransport = new FailoverTransport([
$transport1,
$transport2,
]);
$reflector = new ReflectionClass($failoverTransport);
$method = $reflector->getMethod('getNextTransport');
$method->setAccessible(true);
$usedTransport = $method->invoke($failoverTransport);
Assert::assertEquals($transport1, $usedTransport);
}
Possible Solution
Because the FailoverTransport extends the RoundRobinTransport and triggers the parent::getNextTransport();
Do not rely on the parent::getNextTransport() and rely on an own implementation, of picking the transport in defined order and moving to the next when marked as a 'dead transport'.
Make the $cursor variable of the RoundRobinTransport protected, so it can be initiated as protected $cursor = 0; in the FailoverTransport. So it would bypass the 'randomized' code defined in RoundRobinTransport::getNextTransport().
Additional context
If I misinterpreted the docs and misjudged the functionality, ignore this issue ;)
The text was updated successfully, but these errors were encountered:
This PR was merged into the 5.1 branch.
Discussion
----------
[Mailer] Fix failover transport
| Q | A
| ------------- | ---
| Branch? | 5.1 <!-- see below -->
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets | Fix#37593 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
| Doc PR | n/a
This fixes a regression in 5.1.
Commits
-------
8b673f5 [Mailer] Fix failover transport
Symfony version(s) affected: 5.1
Description
According to the docs, specifically the following part:
If I understand it correctly, it would always pick the first defined transport and only when this transport fails would use the next defined transport in line.
However this is not the case. This is introduced with the following change 6ebe83c.
How to reproduce
Run the following (a bit of hacky) test multiple times to trigger the 'randomized' code.
https://github.com/evertharmeling/failover-transport
Possible Solution
Because the
FailoverTransport
extends theRoundRobinTransport
and triggers theparent::getNextTransport()
;parent::getNextTransport()
and rely on an own implementation, of picking the transport in defined order and moving to the next when marked as a 'dead transport'.$cursor
variable of theRoundRobinTransport
protected, so it can be initiated asprotected $cursor = 0;
in theFailoverTransport
. So it would bypass the 'randomized' code defined inRoundRobinTransport::getNextTransport()
.Additional context
If I misinterpreted the docs and misjudged the functionality, ignore this issue ;)
The text was updated successfully, but these errors were encountered: