[WIP] Replace sendmail with direct in nodemail #1601

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants
Member

javorszky commented Nov 30, 2013

Fixes #1538.

  • upgraded nodemailer to 0.5.11 (current master as of commit)
  • implemented direct mail sending, and ripped out sendmail (we don't need sendmail anymore with direct being in place)
  • modified tests to look for direct instead of sendmail
Owner

ErisDS commented Nov 30, 2013

"Ghost is attempting to use nodemail's direct transport to send e-mail.",
 +            "Keep in mind, because of this, the emails will probably land in your",
 +            "spam folder. It is recommended that you explicitly configure an e-mail",
 +            "service to avoid that.",
 +            "<br>See <a href=\"http://docs.ghost.org/mail\">http://docs.ghost.org/mail</a> for instructions"

Direct mail landing in spam is a bit of a problem. Not a major one, but it means this switch is not quite as straightforward as I'd have liked. Ultimately what we want is to reduce the complexity around mail for Ghost, yet here we still need to tell people to configure SPF headers or to configure a mail relay service.

I'm thinking that as we are ONLY using mail to send forgotten password emails, the best thing we can do right now is use direct mail by default, and tell people that their email might land in spam when they attempt to request a forgotten password email, instead of when they start up.

In future when we add a proper first-run / installer feature to Ghost, we'll have a better opportunity to inform people about mail configuration.

I'd really like to get rid of the startup warning, and of the example config in the config.example.js.

Member

javorszky commented Nov 30, 2013

Updated the PR. Best I could do.

  • uses the url setting in the config.js
  • unless it's my-ghost-blog.com, because we don't want to blacklist that. Uses localhost then instead
  • spf records match sender email and sender machine -> SPF doesn't softfail. Look below:

screenshot 2013-11-30 23 28 23

🍩 ? :)

Owner

ErisDS commented Dec 1, 2013

Turning debug mode on I can see I'm getting the following error:

SERVER:
└──550-5.7.1 [37.152.250.142] The IP you're using to send mail is not authorized
 to
   550-5.7.1 send email directly to our servers. Please use the SMTP relay at yo
ur
   550-5.7.1 service provider instead. Learn more at
   550 5.7.1 http://support.google.com/mail/bin/answer.py?answer=10336 d6si17288
813wic.19 - gsmtp
CLIENT:
└──QUIT
Failed processing message #
Message #1 requeued for 15 minutes

However, I get a success notification when clicking the password reset.

Member

javorszky commented Dec 1, 2013

Hm... Interesting. I mean... if that fails there, then there's not much I can do, besides trying to handle the error. But that will not be useful to the average user.

Also... holdon, testing something.

Member

javorszky commented Dec 1, 2013

Interesting... I sent it to my own gmail address from localhost, and this is what I get:

SERVER:
└──220 mx.google.com ESMTP fs2si28192048wjb.88 - gsmtp
CLIENT:
└──EHLO Gabors-MacBook-Pro.local
SERVER:
└──250-mx.google.com at your service, [86.156.146.167]
   250-SIZE 35882577
   250-8BITMIME
   250-STARTTLS
   250-ENHANCEDSTATUSCODES
   250 CHUNKING
CLIENT:
└──MAIL FROM:<ghost@localhost>
SERVER:
└──250 2.1.0 OK fs2si28192048wjb.88 - gsmtp
CLIENT:
└──RCPT TO:<javorszky.gabor@gmail.com>
SERVER:
└──250 2.1.5 OK fs2si28192048wjb.88 - gsmtp
The following addresses were rejected for #1:
CLIENT:
└──DATA
SERVER:
└──354  Go ahead fs2si28192048wjb.88 - gsmtp
Transmitting message #1

I'll try to capture a failure.

andris9 commented Dec 1, 2013

Just to point out, the direct mail method should have more or less the same succes rate for sending mail as with sendmail - for the receiving server there is no difference which method was used, the transmission looks exactly the same. Additionally both sendmail and direct do not send the message immediatelly but queue it, so they are not able to return explicit success/failed status (message status is not known yet when the callback is fired). This is because sending the mail may take time, for example when graylisting is used on the receiving server, the mail can not be sent for about 15 minutes after the initial send command was used - most probably you would not like to wait your callback for 15 minutes (this is why both sendmail and direct return immediatelly with "message queued" statement). If really needed I can add additional callback or event to the direct method which runs when the message is finally delivered or failed permamently (but as stated before, this might take time until the callback is fired - in most cases it should fire almost immediatelly but in some cases it might take several hours).

The empty "The following addresses were rejected for #1:" line is a bug - the info is currently always printed out, even if there were no rejected addresses (fixed it with andris9/simplesmtp@4c75523)

Member

javorszky commented Dec 2, 2013

Hm, so I guess I'll just get rid of the extra code, fix jslint, and update. There's not much we can do, if I understand correctly? Also there's a callback being bassed to sendMail(message, callback), and that callback receives err, if there was one. But as it's queued, and may take a while, there really is no point, because if it's in the Q for 2 minutes, the user might have navigated away.

So um... by configuring the url, direct will be able to deliver to inbox rather than spam. Localhost though... hit or miss, which we can let them know. For example, @ErisDS 's email was rejected from localhost. Mine arrived in spam. Both were sent from ghost@localhost.

Member

javorszky commented Dec 2, 2013

So Travis is having a hissy-fit with something internal (job 640), which I can't restart :(

javorszky referenced this pull request Dec 3, 2013

Closed

CasperJS random failures #1609

Owner

ErisDS commented Dec 3, 2013

The problem that I have now, is that I cannot verify that this works at all :(

Member

javorszky commented Dec 4, 2013

Um, why? What is missing? (besides travis failing)

Owner

ErisDS commented Dec 5, 2013

Direct mail doesn't work for me when running Ghost on localhost which is where I verify PRs. I understand the reasons why but effectively I get a green light and no mail. I have no way to know how many people that will happen for.

The idea of switching from requiring a configuration to direct mail, is that it is easier and going to result in fewer support requests / frustrated people locked out of their accounts because they cannot reset their password.

However, if a lot of people will now get an OK message, instead of an "email not configured" message, and never receive an email, this will result in more questions & frustration, not less.

So at the very least, in my opinion, this needs to be heavily tested and mitigated as best we can.

I'm thinking:

  1. we should test on common platforms like Digital Ocean and rackspace - does direct mail work there?
  2. we should speak to people who make auto-installers like bitnami and softaculous - are they able to get direct working reliably or will they still require a config
  3. if we can determine that direct mail is only really problematic if you are installed locally rather than on a server, then we can add additional messages for people running Ghost on localhost.

This is a whole tonne of work, so I'm thinking we might want to punt it for 0.5 and make a concerted effort with our partners and providers to switch to direct mail. I realise the work is done and that it effectively "works" but it isn't right to rush into this if it's going to leave people not understanding why they can't get any emails.

Member

javorszky commented Dec 5, 2013

Sure, I understand the reasons, and I +1 what you're saying.

I can champion working with the providers and to test it out for 0.5, and then I can have a nice matrix as per @matthojo's "Ghost works on..." thing.

At the very least it will make me know more about email than I ever wish I did. 😄

andris9 commented Dec 5, 2013

I updated Nodemailer in a way which would probably help you out a bit, see handling direct responses (available since v0.5.14).

While you can't do much if your IP is blocked - most dynamic IP's (eg. your home modem connection) are usually blocked and static ones (eg. VPS servers in the cloud) are not, at least by default - it is possible to check what the server responsed and act accordingly. This update exposes if a message was accepted, rejected or requeued - in most cases the message is either sent or failed immediatelly, requeueing should not be frequent. You can see the details from the provided link above.

Member

javorszky commented Dec 5, 2013

Sweet, thank you @andris9! 👍

I'll implement, test and repush later.

Owner

ErisDS commented Dec 6, 2013

This is all good news. I have punted #1538 to 0.5 for now, I think this issue does deserve having some time taken over it.

@javorszky javorszky Replace sendmail with direct in nodemail
Fixes #1538.
* upgraded nodemailer to 0.5.15 (current master as of commit)
* implemented direct mail sending, and ripped out sendmail (we don't need sendmail anymore with direct being in place)
* modified tests to look for direct instead of sendmail
* implemented directmail response data
* made `send` function more promising
* added `successmessage` to successful mail notification function
* added rudimentary error handling for rejected / failed email messages
d5bf13c

hillct commented Feb 7, 2014

Seems to me this issue is more one of documentation. The user who chooses to use a local (or their local ISP) mail server for outgoing mail from Ghost must set the proper SPF record http://www.openspf.org/ such that mail servers that check SPM records will consider the mail to be authorized. Those users who are running locally on a machine that's part of a dynamic network block will likely get such errors because as a general matter most mail servers filter messages coming from dynamic IPs, but of course that won't be an issue for any hosted scenario where a leased or purchased physical or virtual server is used to run ghost. In such scenarios, proper SPF records as well as in-addr-arpa addressing is important.

Beyond this sort of documentation, from a development standpoint the only issue is whether or not the emails being sent via nodemail is well-formed with proper headers and doesn't contain unreasonably spammy phrasing.

Member

javorszky commented Feb 8, 2014

I think from a technical point of view the emails are good, and I have taken all reasonable measures to let the mail servers know the emails sent aren't spam.

@ErisDS, if I rebase and repush, what else stands in the way of merging this?

Owner

ErisDS commented Mar 3, 2014

@javorszky I agree with @hillct about this being largely a documentation issue - I'd like to have a clear guide as to under what circumstances mail may fail to arrive & how to resolve it - a troubleshooting guide as it were.

If it's purely a case of, on your local machine / in development mode, then we can add additional messaging and documentation around this.

I'd also like to test the new sending feature on a couple of platforms - Digital Ocean, rackspace & perhaps azure to see if the emails arrive or not. What do you think?

Member

javorszky commented Mar 3, 2014

I shall rebase this (or probably take note of what I've done and reimplement on new master), write up documentation, and recommit. I'd like this to arrive :).

Since we can also test whether the URL is localhost or some variation thereof (like, can I get external IP address of it from some API (not sure yet)). I shall go away and do stuff, test stuff and let y'all know :).

ErisDS added the server label Mar 3, 2014

Owner

ErisDS commented Aug 29, 2014

GitHub spring clean in progress. @javorszky I know you're still working on this and you can either 1) drop a comment on this when it's ready and I can reopen or 2) raise a new PR. No real need to keep this open in the meantime I don't think.

ErisDS closed this Aug 29, 2014

@ErisDS ErisDS modified the milestone: 0.5.x Backlog Sep 23, 2014

ErisDS removed the server label Sep 23, 2014

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