Asynchronous call inside a promise

Question

First of all I know this question has been asked many times, but I can't figure how to do this. I am new to Node.js and dealing with the asynchronous aspect of it.

Here is my code

passport.use(new LocalStrategy({ usernameField: 'email', passwordField: 'password' }, function (email, pass, done) {   
    Users.query(email).exec(function (err, item) {
        const message = 'Login Invalid';
        if (err) return done(err);

        var promise = GetHash({ item, pass });        
        promise.then((data) => {                        
            return done(null, data)
        }).catch((err) => {
            console.log(err);
            return done(null, false, { message });
        });                             
    });    
}));

function GetHash(userPasswordRequest) {
    return new Promise(function (resolve, reject) {
        if (userPasswordRequest.item && userPasswordRequest.item.Items[0]) {
            var userRecord = userPasswordRequest.item.Items[0].attrs;
            if (userRecord.old) {
                if (crypto.createHash('sha256').update(userPasswordRequest.pass, 'ucs-2').digest('base64') === userRecord.password) {
                    var newHash = bcrypt.hashSync(userPasswordRequest.pass, 10);
                    Users.update({ email: userRecord.email, password: newHash, old: null }, function (err, newItem) {
                        if (err) { console.log(err); } else {
                            console.log('first call');
                            resolve(newItem.attrs);
                        }
                    });                
                }
            }
            else {
                if (bcrypt.compareSync(userPasswordRequest.pass, userRecord.password)) {
                    console.log('first call');
                    resolve(userPasswordRequest.item.Items[0].attrs);                    
                }
            }
        }
        reject();
    })
}

The issue is situated here :

 if (crypto.createHash('sha256').update(userPasswordRequest.pass, 'ucs-2').digest('base64') === userRecord.password) {
                    var newHash = bcrypt.hashSync(userPasswordRequest.pass, 10);
                    Users.update({ email: userRecord.email, password: newHash, old: null }, function (err, newItem) {
                        if (err) { console.log(err); } else {
                            console.log('first call');
                            resolve(newItem.attrs);
                        }
                    });                
                }

Because the Users.update is asynchronous, I'm rejecting the promise before resolving it.

I tried many things, a promise inside of a promise and many other stuff but I can't make this work properly.

Any help would be greatly appreciated

You wouldn't have everything inside one huge promise but split it out into a promise chain. So first get the user, then bcrypt it as two separate actions. If any part of the chain fails it aborts the chain and allows you to catch errors in one place throughout the chain. — ste2425, Mar 30 '17 at 18:33
Sounds like you simply are missing an `else` in front of the `reject()` call? — Bergi, Mar 30 '17 at 18:44
@Theo Btw, [dynamodb appears to already return promises](http://stackoverflow.com/questions/26475486/how-do-i-promisify-the-aws-javascript-sdk) - so you don't need any `new Promise` at all — Bergi, Mar 30 '17 at 18:59
@Bergi I know but unfortunately I'm using another library call dynogels (there is a promise version of it but no documentation except for query and scan and I was not able to use .then on update) — Theo, Mar 30 '17 at 19:02

score 1 · Answer 1 · answered Mar 30 '17 at 18:37

1

You shouldn't use reject as a fall-through, but call is explicitly when an error condition occurs.

So like this:

if (userPasswordRequest.item && userPasswordRequest.item.Items[0]) {
  ...
} else {
  return reject(Error('invalid userPasswordRequest'));
}

And also:

if (bcrypt.compareSync(userPasswordRequest.pass, userRecord.password)) {
  console.log('first call');
  return resolve(userPasswordRequest.item.Items[0].attrs);                    
} else { 
  return reject(Error('invalid password'));
}

This isn't right either:

if (err) { console.log(err); }

Don't just log the error, reject the promise with it:

if (err) {
  console.log(err);
  return reject(err);
}

As you can see, it's also good form to return after resolving or rejecting.

answered Mar 30 '17 at 18:37

robertklep

198,204
35
394
381

Okay, I know that I can use else everywhere, but I was thinking that nicer solution exists – Theo Mar 30 '17 at 18:40
1

@Theo The nicer solution would be to use promises properly and promisify each asynchronous function separately :-) – Bergi Mar 30 '17 at 18:45
@Bergi Yes! This is what I tried. Putting promise to other functions (especialy the call to the db) but I was not able to make it work – Theo Mar 30 '17 at 18:51
1

@Theo No, I meant your own functions should never call the `Promise` constructor. You should only use it to wrap `Users.query(…).exec`, `Users.update`, and the crypto functions if you used their asynchronous variants. Build a facade around the ugly callbacks, then use pure promises with `then` only. – Bergi Mar 30 '17 at 18:57

Asynchronous call inside a promise

1 Answers1