Angular deferred implementation only outputs last value of loop

Question

I have a custom synchronization process where I queue up, in order, all of my sync records. When my service retrieves more than 1 sync record, it will process them, then update my last sync date for every successful record, or log my error when it fails (without updating the last sync date) and abort the sync process.

I've implemented the $q.all from AngularJS. Here's a subset of the sync loop:

    var processes = [];

    for (var i in data) {
        if (data[i] === null || data[i].TableName == null || data[i].Query == null || data[i].Params == null) {
            // Let's throw an error here...
            throw new TypeError("ERROR! The data retrieved from the download sync process was of an unexpected type.");
        }

        var params = data[i].Params;
        var paramsMassaged = params.replaceAll("[", "").replaceAll("]", "").replaceAll(", ", ",").replaceAll("'", "");
        var paramsArray = paramsMassaged.split(",");

        mlog.Log("Query: " + data[i].Query);
        mlog.Log("Params: " + paramsArray);

        if (data[i].TableName === "table1") {
            var process = $table1_DBContext.ExecuteSyncItem(data[i].Query, paramsArray);

            process.then(
                function () {
                    $DBConfigurations_DBContext.UpdateLastSyncDate(data[i].CreatedDate, function (response) {
                        mlog.Log(response);
                    });
                },
                function (response) {
                    mlog.LogSync("Error syncing record: " + response, "ERROR", data[i].Id);
                },
                null
            );

            processes.push(process);
        } else if (data[i].TableName === "table2") {
            var process = $table2_DBContext.ExecuteSyncItem(data[i].Query, paramsArray);

            process.then(
                function () {
                    $DBConfigurations_DBContext.UpdateLastSyncDate(data[i].CreatedDate, function (response) {
                        mlog.Log(response);
                    });
                },
                function (response) {
                    mlog.LogSync("Error syncing record: " + response, "ERROR", data[i].Id);
                },
                null
            );

            processes.push(process);
        } else {
            mlog.LogSync("WARNING! This table is not included in the sync process. You have an outdated version of the application. Table: " + data[i].TableName);
        }
    }

    $q.all(processes)
        .then(function (result) {
            mlog.LogSync("---Finished syncing all records");
        }, function (response) {
            mlog.LogSync("Sync Failure - " + response, "ERROR");
        });

Example ExecuteSyncItem function:

ExecuteSyncItem: function (script, params) {
    window.logger.logIt("In the table1 ExecuteSyncItem function...");

    var primaryKey = params[params.length - 1];

    var deferred = $q.defer();

    $DBService.ExecuteQuery(script, params,
        function (insertId, rowsAffected, rows) {
            window.logger.logIt("rowsAffected: " + rowsAffected.rowsAffected);

            if (rowsAffected.rowsAffected <= 1) {
                deferred.resolve();
            } else {
                deferred.resolve(errorMessage);
            }
        },
        function (tx, error) {
            deferred.reject("Failed to sync table1 record with primary key: " + primaryKey + "; Error: " + error.message);
        }
    );

    return deferred.promise;
}

The problem I'm running into is, if there are more than 1 sync records that fail, then this line displays the same value for all records that failed (not sure if it's the first failure record, or the last).

mlog.LogSync("Error syncing record: " + response, "ERROR", data[i].Id);

How do I get it to display the information for the specific record that failed, instead of the same message "x" times?

I know this is outside the scope of your question. But you are doubling up on the code for $table1_DBContext && $table2_DBContext and the only diff is the DB reference. Why not config it, and then just reference that so you only have to code that block once? id..var config = { 'table1': $table1_DBContentext, etc..} then in your code: var process = config[ data[i].TableName ].ExecuteSyncItem .. of course, test for that in your 'if'. — james emanon, Jun 27 '14 at 19:44
@jamesemanon you are definitely correct. It's a refactor I'm planning on doing in the very near future. — ganders, Jun 30 '14 at 13:27

ryeballar · Accepted Answer · 2014-07-01T05:13:27.257

As mentioned by comradburk wrapping your processes in a closure within a loop is a good solution, but there is an angular way in solving this problem. Instead of using the native for-in loop, you can do it via angular.forEach() and loop through all the data elements.

var processes = [];

angular.forEach(data, function(item) {
    if (item === null || item.TableName == null || item.Query == null || item.Params == null) {
        // Let's throw an error here...
        throw new TypeError("ERROR! The data retrieved from the download sync process was of an unexpected type.");
    }

    var params = item.Params;
    var paramsMassaged = params.replaceAll("[", "").replaceAll("]", "").replaceAll(", ", ",").replaceAll("'", "");
    var paramsArray = paramsMassaged.split(",");

    mlog.Log("Query: " + item.Query);
    mlog.Log("Params: " + paramsArray);

    if (item.TableName === "table1") {
        var process = $table1_DBContext.ExecuteSyncItem(item.Query, paramsArray);

        process.then(
            function () {
                $DBConfigurations_DBContext.UpdateLastSyncDate(item.CreatedDate, function (response) {
                    mlog.Log(response);
                });
            },
            function (response) {
                mlog.LogSync("Error syncing record: " + response, "ERROR", item.Id);
            },
            null
        );

        processes.push(process);
    } else if (item.TableName === "table2") {
        var process = $table2_DBContext.ExecuteSyncItem(item.Query, paramsArray);

        process.then(
            function () {
                $DBConfigurations_DBContext.UpdateLastSyncDate(item.CreatedDate, function (response) {
                    mlog.Log(response);
                });
            },
            function (response) {
                mlog.LogSync("Error syncing record: " + response, "ERROR", item.Id);
            },
            null
        );

        processes.push(process);
    } else {
        mlog.LogSync("WARNING! This table is not included in the sync process. You have an outdated version of the application. Table: " + item.TableName);
    }
});

$q.all(processes)
    .then(function (result) {
        mlog.LogSync("---Finished syncing all records");
    }, function (response) {
        mlog.LogSync("Sync Failure - " + response, "ERROR");
    });

score 0 · Answer 2 · edited May 23 '17 at 12:05

The problem is due the closure you have on i. When the callback function executes, the value of i will be the last value in the for loop. You need to bind that value i to a separate, unchanging value. The easiest way to do that is with a self invoking function.

for (var i in data) {
    (function(item) {
        // Put your logic in here and use item instead of i, for example
        mlog.LogSync("Error syncing record: " + response, "ERROR", data[item].Id
    })(i);
}

Here's a good read for why closures cause this (it's a pretty common problem): Javascript infamous Loop issue?

Angular deferred implementation only outputs last value of loop

2 Answers2