javascript one callback for .catch() and another callback for .then()?

Question

I'm trying to register as user in my mongodb, if that goes well I want to send an email to the registered user. To do that i'm using sendgrid, which ships with a .send(msg) which returns a promise .catch(error) and .then({...}).

If the error is present I want to send a response to the webpage, that the user was registered, but the email failed. I cannot do that directly from the .catch() since it is in another part of the application, out of scope and should be used by multiple other functions....

sendgrid.js

module.exports.sendRegMail = function (user, password, adminUserID, adminName, success, errorCB) {
    msg.to = email;
    msg.subject = `Welcome ${user.name}! You have been registered!`;
    msg.text = `Dear ${user.name}.\nYou just been registered as a ${user.userType}.\n\nPlease log in with following credentials:\nUsername: ${user.username}\nPassword: ${password}\n\nOBS: it is important that you change your password to a personal one in order to use the platform!\nMost features will be disabled until your account has been secured with a personal password.`;
    sendMail(msg, success, errorCB);
}

function sendMail(msg, success, errorCB){
    sgMail.send(msg)
    .then(() => {
        Logger.logUserAction(userID, name, 'Successfully sent email:\n' + JSON.stringify(msg));
        success();
    })
    .catch(error => {
        Logger.logUserAction(userID, name, 'Tried to send email:\n' + JSON.stringify(msg) + '\nBut failed due to error:\n' + error);
        errorCB();
    });
}

cut out from where the user is being saved....

 User.addUser(newUser, (err, user) => {
                if (err) {
                    Logger.logAdminAction(decoded.data._id, decoded.data.name, 'Tried to register user: ' + user + '. but failed. Error: ' + err);
                    res.json({ success: false, msg: 'Failed to register user' });
                } else {
                    Logger.logAdminAction(decoded.data._id, decoded.data.name, 'Successfully created user: ' + user);
                    mail.sendRegMail(user, req.body.password, decoded.data._id, decoded.data.name, ()=>{
                        res.json({ success: true, msg: 'User registered, And email sent successfully!' })
                    }, () =>{
                        res.json({ success: true, msg: 'User registered, but email could not be sent! Contact the person manually.' })
                    });  
                }
            });

as for now i'm trying to give two callback functions as argument in sendRegMail(). and then call one callback in .then() and another callback in .catch() But it seems to me over complicated this way? What is the correct way of handling a promise error/success from a parent function??

Why are you passing callbacks to a function that could simply return a promise for its result? — Bergi, Jan 11 '18 at 14:35
Have a look at the [difference between `.then(…, …)` and `.then(…).catch(…)`](https://stackoverflow.com/q/24662289/1048572). — Bergi, Jan 11 '18 at 14:36
Yes, I've noticed that. wasn't aware that .then actually takes two arguments. Can you tell if if the failure (2nd) callback handles all errors that otherwise would have been caught in .catch? - in other words, can i remove .catch() if i have a failure callback in .then() without missing any functionality? — Rasmus Puls, Jan 12 '18 at 07:58
I would have hoped the linked post explains the difference well enough. It depends on whether the desired functionality is to handle errors from the success callback or not (in your case, it's not). — Bergi, Jan 12 '18 at 13:14

T.J. Crowder · Accepted Answer · 2018-01-11T14:48:14.120

2

It doesn't work because you never call your callbacks. You just refer to them (which is basically a no-op). To call them, you'd have () after them:

function sendMail(msg, success, errorCB){
    sgMail.send(msg)
    .then(() => {
        Logger.logUserAction(userID, name, 'Successfully sent email:\n' + JSON.stringify(msg));
        success();
// ------------^^
    })
    .catch(error => {
        Logger.logUserAction(userID, name, 'Tried to send email:\n' + JSON.stringify(msg) + '\nBut failed due to error:\n' + error);
        errorCB();
// ------------^^
    });
}

That said, why not just return the promise?

function sendMail(msg) {
    return sgMail.send(msg)
    .then(() => {
        Logger.logUserAction(userID, name, 'Successfully sent email:\n' + JSON.stringify(msg));
        // Nothing particularly useful to return here, so just leave it;
        // the promise will resolve with `undefined`
    })
    .catch(error => {
        Logger.logUserAction(userID, name, 'Tried to send email:\n' + JSON.stringify(msg) + '\nBut failed due to error:\n' + error);
        throw error; // <== Because we haven't *handled* it and want it to propagate
    });
}

Then you could use it like this:

sendMail("message")
.then(() => { /* it worked */ })
.catch(err => { /* it failed */ });

...and build it into chains and use it in async functions with await and all the other useful things promises offer over simple callbacks.

edited Jan 11 '18 at 14:48

answered Jan 11 '18 at 14:35

T.J. Crowder

1,031,962
187
1,923
1,875

and then return it again? -- return sendMail(msg); so i get the promis all the way up to where i send the response? (where i before had two callback functions) – Rasmus Puls Jan 11 '18 at 15:00
@RasmusPuls: If there are more layers, yes (for instance, if `sendRegMail` is between where you're generating the response and `sendMail`, then you'd return `sendMail`'s promise from it -- quite probably without any `then` or `catch` in `sendRegMail` at all, but you haven't shown the code for it, so...). – T.J. Crowder Jan 11 '18 at 15:05
Okay, so i tried returning the promise from sendMail to sendRegMail and return it in that function as well. So the last layer where i call mail.senRegMail i now get a promise. I can without error now use .then() and .catch() there. so far so good. Only problem is that the deepest layer (the original promise) catches the error that I am trying to provoke. The point was that the most upper layer of the returned promise also should catch that error, so I can return the error message the the end user. – Rasmus Puls Jan 12 '18 at 08:29
But instead the it runs the .then() as everything went fine.... Is that because the error already are being handled in the deepest/first level? How can I trigger the error in all the layers? - in other words, if the sgMail.send(msg) catches an error, i need to get the error all the way to the top where i call the function chain – Rasmus Puls Jan 12 '18 at 08:30
1

@RasmusPuls: Note the line in the code in the answer in the `catch` callback: `throw error; // <== Because we haven't *handled* it and want it to propagate` :-) Either don't include a `catch` callback at all, or if you do but don't want it to convert rejection to resolution, re-throw the error. Remember that promise chains are *chains* and each link in the chain can transform the result. `catch` callbacks that don't throw (or return a rejected promise) convert rejections into resolutions. – T.J. Crowder Jan 12 '18 at 08:32
You are the boss! Thanks for very fast response. – Rasmus Puls Jan 12 '18 at 08:50

score 1 · Answer 2 · answered Jan 11 '18 at 14:34

1

You miss parenthesis on "success()" and "errorCB()"

answered Jan 11 '18 at 14:34

Mosè Raguzzini

15,399
1
31
43

javascript one callback for .catch() and another callback for .then()?

2 Answers2