0

I have some data in a array.

let data = ['firstData', 'secondData'];

The first object (firstData) always exists, secondData could be null. So I always use that to call my async function like:

myAsyncFunction(data[0]).then(result => {
    //do something here
});

If data[1] exists, I wanna call myAsyncFunction with data[1].

My current solution is with nested promises. Like this:

myAsyncFunction(data[0]).then(result => {
    return result;
}).then(result => {
    if(data[1] !== null){
        myAsyncFunction(data[1].then(resultTwo => {
        //dostuff with data
      });
    }
});

I don't really like this solution, but it does work. It must be a better way tho. result is always a object. I tried Promise.all but it does not work. I think it's a problem with "race condition" of some sort (I am on thin ice here).

Is my current solution the only one?

Here is what myAsuncFunction looks like:

myAsyncFunction(personData){
    return new Promise<any> ((resolve, reject) => {

        //Assingen private variables

    secondAsyncFunction(somePersonData).then(result =>
     {

                    //Do some sync stuff

          return someNewData; //not a promise
     })
         .then(thirdAsyncFunction)
         .then(data => {
              resolve(data);
         })
         .catch(error => {
          reject(error);
     });
});
  • So I have a few questions... your if statement is checking that it's not equal to null... does that mean that your array might be data = ["firstData", null]? Also, you may want to try to do something with Promise.map. http://bluebirdjs.com/docs/api/promise.map.html – jas7457 Apr 02 '17 at 21:48
  • Hi! Correct, data[1] could be null sometimes. I'm not using blubird, I'm using ES6 promises. I think there is a difference (?). – mintermasher Apr 02 '17 at 21:51
  • If you're simply looking to figure out when they're all done, and they might be null, you could create a map from the data array like so.... let data_promises = data.map(function(data){ return data ? myAsyncFunction(data) : Promise.resolve(); }); Then call Promise.all(data_promises);. This pretty much checks if the value exists (null is of course falsey) and will create a Promise that resolves immediately if it is null – jas7457 Apr 02 '17 at 21:58
  • If you could share the error message or code for when you tried `Promise.all`, it would be appreciated. If there's a race condition, does that mean that the first promise must be finished before the second promise begins? – 4castle Apr 02 '17 at 22:11
  • I don't get any error message. But I get two objects back in Promise.all. The problem is both objects are based on data from the second object (data[1]). – mintermasher Apr 02 '17 at 22:16
  • @mintermasher It sounds like `myAsyncFunction` is buggy. In theory, each invocation should be completely independent. – 4castle Apr 02 '17 at 22:18
  • @4castle, in _ALL_ cases? – mintermasher Apr 02 '17 at 22:32
  • Regarding `myAsyncFunction`, avoid the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Apr 02 '17 at 22:50
  • @mintermasher The code you have looks correct in structure. By "buggy", I mean that your function is stateful, when it shouldn't be. You've got some kind of closure or global variable which is making the function stateful, and the results of the last invocation is bleeding into the results of the first invocation. – 4castle Apr 02 '17 at 23:44

3 Answers3

1

Promise.all is designed to be used when you have a dynamic number of promises in an array, and you want to combine them into a promise which resolves when all of the promises in the array have resolved.

According to your use case, you should be able to use it like this:

data = data.filter(x => x !== null);

Promise.all(data.map(myAsyncFunction)).then((results) => {
    const [resultOne, resultTwo] = results;
    if (resultTwo) {
        // handle resultTwo
    }
    // handle resultOne
});
4castle
  • 32,613
  • 11
  • 69
  • 106
  • When I use Promise.all I only get result based on the second value. As I said, I think it has something to do with race condition. – mintermasher Apr 02 '17 at 22:11
  • In that case, you may have a promise leak (*ie. your async function resolves/rejects itself before it's actual execution is completed*) – AP. Apr 02 '17 at 22:12
  • Oh... That could explain it. What is the best way to find out if there is any promise leaks? – mintermasher Apr 02 '17 at 22:14
  • @4castle, you may want to do `data.filter(i=>i).map(myAsyncFunction)`. Since `myAsyncFunction` should **not** execute when `data[x]` is `null` – AP. Apr 02 '17 at 22:15
  • @mintermasher Only way I know is reading the code, adding `console.logs` at different parts of the promise chain (and see which print statements execute in the wrong order!) – AP. Apr 02 '17 at 22:17
  • @AP, I just wanna be clear - is there 100% a promise leak, or could Promise.all not work in some cases? – mintermasher Apr 02 '17 at 22:29
  • 1
    Without seeing the function body I cannot say for certain, but NO `Promise.all` will always execute **all** the promises inside, and ***will return*** their resolved callbacks. UNLESS any of them `reject` or `throw` an exception, in which case the `Promise.all` will reject on the first failure, and none of the successes (`pending` or `completed`) will be reported – AP. Apr 02 '17 at 22:35
  • @mintermasher I've edited my answer above/below with a code block, which I believe should not have any leaks. – AP. Apr 02 '17 at 22:58
  • @4castle, this would probably work. But when I call myAsyncFunction like that, "this" is not defined. – mintermasher Apr 02 '17 at 23:50
  • @mintermasher If `myAsyncFunction` is a method of an object, then surround it with an arrow function like `x => obj.myAsycFunction(x)`. You could also use `obj.myAsyncFunction.bind(obj)`. – 4castle Apr 02 '17 at 23:53
0

Given that data is known statically before the promise result, you don't need to put the condition inside the then callback - you can make the whole chaining conditional:

var promise = myAsyncFunction(data[0]);
if(data[1] !== null){
    promise.then(result =>
        myAsyncFunction(data[1])
    ).then(resultTwo => {
        //dostuff with data
    });
}

In general, notice that nesting is not a bad thing, it is required when you want to branch control flow. Just don't forget to always return your promises from all (callback) functions.

Bergi
  • 630,263
  • 148
  • 957
  • 1,375
0

You could just simply do:

const stuffOnlyRelatedToSecondData = result2 => /*do something*/

myAsyncFunction(data[0]).then(result => {
    if (data[1] === null) {return null}
    return myAsyncFunction(data[1]).then(stuffOnlyRelatedToSecondData) 
}).then(/*Anything to do after [1] or [1,2]*/)

This way you can use the same chain and the two possible cases will look like:

myAsyncFunction(data[0]) --> END
myAsyncFunction(data[0]) --> myAsyncFunction(data[1]) --> END

Edit:

It seems that myAsyncFunction may have a promise leak, try using something like this instead:

const myAsyncFunction = personData => {
    //Assigning private variables
    return secondAsyncFunction(somePersonData).then(result => {
        //Do some sync stuff
        return someNewData; //not a promise
    })
    .then(thirdAsyncFunction)
}

We can write it this way because secondAsyncFunction kicks off a new Promise, hence we don't need another Promise wrapper around it. And the catch can be put in the parent Promise chain itself which executes myAsyncFunction

AP.
  • 8,082
  • 2
  • 24
  • 33
  • I tried the code within the edit. I understand way wrapping it again is unnecessary. But the problem is that secondAsyncFunction is called with the same value twice. Even tho before returning secondAsyncFunction all values are correct. – mintermasher Apr 02 '17 at 23:09
  • Hm. I create a variable (lets call it y) before returning secondAsyncFunction. If I console.log(y) (inside secondAsyncFunction) I get the correct value of the property .prop (for example). But when I console.log(y.prop) - I get the wrong value? – mintermasher Apr 02 '17 at 23:18
  • @mintermasher It sounds like you need to do a bit more debugging, until you can narrow down the problem to a [mcve], with actual code. – 4castle Apr 02 '17 at 23:41