Library is not promisify-able

[antony]

To keep my application's strhatucture and approach consistent, I'm using Bluebird's promisify function to turn callbacks into promises.

Unfortunately, and it seems to be due to the use of 'this' inside sendgrid.js, promisifying the 'send' method on SendGrid causes 'this' to become undefined, and hence 'api_user' cannot be fetched, and email cannot be sent.

This is a shame as it forces me back to the callback approach.

May I suggest that the internals of the library bind the correct context when using this, or _this is used (as it does appear to be defined within sendgrid.js).

If you take pull requests, I'd be happy to make these fixes myself.

[thinkingserious]

Hello @antony,

We will add this request to our back log.

Yes, we do accept pull requests and would be happy to review your changes.

Thanks!

[antony]

Great - thanks for that!

[dslounge]

I just wanted to second this issue. I was hoping I could promisify but it didn't work. @antony if you end up making that change it would make my life easier :D

[dslounge]

Btw, it's not a big deal to wrap sendgrid.send in a promise if anybody really needs it. All you have to do is (es6 example):

const Promise = require('bluebird');
const sendgrid = require('sendgrid')(sendgridKey);
const sendEmail = (params) => {
  return new Promise((resolve, reject) => {
    sendgrid.send(params, (err, json) => {
      if (err) {
        reject(err);
      } else {
        resolve(json);
      }
    });
  });
};

sendEmail(emailParams)
  .then((json) => {
    //success
  })
  .catch((err) => {
    //error
  });

[mynameiscoffey]

@antony @dslounge there is actually already a built in way to handle this using Bluebird's promisify method by passing in a context for Bluebird to use for "this"

const Promise = require('bluebird');
const sendgrid = require('sendgrid')(sendgridKey);
const sendEmail = Promise.promisify(sendgrid.send, { context: sendgrid });

[manuel-di-iorio]

I would like to not use Bluebird at all. When a callback to sendgrid.send() is not provided, just return a promise. Bluebird is actually a temporary workaround

[dslounge]

@mynameiscoffey your solution looks like the correct one.

[lolobosse]

Hey people,

Maybe something useful for people using Parse and Sendgrid together and having migrated to their own server because Parse is closing. Here is the "Parse.Promified" version of @dslounge code (which I want you to star, it saved me one hour):

return new Parse.Promise(function (resolve, reject) {
  sendgrid.send({
    to: xxx,
    from: xxx,
    subject: xxx,
    text: xxx,
    replyto: xxx
  }, function (err, json) {
    if (err) {
      reject(err);
    } else {
      resolve(json);
    }
  });
});

[thinkingserious]

Hello everyone,

I'm not sure is the 'this' issue persists in the new version of this library, so I'm keeping this one open for now.

You can check out the new library here: https://github.com/sendgrid/sendgrid-nodejs/tree/v3beta. It will be out of beta in a few weeks.

Thanks for your support!

With Best Regards,

Elmer

[adamreisnz]

@thinkingserious have you given any thought to @manuel-di-iorio's suggestion? E.g. return promises from your API if no callback is provided? That would remove the need for having to use promisification altogether.

[thinkingserious]

Not yet @adambuczynski, but I'm definitely open to any improvements to this library.

Currently, we just released our new v3 /mail/send endpoint out of beta and in the near future we will fixing any bugs that surface since the re-launch of all our libraries yesterday.

I'm leaving this ticket open for further consideration. Any additional feedback you have is appreciated!

[adamreisnz]

@thinkingserious yeah I've looked into the code, there are some improvements that I would suggest. I'll see if I have time to create some PR's for them, including this one.

[adamreisnz]

I have created this wrapper for now to be able to work with sendgrid in a promise chain:

/**
 * Send email (wrapped in promise)
 */
function sendMail(mail) {
  return new Promise((resolve, reject) => {

    //Build request
    let request = sg.emptyRequest();
    request.method = 'POST';
    request.path = '/v3/mail/send';
    request.body = mail.toJSON();

    //Send request
    sg.API(request, response => {
      if (response && response.statusCode &&
        response.statusCode >= 200 && response.statusCode <= 299) {
        resolve(response);
      }
      reject(new SendMailError(
        'Sendgrid response error ' + response.statusCode
      ));
    });
  });
}

[thinkingserious]

@adambuczynski has added support here: #261

To help move it up the queue for merge, we need comments and +1's

Thanks everyone!

[adamreisnz]

@thinkingserious I think you meant to link to the PR, #261 :)

[thinkingserious]

Fixed with https://github.com/sendgrid/sendgrid-nodejs/releases/tag/v4.0.0

[vsmori]

@adamreisnz , one fix on your code: acording to documentation, the function receives error as first param:

sg.API(request, function (error, response) {

Anyway, thanks you sharing!

[adamreisnz]

Ok, maybe that has changed or maybe I missed it, thanks :)
I have since created this simple wrapper library to simplify the interaction with the Sendgrid package: https://www.npmjs.com/package/sendgrid-mailer