Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Mailer] Do not ping the SMTP server before sending every message#35633
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| $this->executeCommand("\r\n.\r\n", [250]); | ||
| $message->appendDebug($this->stream->getDebug()); | ||
| $this->lastSent =microtime(true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You only update lastSent when a message has been sent. In case of an error, it won't be reset. By reading the issue, I would have expected that lastSent would be the last time we sent a ping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, because the variable should represent the time when the last message was sent. I chose the name to be consistent with the parentAbstractTransport. Do you want me to change it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
But if sending the email does not work for some reason, you will ping the server next time you try, not sure if that's what we want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't have a strong opinion about this, so I changed it. I updated the name of the field as well, since it now has a slightly different meaning then the one in theAbstractTransport.
fabpot commentedFeb 7, 2020
Thank you@micheh. |
…message (micheh)This PR was squashed before being merged into the 4.4 branch (closes#35633).Discussion----------[Mailer] Do not ping the SMTP server before sending every message| Q | A| ------------- | ---| Branch? | 4.4| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets |Fix#35515| License | MITThis pull request changes the SMTP transport to only ping the server if the last message was sent more than a specified number of seconds ago (instead of pinging the server before every message). By default, it will ping the server if 100 or more seconds since the last message have passed.This should make sending emails with the SMTP transport more robust with many emails, as SMTP servers will often drop the connection if too many non-mail commands are sent (like pinging the server with NOOP commands).Commits-------2817810 [Mailer] Do not ping the SMTP server before sending every message
This pull request changes the SMTP transport to only ping the server if the last message was sent more than a specified number of seconds ago (instead of pinging the server before every message). By default, it will ping the server if 100 or more seconds since the last message have passed.
This should make sending emails with the SMTP transport more robust with many emails, as SMTP servers will often drop the connection if too many non-mail commands are sent (like pinging the server with NOOP commands).