0

I have an HTTP helper that returns a promise for each request. Sometimes those requests depend on each other. I am confusing myself into coding what I think are anti-patterns for this.

Normally when chaining promises you use .then()

function giveMeASimplePromise(){
     return new Promise( function(resolve, reject){
          setTimeout( function(){
               console.log("I'm finishing a promise.");
               resolve();
          }, 3000 );
     });
}

giveMeASimplePromise().then( giveMeASimplePromise ).then( giveMeASimplePromise );

//prints 'I'm finishing a promise' 3 times spaced 3 seconds apart

But I am getting confused if there is a promise within one of those promises that needs to be part of the chain.

This is coming up for me because I have an HTTP helper function that returns a promise with each request. A couple of those requests depend on each other. And the whole process needs to return a promise to keep everything async.

function login( credentials ){
    //outer promise that's returned and either needs to resolve or reject based
    // on complete login cycle
    return new Promise( function(resolve, reject){
        //inner promise checking for username/password match. can be rejected 
        // for several reasons.
        var checkCredentials = httpHelper.checkCredentials( credentials );
        checkCredentials.then( function(result){
            if ( result.credentialsMatch ){
                //another inner promise checking for privilege levels. never rejects.
                var checkPrivileges = httpHelper.getPrivileges( result.userID ); 
                getPrivileges.then( function(result) ){
                    if( result.privilege > 0 ){
                        //want this to resolve the entire chain
                        resolve();
                    }else{
                        //want this to reject the entire chain
                        reject('You do not have sufficient privileges.');
                    }
                }
            }else{
                reject('Incorrect username and/or password.');
            }
        }).catch( function(reason){
            reject( reason );
        });
    });
}

var credentials = { ... };
var loginResult = login( credentials ).then( function(value){
    console.log('Logged in successfully.');
}).catch( function(reason){
    console.log('Could not log in: ', reason);
})

That's an anti-pattern, right? It feels wrong.

trincot
  • 317,000
  • 35
  • 244
  • 286
tmdesigned
  • 2,098
  • 2
  • 12
  • 20
  • Sorry - just left it out. The snippet is a simplification of something a little more interwoven. – tmdesigned Nov 02 '18 at 14:16
  • Creating (and returning) promises in (possibly nested) `then` callbacks is totally fine. But you should avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Nov 02 '18 at 14:22

2 Answers2

2

You should indeed not need to create a promise with new when you already get a promise from a service you use. In that case, just use that promise and chain on it and finally return the chained promise.

When not using new Promise you obviously don't call reject. Instead throw within the then callback(s). Also the nested then calls should better be moved one level up so you get a flat then chain.

Here is how that could look:

function login(credentials) {
    return httpHelper.checkCredentials(credentials).then(function(result) {
        if (!result.credentialsMatch) throw "Wrong credentials!";
        return httpHelper.getPrivileges(result.userID); // Don't chain here, but return!
    }).then(function(result) {
        if (result.privilege <= 0) throw "You do not have sufficient privileges.";
    });
}

As you probably know, things look even simpler when you use async/await syntax:

async function login(credentials) {
    const result = await httpHelper.checkCredentials(credentials);
    if (!result.credentialsMatch) throw "Wrong credentials!";
    const result2 = await httpHelper.getPrivileges(result.userID);
    if (result2.privilege <= 0) throw "You do not have sufficient privileges.";
}
trincot
  • 317,000
  • 35
  • 244
  • 286
0

Just thinking of a hypothetical case which could be useful to anyone landin on this thread. Say if the getPrivileges() Promise does not resolve() to a value which could be validated in the second .then() , and instead returns a reject() with an error message, we could chain a catch() and check for the error message to determine the next action.

Lakshmi
  • 3
  • 2