veliovgroup/mail-time

Emails go out of order sometimes

theoutlander opened this issue · 5 comments

I have a suggestion:

It appears that emails can go out of order. If I were to enqueue multiple emails for a user a few seconds apart, they can be sent out in a random order. This breaks the user experience.

When a user signs-up, we enqueue a thank-you email. Once a specific activity is completed after that, another email is enqueued. This could be a few seconds to minutes apart. It would be nice if the order of those emails is preserved based on the email address and when it was enqueued.

Mail-Time 0.1.7
Node 8.9.1
Npm 5.6.0

Hello @theoutlander ,

  1. Please post your definition of new MailTime({ /*...*/ });.
  2. What version of nodemailer you're on?
  3. .sendMail() method accepts sendAt option, you can order emails using this option;
  4. Is this behavior easily reproducible? Want to make sure it's a bug, and not a network/mail protocol flaw.

I have noticed during our pre-production testing that in my first test the emails were out of order. I haven't tried to repro it, but we are testing our app for the next release so I will be able to answer it after a few more runs.

The sendAt option won't work most likely as these are supposed to be instantaneous emails. I don't know how much of a delay would need to be introduced and if the library would ensure the order of delivery.

Here's rest of the info you requested:

    "nodemailer": "^4.4.1",
    "nodemailer-mailgun-transport": "^1.3.6"
let mailServer = new MailTime({
  debug: true,
  db: db, // MongoDB
  type: 'server',
  strategy: 'balancer', // Transports will be used in round robin chain
  transports: transports,
  from: function (transport) { return `****Redacted****` },
  concatEmails: true, // Concatenate emails to the same addressee
  concatDelimiter: '<h1>{{{subject}}}</h1>', // Start each concatenated email with it's own subject
  template: MailTime.Template // Use default template
})

@theoutlander you have two options to tune:

  1. Disable concatenation with concatEmails: false
  2. Lower concatThrottling (which is 1 minute by default);

In a case you have describe user should receive only one email including both messages (sign up, and thank-you).

Closed due to silence at issue owner end.
Feel free to reopen it in case if the issue still persists on your end.

@dr-dimitru maybe should be add sort: {sendAt: 1} in __send? Because by default sort by _id (createdAt), so maybe the lastest add document's sendAt is the earliest.

___send(ready) {
    this.collection.findOneAndUpdate({
      $or: [{
        isSent: false,
        sendAt: {
          $lte: new Date()
        },
        tries: {
          $lt: this.maxTries
        }
      }, {
        isSent: true,
        sendAt: {
          $lt: new Date(Date.now() - (this.interval * 4))
        },
        tries: {
          $lt: this.maxTries
        }
      }]
    }, {
      $set: {
        isSent: true,
        sendAt: new Date(Date.now() + this.interval)
      },
      $inc: {
        tries: 1
      }
    }, {
      returnOriginal: false,
      sort: {sendAt: 1},
      projection: {
        _id: 1,
        tries: 1,
        template: 1,
        transport: 1,
        mailOptions: 1,
        concatSubject: 1
      }
    }