More problems with promises

Question

In my previous question I thought I'd got it sorted, but I've found an intermittent edge condition where the "then" part is being carried out before all the promises resolve in the Q.all call.

Simply, I have the following set up where a generic calculation is called multiple times with different parameters: (code simplified a bit to remove irrelevant code and actual parameter names)

    var promiseArray = [];
    promiseArray.push(genericCalc(param1, param2, param3));
    promiseArray.push(genericCalc(param1, param2, param3));
    promiseArray.push(genericCalc(param1, param2, param3));

    var calcPromise = Q.all(promiseArray);

    return calcPromise
            .then(calcsDone)
            .fail(calcsFailed);

    function calcsDone(res) {
        calcTotals(); 
        setChart(selectedRow());
        console.log("done recalculation routine");
    }

    function calcsFailed() {
        logger.logError("Failure in Calculations", "", null, true);
    }

genericCalc (with some irrelevant code removed)

var genericCalc = function (param1, param2, param3) {

    //specific calculation is determined using parameters passed in and varies but is same structure for each as below
    calcPromise = specificCalc(params...);

    return calcPromise
       .then(calcsDone)
       .fail(calcsFailed);

    function calcsDone(res) {  
      //some minor subtotalling in here using "res" and flag setting             
      return Q.resolve();
    }

    function calcsFailed() {
       //handle error....
    }
};

There are 3 or 4 different specific calculations and they all have roughly the same sort of layout:

function specificCalc1(params...) {

    var promise = calcContext.ajaxCalc(params);

    return promise
            .then(calcsDone)
            .fail(calcsFailed);

    function calcsDone(res) {
        rv(res);
        console.log("done specific calc 1");
        return Q.resolve();
    }
    function calcsFailed() {
        logger.logError("Failure in specific calc 1", "", null, true);
        return Q.resolve();
    }
};

Now I know that this is not a great idea to push the result of the ajax calc into a return value, but at present I don't want to change this as there is just too much code change involved and I feel that, even if not the best methodology at present, this is a learning curve for me and I'll address that part once I have my head around this strange edge condition.

So... what throws me is that every now and again when I change some values on screen which trigger a recalculation, one of the "done" console log messages from one of the specific calcs appears AFTER the "done recalculation routine" one from the first section!

I thought it was being caused by a poorly-constructed promise leading to the function being executed immediately, but the REALLY weird thing is that when I put a debug stop in the server code for that calculation, all works correctly and the client browser is paused until the server code is continued and then the client side debugs show that the next points are being hit.

9 times out of 10 it all works perfectly. On the 10th one, I see "done recalculation routine" in the console immediately followed by "done specific calc 2" and this causes the totals to be wrong.

I cannot see how this can be happening. I've artificially put time delay loops in all the specific calc functions to make them take several seconds and never once do I see the out of sequence debug messages, but when there is no artificial slowdown in place, I see it 1 in 10 times approximately.

Please would someone try and put me out of my misery here... I just want to know I can rely on the "Q.all" call working in the first block of code, such that when it hits "calcsDone" it has ALWAYS done the generic and specific calcs!

Edit 1: To explain the "rv" or returnvalue a bit. In "genericCalc", I declare an observable:

    var res = ko.observable(); //holds returned results

And as part of the call to "specificCalc" I pass that return value in, for example:

calcPromise = specificCalc1(isPackaging, res, thisArticle);

In the specificCalc I just put the ajax result into the observable, so it is available in "calcsDone" in "genericCalc" and, once all calcs are completed from the "Q.all()" function, the calculation routines can tot up the individual results from each specific Calc.

Edit 2 The console log, when it goes wrong is:

done specificCalc1 
done specificCalc2, isPackaging=true 
Done specificCalc3, redraw = false 
done calctotals 
chart replotted 
done recalculation routine
done specificCalc2, isPackaging=false  

...you can see the "done specificCalc2" after "done recalculation routine" there.

Edit 3

I reduced the promiseArray to just one item to check, using the parameters for what seems to be the troublesome call (specificCalc2):

var promiseArray = [];
promiseArray.push(genericCalc(param1, param2, param3));

And then I put a stop point in the server code.

The result is that the stop in the server code happens and the console already says "done" so it's a problem with the promise construction after all and was somehow being masked by one of the other calls being done. Once I release the stop in server code, I get my console messages from the ajax call function AND the generic Calc function saying "done".

So, it appears to me as if the problem is in the main Q.all call from what I can tell, as that doesn't wait for the generic calc function being completed before it carries on with its "calcsDone" function.

I just tried returning the genericCalc promise:

return genericCalc("eol_", true, false, false)
//return calcPromise
        .then(calcsDone)
        .fail(calcsFailed);

...and it instantly fails with "cannot call method "then" of undefined, thus confirming my problem is in the generic calc function, so off to look at that now.

Edit 4

Narrowed the problem down to a further function call within genericCalc. As part of the calculation, this calls a function to remove the impact value as stands before the calculation is done. When the calculation returns it then adds the result back in to the overall amount.

If I "return Q.resolve()" from genericCalc on the line before I do:

impactRemove(currentPrefix, currentArticle); //remove impacts for this prefix

then the promise works, if I do it on the line after, it fails. So for some reason calling that function seems to resolve the promise. Investigating further now.

Edit 5 So the problem is caused when I call a standard function midway through the genericCalc routine. The logic behind this is:

1. Change on browser form retriggers calculation
2. Function is called that sets up array of promises (several of which call the same function with different parameters)
3. Inside that generic function (genericCalc) I call a standard non-promise function that removes the current calculation totals from the project total
4. Calculation is complete
5. Standard non-promise function called to add results of calculation back to project total
6. GenericCalc returns promise to main function
7. Overall totals updated with latest project totals, graphics are updated

What actually seems to happen is that when I call the standard javascript functions with genericCalc, they execute immediately, therefore resolving the promise and although the ajax call is still done, the Q.all call does not wait as it believes the promise is resolved as "genericCalc" is returning "undefined" and not a promise.

At this point, Bergi is screaming at me about my hideous anti-pattern noob coding and I tend to agree with him. Trouble is, I'd like to get it working this way so I have something to test against when I finally adapt it to work correctly.

So... if I have two functions called from within "genericCalc" like so:

impactRemove(currentPrefix, currentArticle); //remove impacts for this prefix

and

impactAdd(currentPrefix, currentArticle); //addimpacts for this prefix

Which are basically like this:

function impactAdd(prefix, prjArt) {
    if (!prjArt) {return} //just get out as there's nothing to calculate
    factorArray.forEach(function (f) {
        var orig_projValue = pGlobalVar.editProject()[prefix + f]();
        var new_projArtValue = prjArt[prefix + f](); //must be set first!
        pGlobalVar.editProject()[prefix + f](orig_projValue + new_projArtValue); //add new value back in to project value
    });
}

...then how do I call these "midpoint" functions within the promise of genericCalc so that I Can still return a promise when a) impactRemove has been completed, b) the remote ajax call has been completed and c) the impactAdd function has been completed?

Do I need to set up code within genericCalc to do something like:

impactRemove(params...).then(<ajax function that returns new calculation results>).then(impactAdd(params))

...or will (params) after my functions automatically invoke those functions, thus resolving the promise too early? This is all so confusing compared to what I'm used to!

Edit6 All genericCalc does is this:

var genericCalc = function (param1, param2, param3) {

    //specific calculation is determined using parameters passed in and varies but is same structure for each as below
    calcPromise = specificCalc(params...);

    impactRemove(currentPrefix, currentArticle); //remove impacts for this prefix

    return calcPromise
       .then(calcsDone)
       .fail(calcsFailed);

    function calcsDone(res) {  
      //some minor subtotalling in here using "res" and flag setting    

      impactAdd(currentPrefix, currentArticle); //addimpacts for this prefix
      return Q.resolve();
    }

    function calcsFailed() {
       //handle error....
    }
};

"specificCalc" returns a promise - that one works as I've checked the contents of the promise at a breakpoint. If I remove the calls to "impactRemove" and "impactAdd" above, then "genericCalc" also works. It is those calls that cause the problem.

This is why I think I need something like:

impactRemove(params)
    .then(return calcPromise(params)
    .then(impactAdd(params);

...but neither impactAdd nor impactRemove do anything asynchronously and I'm also not sure how I can set this up as I need to use params and yet you said those params will mean the functions are immediately invoked...

Edit 7

So, as mentioned in the lengthy comments section, this is being caused by a "forEach" loop in genericCalc:

var genericCalc = function (calcPrefix, other params...) {
    gcCount++;
    console.log("generic calc starting for: " + calcPrefix + ", count=" + gcCount);
    pGlobalVar.projectIsCalculating(true); //controls spinner gif

    var res_Array = ko.observable(); //holds returned results
    var _prjArticleArray = []; //create local array to use

    if (currentArticle == true) {
        _prjArticleArray.push(pGlobalVar.editProjectArticle());
    } else {
        projectResults().projectArticles().forEach(function (pa) {
            _prjArticleArray.push(pa);
        });
    };

    _prjArticleArray.forEach(function (thisArticle) {

        var calcPromise;

        switch (calcPrefix) {
            case "eol_":
                calcPromise = Q.all([calcEOL(isPackaging, res_Array, thisArticle)]);
                break;
            case "use_":
                calcPromise = Q.all([calcUse(isPackaging, res_Array, thisArticle)]);
                break;
            case "air_":
                calcPromise = Q.all([calcFlight(isPackaging, res_Array, thisArticle)]);
                break;
            default:
                break;
        }

        impactRemove(calcPrefix, thisArticle); //remove impacts for this prefix

        return calcPromise
            .then(calcsDone)
            .fail(calcsFailed);

        function calcsDone(args) {

            //do some calcs and totalling based on returned results

            impactAdd(calcPrefix, thisArticle); //add this article impact into project total

            console.log("generic calc done for: " + calcPrefix + ", count=" + gcCount);

            calcTotals(); //accmulates individual results into the "total_xxx" used on the results table and in the chart
            setChart(selectedRow());

            pGlobalVar.projectIsCalculating(false);

        }
        function calcsFailed() {
            logger.logError("Failure in " + calcPrefix + "calculation", "", null, true);
            impactAdd(calcPrefix); //will add back in results as they were at start of calc
            pGlobalVar.projectIsCalculating(false);
        }

    });
};

The only reason I've posted it in all its ugly glory is to point out that all works perfectly IF I remove the "forEach" loop and just run this for one article. Why on earth would the forEach loop make it die a horrible death ?

Answer 1

I think you just want to exchange the order of the specificCalc() and impactRemove() calls. Even if the first is asynchronous, it will start doing its task right now - only the results will arrive in the next turn. If you want to chain anything after a synchronous task, just put it on the next line ("semicolon monad").

Also, if impactRemove does assign to your global (!) variable calcPromise, it might not be a promise any more and throw when calling a .then() method on it. What you want seems to be

function genericCalc(param1, param2, param3) {
    impactRemove(currentPrefix, currentArticle); //remove impacts for this prefix
    return specificCalc(params...).finally(function() {
        impactAdd(currentPrefix, currentArticle); // add impacts for this prefix
    }).then(function calcsDone(res) {  
        // some minor subtotalling in here using "res" and flag setting
        return subtotal;
    }, function calcsFailed() {
        // handle error....
    });
}

---

Why on earth would the forEach loop make it die a horrible death?

Because you're not returning a promise. A forEach loop has its own callback, from which you can return, but from the genericCalc function nothing is returned. Q.all will not fret about that, but just take the undefined as a value. However, the async action is to be started and you get your callbacks, but Q.all won't wait for it, because it does not know of it.

The solution is a quite simple change and has already been explained here.