0
    while (repoUrlLink != null && count < 90) {
        var xmlHttp = new XMLHttpRequest();
        xmlHttp.open('GET', repoUrlLink, false);
        xmlHttp.setRequestHeader('Authorization', 'Bearer ' + userData.accessToken);
        xmlHttp.onload = function () {
            if (xmlHttp.status != 200) {
                displayNoAccessMessage();
                break;
            }
            var result = JSON.parse(xmlHttp.responseText);
            if (result.length == 0) {
                displayNoRecordsMessage(); 
                break;                
            }
            var header = xmlHttp.getResponseHeader('link');
            if (header) {
                //doing something
            }
            else
                repoUrlLink = null;

            $.each(result, function (index, eachData) {
                // doing something with data
                count++;
            });
        }
        xmlHttp.send();
    }

Is there any better way to come out of the loop as soon as i display error.The break statement is not working. Is there any callback which can be useful ?

Ashish
  • 279
  • 1
  • 2
  • 11
  • 2
    It's not possible. The callback will never run before the loop has already finished. You cannot use a loop, use a recursive approach. – Bergi Jul 28 '17 at 22:54
  • @Bergi Could you please provide me the modified example ?Also just wanted to confirm if this approach is good or is there any better approach to do the same thing ? – Ashish Jul 28 '17 at 23:11
  • Putting synchronous `break` statements for the `while` loop inside asynchronous callback function code doesn't work. The JavaScript compiler error message on the console complains about it. See https://stackoverflow.com/q/14220321/5217142 for in-depth discussion on how to process asynchronous results. I suggest creating a function that returns a promise for one request operation, which is rejected on error and used to implement asynchronous recursion for retries. – traktor Jul 28 '17 at 23:20
  • It would help to separate operations of getting data, identifying and responding to errors, and processing data. Currently they are all mixed in together and cause the code to fail. – traktor Jul 28 '17 at 23:38
  • @Traktor53 could you please provide me schema/layout of the code ? that would be really helpful. – Ashish Jul 29 '17 at 00:23

2 Answers2

0

As someone said in the comments, you'll want to use recursion. This is because xmlHttp is an asynchronous operation. When you call send, the browser will send off the request, then continue on with the code, so by the time the onload function is called, it's no longer valid to break out of the loop. Also, being a function, onload is in no position to call break, since the while loop isn't even in the same scope.

var count = 0;
var doXmlHttpCall = function() {

    var xmlHttp = new XMLHttpRequest();
    // additional setup code here
    xmlHttp.onload = function() {
        var errorFound = false;

        if (/* some error case */) {
            errorFound = true;
        }

        // process request

        if (count < 90 && !errorFound) {
            doXmlHttpCall();
        }
    }
}

doXmlHttpCall();
Dr_Derp
  • 1,185
  • 6
  • 17
0

Some ideas to refactor the code using promises.

  • a promisified XMLHttpRequest request function getRepLink that performs one request. It rejects for request errors and HTTP errors (not "200" status).

  • a promisifed getSetOfRecords function to get a single response, parse it as JSON data and extract the link header value. It rejects iff there are no records.

  • a promisified process records function which tries to process a given number of records in sets. It fulfills with the number of records processed. It ignores the no records error if some records have already been processed.

 // XMLHttp request

function getRepoLink (repUrlLink) {
    return new Promise( function (resolve, reject) {
        var xmlHttp = new XMLHttpRequest();
        xmlHttp.open('GET', repoUrlLink, false);
        xmlHttp.setRequestHeader('Authorization', 'Bearer ' + userData.accessToken);
        xmlHttp.onload = function () {
            if( xmlHttp.status == 200) {
                resolve( xmlHttp);
            }
            else {
                reject( new Error("HTTP error " + xmlHttp.status));
            }
        };
        xmlHttp.onerror = reject;
        xmlHttp.send();
    });
}


// get a set of records

const EndOfRecords = new Error( "End of Records");

function getSetOfRecords( repUrlLink) {
    return getRepoLink( repUrlLink).then(
        function( xmlHttp) {
            var result = JSON.parse(xmlHttp.responseText);
            if (result.length == 0) {
                 displayNoRecordsMessage();
                 throw EndOfRecords;  // reject the returned promise
            }
            var header = xmlHttp.getResponseHeader('link');
            return {result, header};  // fulfill the returned promise
        }
    );
}

// get up to `count` records and process them

 function processRecords( repUrlLink, count) {
    var processed = 0;
    function processSomeMore() {
        return getSetOfRecords().then( function ({result, header}) {
            $.each(result, function (index, eachData) {
              // do something with data
              processed++;
            });
            if( header) {
                 //do something with header
                 if( processed < count)
                     return processSomeMore() // asynchronous "recursion"
            }
            else {
                // do as required if no link header present.
            }
            return processed;  // fulfill returned promise
        },
        function( error) {
            if( error === EndOfRecords && processed > 0) {
                return processed; // got some records
            };
            throw error;  // reject returned promise
        });
    }
    return processSomeMore();
}

//  Start asynchronous operation

processRecords( repUrlLink, 90)
.then( processed => console.log( processed + " records processed"))
.catch( error => console.log( error));
traktor
  • 17,588
  • 4
  • 32
  • 53
  • Someone suggested me to use jquery ajax calls instead of xmlhttp as a cleaner way. What are your thoughts on that? – Ashish Jul 29 '17 at 05:24
  • 1
    $ajax is good - saves on debugging and proving code, you're using Jquery anyway, and it's documented. But be wary of treating jqXHR objects returned by $ajax before JQuery version 3 as promise objects - they were not fully compliant with promise specifications. – traktor Jul 29 '17 at 05:38
  • @Tracktor53 Could you please provide any structure layout of the code for my example with ajax. Actually I am new to javascript/ajax so it being hard for me to put the pieces together. – Ashish Jul 29 '17 at 05:43