Accessing outer variables within a Promise chain

Question

I wanted to know if I am doing anything horrible with this example code:

something.on('register', function(user) {
    client.setAsync(config.id, user.id) // From a package (I can't set the return value)
    .then(function(){
        return user; // is this OK?
     })
    .then(handleNewUser)
    .then(getSomeStuff)
    .catch(function(err) {
        console.error("Promise chain error: ", err);
    });
});

Will there be any scope issues? (race conditions where user isn't what I think it is?)

Can I pass user in a more elegant way?

`client.setAsync(config.id, user.id).return(user)` - bluebird. — Benjamin Gruenbaum, Dec 14 '15 at 14:58
You could also do `.then(() => handleNewUser(user))` or `.then(handleNewUser.bind(null, user))` making `null` whatever context `handleNewUser` needs to operate correctly. As you are using Bluebird though, Benjamin's comment is the best approach. — sdgluck, Dec 14 '15 at 15:13

score 1 · Answer 1 · edited May 23 '17 at 11:52

If anything, your pattern better guarantees the same user than if client.setAsync() was to return a user. If the package was badly written or badly documented, you could receive back some other object entirely.

One proviso though - your user object cannot be augmented by the intermediate client.setAsync() process. There is no mechanism to do so unless user itself was passed as a parameter to client.setAsync(). It can only be assumed that that is not an issue in this case.

Working with what you have, the only obvious improvement would be to form your chain with one less .then() as follows :

something.on('register', function(user) {
    client.setAsync(config.id, user.id) // From a package (I can't set the return value)
    .then(function() {
        return handleNewUser(user);
    })
    .then(getSomeStuff)
    .catch(function(err) {
        console.error("Promise chain error: ", err);
    });
});

You might enjoy reading through this question and its answers. In particular, find the answer entitled "Nesting (and) closures" and you'll see that accessing previous values in a closure is entirely reasonable and proper.

score 0 · Answer 2 · answered Dec 14 '15 at 13:01

It's a little odd;

There is no advantage to making the promise library call the handleNewUser function itself given you have to add the extra then function to return user to make it work.

I'd recommend simply calling the function yourself in a then function:

something.on('register', function(user) {

    client.setAsync(config.id, user.id)
    .then(function(){ return handleNewUser(user); }
    .then(getSomeStuff)
    .catch(function(err) {
      console.error("Promise chain error: ", err);
    });

});

[Note: I haven't tested this version] Or, you may be able to utilise a partially applied function to make this even cleaner:

something.on('register', function(user) {

    client.setAsync(config.id, user.id)
    .then(handleNewUser.bind(this, user); }
    .then(getSomeStuff)
    .catch(function(err) {
      console.error("Promise chain error: ", err);
    });

});

Accessing outer variables within a Promise chain

2 Answers2