Colin
Colin

Reputation: 22595

How to retry failures with $q.all

I have some code that saves data using Breeze and reports progress over multiple saves that is working reasonably well. However, sometimes a save will timeout, and I'd like to retry it once automatically. (Currently the user is shown an error and has to retry manually) I am struggling to find an appropriate way to do this, but I am confused by promises, so I'd appreciate some help. Here is my code:

//I'm using Breeze, but because the save takes so long, I
//want to break the changes down into chunks and report progress
//as each chunk is saved....
var surveys = EntityQuery
    .from('PropertySurveys')
    .using(manager)
    .executeLocally();

var promises = [];
var fails = [];
var so = new SaveOptions({ allowConcurrentSaves: false});

var count = 0;

//...so I iterate through the surveys, creating a promise for each survey...
for (var i = 0, len = surveys.length; i < len; i++) {

    var query = EntityQuery.from('AnsweredQuestions')
            .where('PropertySurveyID', '==', surveys[i].ID)
            .expand('ActualAnswers');

    var graph = manager.getEntityGraph(query)
    var changes = graph.filter(function (entity) {
        return !entity.entityAspect.entityState.isUnchanged();
    });

    if (changes.length > 0) {
        promises.push(manager
            .saveChanges(changes, so)
            .then(function () {
                //reporting progress
                count++;                
                logger.info('Uploaded ' + count + ' of ' + promises.length);
            },
            function () {
                //could I retry the fail here?
                fails.push(changes);
            }
        ));
    }
}

//....then I use $q.all to execute the promises
return $q.all(promises).then(function () {
    if (fails.length > 0) {
        //could I retry the fails here?
        saveFail();
    }
    else {
        saveSuccess();
    }
});

Edit To clarify why I have been attempting this: I have an http interceptor that sets a timeout on all http requests. When a request times out, the timeout is adjusted upwards, the user is displayed an error message, telling them they can retry with a longer wait if they wish.

Sending all the changes in one http request is looking like it could take several minutes, so I decided to break the changes down into several http requests, reporting progress as each request succeeds.

Now, some requests in the batch might timeout and some might not.

Then I had the bright idea that I would set a low timeout for the http request to start with and automatically increase it. But the batch is sent asynchronously with the same timeout setting and the time is adjusted for each failure. That is no good.

To solve this I wanted to move the timeout adjustment after the batch completes, then also retry all requests.

To be honest I'm not so sure an automatic timeout adjustment and retry is such a great idea in the first place. And even if it was, it would probably be better in a situation where http requests were made one after another - which I've also been looking at: https://stackoverflow.com/a/25730751/150342

Upvotes: 0

Views: 717

Answers (3)

Roamer-1888
Roamer-1888

Reputation: 19288

Orchestrating retries downstream of $q.all() is possible but would be very messy indeed. It's far simpler to perform retries before aggregating the promises.

You could exploit closures and retry-counters but it's cleaner to build a catch chain :

function retry(fn, n) {
    /* 
     * Description: perform an arbitrary asynchronous function,
     *   and, on error, retry up to n times.
     * Returns: promise
     */
    var p = fn(); // first try
    for(var i=0; i<n; i++) {
        p = p.catch(function(error) {
            // possibly log error here to make it observable
            return fn(); // retry
        });
    }
    return p;
}

Now, amend your for loop :

  • use Function.prototype.bind() to define each save as a function with bound-in parameters.
  • pass that function to retry().
  • push the promise returned by retry().then(...) onto the promises array.
var query, graph, changes, saveFn;

for (var i = 0, len = surveys.length; i < len; i++) {
    query = ...; // as before
    graph = ...; // as before
    changes = ...; // as before
    if (changes.length > 0) {
        saveFn = manager.saveChanges.bind(manager, changes, so); // this is what needs to be tried/retried
        promises.push(retry(saveFn, 1).then(function() {
            // as before
        }, function () {
            // as before
        }));
    }
}

return $q.all(promises)... // as before 

EDIT

It's not clear why you might want to retry downsteam of $q.all(). If it's a matter of introducing some delay before retrying, the simplest way would be to do within the pattern above.

However, if retrying downstream of $q.all() is a firm requirement, here's a cleanish recursive solution that allows any number of retries, with minimal need for outer vars :

var surveys = //as before
var limit = 2;

function save(changes) {
    return manager.saveChanges(changes, so).then(function () {
        return true; // true signifies success
    }, function (error) {
        logger.error('Save Failed');
        return changes; // retry (subject to limit)
    });
}
function saveChanges(changes_array, tries) {
    tries = tries || 0;
    if(tries >= limit) {
        throw new Error('After ' + tries + ' tries, ' + changes_array.length + ' changes objects were still unsaved.');
    }
    if(changes_array.length > 0) {
        logger.info('Starting try number ' + (tries+1) + ' comprising ' + changes_array.length + ' changes objects');
        return $q.all(changes_array.map(save)).then(function(results) {
            var successes = results.filter(function() { return item === true; };
            var failures = results.filter(function() { return item !== true; }
            logger.info('Uploaded ' + successes.length + ' of ' + changes_array.length);
            return saveChanges(failures), tries + 1); // recursive call.
        });
    } else {
        return $q(); // return a resolved promise
    }
}

//using reduce to populate an array of changes 
//the second parameter passed to the reduce method is the initial value
//for memo - in this case an empty array
var changes_array = surveys.reduce(function (memo, survey) {
    //memo is the return value from the previous call to the function        
    var query = EntityQuery.from('AnsweredQuestions')
                .where('PropertySurveyID', '==', survey.ID)
                .expand('ActualAnswers');

    var graph = manager.getEntityGraph(query)

    var changes = graph.filter(function (entity) {
        return !entity.entityAspect.entityState.isUnchanged();
    });

    if (changes.length > 0) {
        memo.push(changes)
    }

    return memo;
}, []);

return saveChanges(changes_array).then(saveSuccess, saveFail);

Progress reporting is slightly different here. With a little more thought it could be made more like in your own answer.

Upvotes: 2

Colin
Colin

Reputation: 22595

Two useful answers here, but having worked through this I have concluded that immediate retries is not really going to work for me.

I want to wait for the first batch to complete, then if the failures are because of timeouts, increase the timeout allowance, before retrying failures. So I took Juan Stiza's example and modified it to do what I want. i.e. retry failures with $q.all

My code now looks like this:

    var surveys = //as before

    var successes = 0;
    var retries = 0;
    var failedChanges = [];

    //The saveChanges also keeps a track of retries, successes and fails
    //it resolves first time through, and rejects second time
    //it might be better written as two functions - a save and a retry
    function saveChanges(data) {
        if (data.retrying) {
            retries++;
            logger.info('Retrying ' + retries + ' of ' + failedChanges.length);
        }

        return manager
            .saveChanges(data.changes, so)
            .then(function () {
                successes++;
                logger.info('Uploaded ' + successes + ' of ' + promises.length);
            },
            function (error) {
                if (!data.retrying) {
                    //store the changes and resolve the promise
                    //so that saveChanges can be called again after the call to $q.all
                    failedChanges.push(data.changes);
                    return; //resolved
                }

                logger.error('Retry Failed');
                return $q.reject();
            });
    }

    //using map instead of a for loop to call saveChanges 
    //and store the returned promises in an array
    var promises = surveys.map(function (survey) {
        var changes = //as before
        return saveChanges({ changes: changes, retrying: false });
    });

    logger.info('Starting data upload');

    return $q.all(promises).then(function () {
        if (failedChanges.length > 0) {
            var retries = failedChanges.map(function (data) {
                return saveChanges({ changes: data, retrying: true });
            });
            return $q.all(retries).then(saveSuccess, saveFail);
        }
        else {
            saveSuccess();
        }
    });

Upvotes: 0

Juan Stiza
Juan Stiza

Reputation: 523

This is a very rough idea of how to solve it.

var promises = [];
var LIMIT = 3 // 3 tris per promise.

data.forEach(function(chunk) {
  promises.push(tryOrFail({
    data: chunk,
    retries: 0
  }));
});

function tryOrFail(data) {
  if (data.tries === LIMIT) return $q.reject();
  ++data.tries;
  return processChunk(data.chunk)
    .catch(function() {
      //Some error handling here
      ++data.tries;
      return tryOrFail(data);
    });
}

$q.all(promises) //...

Upvotes: 1

Related Questions