-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Meteor email keeps SMTP connection opened and causes error with some email providers #8591
Comments
We are experiencing the same problem here at Rocket.Chat |
Downgrading to |
Thanks @HoptimizeME we used your temporary fix on RocketChat/Rocket.Chat#6678 |
I think I've figured out why things have behaved differently from old, in my changes in #8495. Namely, the old way used |
In other good news, the latest Nodemailer also just switched back to MIT license, so this will hopefully simplify everything... (see #8425 for the messy old story) |
Had to step down from Meteor 1.4.4 to resolve. This is really horrible for us as it caused all mail services to break/time out and eventually have keys revoked. Have patched in the meantime, but @abernix I think this could turn out to be a pretty serious issue as people upgrade to 1.4.4 |
Might be worth a Meteor release that points back to |
@edemaine any luck with the email package updates? |
Yes, I've finished a draft here: https://github.com/edemaine/meteor |
Specifically, you can copy the packages/email directory into your project's packages directory, then |
Awesome, will give this a try in the morning on my side. Thanks @edemaine |
* Most critically, use a pool instead of direct SMTP connection, to handle dropped connections and increase throughput, like mail module 1.1. (meteor#8591) * New nodemailer's sendMail wants an options object, not a MailComposer object. Luckily, a MailComposer object has a "mail" field that remembers the original options, so we can keep original behavior. * However, we no longer support the mailComposer option set to a compiled MailComposer object (functionality that was briefly added in 1.2.0). * nodemailer does SMTP URL parsing now automatically for us, simplifying code. * Tests' outputs now end with additional "\r\n" * Drop underscore package dependency (no longer needed)
Good morning Erik! I tried your devel package and it seems to work well. I did my tests using Amazon SES. When the email is sent, I see the connection to the mail server staying established for about 10 seconds. I understand this package doesn't I also tried to send 3 emails in the same connection and it worked well. Thanks!! |
I've also just tried and unfortunately it seems that I still have issues sending through the Sendgrid STMP API. My question is, like @HoptimizeME mentioned above, if this doesn't If so, I'm assuming the switch to modern nodemailer in this case mainly helps by handling the timeout error, correct? |
@etyp do you also have issues sending emails with I'm asking that because I'm mostly convinced
When I send email with p.s. *rant* I hate encrypted connection, so hard to debug with tcpdump ... |
@HoptimizeME nope, didn't have the same issues with Sendgrid also has unreliable documentation/error verbosity, so I'll have to poke around a bit more to see. |
Indeed, 1.1.18 also maintained a pool of connections (up to 5, I think), and 1.2.1 should restore this behavior. Please ignore 1.2.0, which didn't use a pool, and instead maintained a single connection. But 1.2.1 should restore the behavior of 1.1.18, except that now it's using a much newer version of Nodemailer. @HoptimizeME, I'm glad 1.2.1 is working for you. I've also done a bunch of tests, with several minutes or even an hour between sending messages, and 1.2.1 seems to have had no problem with reconnecting as needed. @etyp, could you elaborate on the problem you're having with Sendgrid SMTP? Do you get an error, or hanging behavior, or something else? |
@edemaine sure - so with Sendgrid SMTP, no errors are thrown in the Meteor app, but once the email comes through, the subject, body, etc. is completely blank. I've replaced our Sendgrid SMTP API usage throughout the app withe the HTTP API as a stop gap, and would suggest anybody who has this same issue do the same until we can get to the bottom of this. Unfortunately, I don't have enough time today to dig into the issue with Sendgrid any further, but will open a ticket with them first thing tomorrow to help diagnose what they might be receiving on their end that causes the empty subject line and email. Sorry, I wish I had more time today - I'll provide updates as soon as possible. |
@etyp Interesting. The generated email format in the new Nodemailer is different from the old one, though my reading of the spec was that this format was more correct than old. But perhaps there's an incompatibility... Can you paste the exact message that came through? Were From and To headers also blank? |
We are waiting on this to close: |
@engelgabriel Would you be able to test the email package from https://github.com/edemaine/meteor/tree/devel/packages/email to see if it works in your scenario(s)? |
I've asked @rodrigok to test it and will get back to you. Would that fix be released soon? |
@engelgabriel We can release the |
Oh, right, forgot that was possible. That might be worth doing now; my tests and @HoptimizeME's suggest that 1.2.1 is reasonably functional -- definitely a major improvement over 1.2.0. If we end up needing to make more changes, or revert all the way back to 1.1.18, we can just increment the version number again... |
@edemaine It's working for us using your version |
* Switch to modern nodemailer 4, Node 4 version. Fix #8591 * Most critically, use a pool instead of direct SMTP connection, to handle dropped connections and increase throughput, like mail module 1.1. (#8591) * New nodemailer's sendMail wants an options object, not a MailComposer object. Luckily, a MailComposer object has a "mail" field that remembers the original options, so we can keep original behavior. * However, we no longer support the mailComposer option set to a compiled MailComposer object (functionality that was briefly added in 1.2.0). * nodemailer does SMTP URL parsing now automatically for us, simplifying code. * Tests' outputs now end with additional "\r\n" * Drop underscore package dependency (no longer needed) * General formatting/style cleanup for `packages/email`. * snake_cased => camelCased for some local variables. * Added curly-brackets to `if`s. * Removed trailing spaces. * Removed commented-out code. * Removed older doc text and changed some links. * Get rid of back-and-forth assigning of `mailUrlString`.
Thanks to the great work by @edemaine in #8605, this should be fixed with
Closing, but happy to reopen if this persists. /cc @rodrigok |
The real published package ( Thanks @edemaine for the quick fix :) |
Updating Meteor has fixed the issue on all my sites, many thanks ! |
I'm going to jump in here. I'm still having issues with
This all started when I upgraded to I didn't have this issue in earlier versions, but now after upgrading Meteor, package constraints make it impossible for me to downgrade the |
Using Amazon SES SMTP here and it works well with @michaelcbrook Which SMTP zone? smtp or smtps? Which port? What does your MAIL_URL look like? Mine looks like this:
Try to add debug to the MAIL_URL. The debug will appear in your server's console. You might be able to better see what is happening.
|
Side note: I have just realized that the pool option may now be selectively disabled (it is activated by default in the
Personally, I will keep it activated on Amazon SES SMTP even though Amazon times out and closes the pool after 10 seconds. At least, if I have bursts of emails, they will enjoy using the SMTP channel that is already opened. Thanks @edemaine for the complete handling of the mailUrl string. That's a very nice addition to |
@HoptimizeME Here is my MAIL_URL format:
And after enabling debugging, this is the result:
|
Aha! I'm 90% certain that should be
|
That worked flawlessly! Thanks @edemaine! |
* Switch to modern nodemailer 4, Node 4 version. Fix #8591 * Most critically, use a pool instead of direct SMTP connection, to handle dropped connections and increase throughput, like mail module 1.1. (#8591) * New nodemailer's sendMail wants an options object, not a MailComposer object. Luckily, a MailComposer object has a "mail" field that remembers the original options, so we can keep original behavior. * However, we no longer support the mailComposer option set to a compiled MailComposer object (functionality that was briefly added in 1.2.0). * nodemailer does SMTP URL parsing now automatically for us, simplifying code. * Tests' outputs now end with additional "\r\n" * Drop underscore package dependency (no longer needed) * General formatting/style cleanup for `packages/email`. * snake_cased => camelCased for some local variables. * Added curly-brackets to `if`s. * Removed trailing spaces. * Removed commented-out code. * Removed older doc text and changed some links. * Get rid of back-and-forth assigning of `mailUrlString`.
* Switch to modern nodemailer 4, Node 4 version. Fix #8591 * Most critically, use a pool instead of direct SMTP connection, to handle dropped connections and increase throughput, like mail module 1.1. (#8591) * New nodemailer's sendMail wants an options object, not a MailComposer object. Luckily, a MailComposer object has a "mail" field that remembers the original options, so we can keep original behavior. * However, we no longer support the mailComposer option set to a compiled MailComposer object (functionality that was briefly added in 1.2.0). * nodemailer does SMTP URL parsing now automatically for us, simplifying code. * Tests' outputs now end with additional "\r\n" * Drop underscore package dependency (no longer needed) * General formatting/style cleanup for `packages/email`. * snake_cased => camelCased for some local variables. * Added curly-brackets to `if`s. * Removed trailing spaces. * Removed commented-out code. * Removed older doc text and changed some links. * Get rid of back-and-forth assigning of `mailUrlString`.
Please change the readme to uncomment the port and host and to use smtps. meteor/meteor#8591 This is the solution that worked for me in regards to this bug.
Hi I upgraded my meteor version from 1.4.2.* to 1.4.4.2 now I'm having the same error. Here's the debug logs:
My MAIL_URL is smtp://user:password@email-smtp.us-east-1.amazonaws.com:587 email version 1.2.1 Any ideas? |
ok found the issue. I was using nodejs 4.2.6 which comes with the official ubuntu repository. I updated to nodejs 4.8.3 and the error was gone. |
@ianpogi5 FWIW, |
Not sure what things need to do with this error, I cant send email.
|
@juvyc Do you have a correct |
please explain MAIL_URL format in gmail err for MAIL_URL\error greeting never received code 'etimedout' |
RE: #8425 #8495
@edemaine @abernix
Since the "Upgrade to mailcomposer@4 and smtp-connection@2." I have issues with SMTP connections.
The MDG-email package keeps the SMTP connection opened. It never send a SMTP "QUIT" command.
Some email providers, for instance Amazon SES, don't like people to keep their connections opened. Amazon SES close the SMTP connection after 10 seconds with a timeout message and when it happens I get this error:
This error wouldn't happen if you would close the SMTP connection after having sent the email.
Also, this error message is very hard to process in a try/catch because it's impossible to know if the mail has been sent correctly prior the timeout. In my particular case, the "421 Timeout" happens after the "250 Ok" (that follows the email being sent). So I could probably ignore the error ... but if the "421 Timeout" was real, I would miss the chance to retry sending the email.
(edit: I am not sure if the previous package also kept the SMTP connection opened and simply ignored the resulting error)
The text was updated successfully, but these errors were encountered: