0

I'm using the following two pieces of code :

    Store.addUser(newUserInfo).then(function(firstResult) {
        Store.getUserList().then(function(result){
                console.log('this side');
                console.log(result);
                io.sockets.emit('userAdded', {
                    userMapByUserId: result
                });
            }, function(error) {
                console.log('List of users could not be retrieved');
                console.log(error);
                io.sockets.emit('userAdded', {
                    userMapByUserId: []
                });
            }
        );
    }, function(rejection) {
        socket.emit('userNotAdded', {
            userId: -1,
            message: rejection.reason,
            infoWithBadInput: rejection.infoWithBadInput
        });
    });

and in Store :

var addUser = function(newUserInfo) {
    var validationResult = Common._validateUserInfo(newUserInfo);
    if (validationResult.isOK) {
        return keyValueExists('userName', newUserInfo.userName).then(function(userNameAlreadyExists) {
            if (userNameAlreadyExists) {
                validationResult = {
                    isOK: false,
                    reason: 'Username already exists',
                    infoWithBadInput: 'userName'
                };
                return Promise.reject(validationResult);
            } else {
                var newUserId = generateUserId();
                //TODO: change it somehting more flexible. e.g. a predefined list of attributes to iterate over
                var newUser = {
                    'userName': newUserInfo.userName,
                    'password': newUserInfo.password,
                    'userId': newUserId,
                    'lastModificationTime': Common.getCurrentFormanttedTime(),
                    'createdTime': Common.getCurrentFormanttedTime()
                };
                var user = new User(newUser);
                user.save(function(err) {
                    if (err) {
                        console.log(err);
                        console.log('There is a problem saving the user info');
                        return Promise.reject('There is a problem saving the user info');
                    } else {
                        console.log('A new user added: ');
                        console.log(newUser);
                        //return getUserList();
                        return Promise.accept(newUser);
                    }
                });
            }
        });
    } else {
        return Promise.reject(validationResult);
    }
};

But in the first code , when I do Store.addUser(newUserInfo) it always runs the first function (resolve function) which shouldn't be the case if we do return Promise.reject() in addUser. Any idea on why this happens ?

Arian
  • 7,397
  • 21
  • 89
  • 177
  • How do you know that the `return Promise.reject()` executed? – Bergi Aug 13 '15 at 09:01
  • Which of the 3 `return Promise.reject()`s in `addUser` do you mean? – Bergi Aug 13 '15 at 09:03
  • What is `Promise.accept`? – Bergi Aug 13 '15 at 09:03
  • 1
    if the code is getting as far as `user.save`, and that function is asynchronous then you'll always be `resolving` undefined, no rejection – Jaromanda X Aug 13 '15 at 09:13
  • I cut down your code to the bare minimum and don't see what you are seeing. What platform is this happening in ? browser? which one? nodejs? other? – Jaromanda X Aug 13 '15 at 09:22
  • @Bergi I changed `accept` to `resolve("HELLO")` BUT `firstResult` is `undefined`. any idea ? – Arian Aug 13 '15 at 09:58
  • @JaromandaX It's on Nodejs – Arian Aug 13 '15 at 09:59
  • @Bergi : and actually the recent question was out of curiosity BUT there's another problem. When `getUserList()` is called , the change which happened in `addUser` is not reflected ! (MongoDB is being used in these functions) although I can see that the entity was added to the database and if I call `getUserList` again, later I get the list of users. It's somehow like that `getUserList` doesn't see the latest changes by `addUser`. – Arian Aug 13 '15 at 10:01
  • @ArianHosseinzadeh: Which promise library are you using? – Bergi Aug 13 '15 at 10:19
  • @Bergi : native javascript. – Arian Aug 13 '15 at 15:21

1 Answers1

1

You've got two return statements too few, two too much, and are overlooking a non-promisified function call.

Store.addUser(newUserInfo).then(function(firstResult) {
    return Store.getUserList().then(function(result){
//  ^^^^^^
        …

This one is not really problematic, as you don't chain anything after the resulting promise, but it shouldn't be missed anyway.

…
return keyValueExists('userName', newUserInfo.userName).then(function(userNameAlreadyExists) {
    if (userNameAlreadyExists) {
        …
    } else {
        …
        var user = new User(newUser);
        user.save(function(err) { … });
//      ^^^^
    }
});

In this then-callback, you are not returning anything from your else branch. The promise is immediately fulfilled with undefined, and the ongoing save call is ignored - your promises don't know about it, so they can't await it. That's why Store.getUserList() that follows next in the chain doesn't see the changes; they're not yet stored.
It's also the reason why your Promise.reject inside that callback is ignored, and why Promise.accept never caused any problems.

You will need to create a new promise for the result of the save invocation here (so that you actually can return it):

…
var user = new User(newUser);
return new Promise(function(resolve, reject) {
    user.save(function(err) {
        if (err) {
            console.log(err);
            console.log('There is a problem saving the user info');
            reject('There is a problem saving the user info');
        } else {
            console.log('A new user added: ');
            console.log(newUser);
            resolve(newUser);
        }
    });
}); // .then(getUserList);
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Isn't this code following this anti pattern ? http://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it – Arian Aug 13 '15 at 15:50
  • 1
    @ArianHosseinzadeh: No, there's no promise involved within the `Promise` constructor, and we use it to promisify on the lowest possible level. Or does `user.save()` actually return a promise (or thenable)? Then the picture would be different, and you should use `return Promise.resolve(user.save()).then(…, …);` – Bergi Aug 13 '15 at 22:37