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

MailHelper: Reset transport as well on reset, solves connection drops #6980

Merged

Conversation

jbransen
Copy link
Contributor

@jbransen jbransen commented Dec 7, 2018

Q A
Bug fix? Y
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

This PR improves the way the connection to an external SMTP server is reset. In particular, when using Amazon SES, on regular basis we saw the following error in the logs:

mautic.ERROR: [MAIL ERROR] Expected response code 250 but got code "", with message "" Log data: >> MAIL FROM:<***@****.net>  <<  !! Expected response code 250 but got code "", with message "" (code: 0) (send);

This seems to be related to a connection losses, so to solve those, this PR closes the connection upon resetting the MailHelper. It is hard to reproduce this exact bug, but at least if feels very natural to close the connection upon a reset, and in our environment this has solved the issue.

Steps to reproduce the bug:

  1. Send out many emails in different campaigns
  2. Observe occasional errors as the one above

Steps to test this PR:

  1. Load up this PR
  2. Send out many emails in different campaigns
  3. Notice the errors being gone :-)

@Woeler Woeler added bug Issues or PR's relating to bugs code-review-needed PR's that require a code review before merging labels Dec 7, 2018
@Woeler Woeler added this to the 2.15.1 milestone Dec 7, 2018
@heathdutton heathdutton added this to Code Review (2 required) in Mautic 2 Dec 12, 2018
@npracht npracht added the ready-to-test PR's that are ready to test label Dec 18, 2018
@npracht npracht moved this from Code Review (2 required) to Ready to Test (first time) in Mautic 2 Jan 3, 2019
@npracht npracht moved this from Ready to Test (first time) to Code Review (2 required) in Mautic 2 Jan 3, 2019
@alanhartless alanhartless added this to Needs Testing in 2.15.1 Jan 14, 2019
Mautic 2 automation moved this from Code Review (2 required) to Ready to Test (first time) Jan 30, 2019
@alanhartless alanhartless merged commit 1295268 into mautic:staging Jan 30, 2019
Mautic 2 automation moved this from Ready to Test (first time) to Merged Jan 30, 2019
@alanhartless alanhartless moved this from Needs Testing to Discussion in 2.15.1 Jan 30, 2019
@alanhartless alanhartless moved this from Discussion to Merged in 2.15.1 Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs code-review-needed PR's that require a code review before merging ready-to-test PR's that are ready to test
Projects
No open projects
2.15.1
Merged
Development

Successfully merging this pull request may close these issues.

None yet

4 participants