Failures prevent future connections #63

Closed
gtracy opened this Issue Mar 21, 2013 · 10 comments

Projects

None yet

5 participants

@gtracy
gtracy commented Mar 21, 2013

I'm running into a problem where a send() call fails with the below error. After the failure, all future send() calls fail silently.

I've experimented with recreating the connection with emailjs.server.connect() but that doesn't seem to resolve it either.

What pattern do most people use? Since, I don't see a disconnection(), I've created a single connection and send all messages through that connection.

{ [Error: connection encountered an error]
code: 5,
previous: { [Error: connect ETIMEDOUT] code: 'ETIMEDOUT', errno: 'ETIMEDOUT', syscall: 'connect' },
smtp: undefined }

@defunctzombie
Contributor

I am seeing the same issue. I am exploring ways around this as I believe when this happens the SMTP connection must be destroyed and a new one established.

@gtracy
gtracy commented Mar 22, 2013

@shtylman Have you been able to build a reliable test harness to reproduce the problem?

@defunctzombie
Contributor

I have not dug into it enough. Honestly, given that I use sendgrid for
email, I am debating just moving to the http api over SMTP. I will shed a
tear as this module has been good to me, but sometimes we have to move
forward :)

If I do stick with smtp, I will dig into this.

On Thu, Mar 21, 2013 at 10:04 PM, Greg Tracy notifications@github.comwrote:

@shtylman https://github.com/shtylman Have you been able to build a
reliable test harness to reproduce the problem?


Reply to this email directly or view it on GitHubhttps://github.com/eleith/emailjs/issues/63#issuecomment-15277323
.

@rianhunter

i know why this happens, it's because in the response function created by SMTP.prototype.connect invoked by SMTPResponse (while handling the socket error event), it does not call self.close(). here's the fix (in emailjs/smtp/smtp.js):

      var response = function(err, msg)
      {
         if(err)
         {
            self.close(true); // <------ THIS LINE MUST BE ADDED
            caller(callback, err);
         }
         else if(msg.code == '220')
         {
            log(msg.data);

            // might happen first, so no need to wait on connected()
            self._state = SMTPState.CONNECTED;
            caller(callback, null, msg.data);
         }
         else
         {
            log("response (data): " + msg.data);
            self.quit();
            caller(callback, SMTPError("bad response on connection", SMTPError.BADRESPONSE, err, msg.data));
         }
      };
@swang swang added a commit to metamx/emailjs that referenced this issue May 7, 2013
@swang swang Added fix described in eleith/emailjs#63 116a62d
@swang
Contributor
swang commented May 9, 2013

Hey @rianhunter, are you going to submit these changes as a patch/PR?

@rianhunter

@swang nope, feel free to submit!

@defunctzombie
Contributor

@rianhunter please submit these changes as a PR. This is what GH is based around.

@rianhunter

Don't really have the time but feel free to take this code and submit a PR on your own behalf.

On Jul 22, 2013, at 5:18 PM, Roman Shtylman notifications@github.com wrote:

@rianhunter please submit these changes as a PR. This is what GH is based around.


Reply to this email directly or view it on GitHub.

@swang
Contributor
swang commented Jul 23, 2013

I already submitted a PR for this. Just no merge from @eleith

@eleith
Owner
eleith commented Apr 7, 2014

the PR was eventually merged in

@eleith eleith closed this Apr 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment