Javascript functions not calling in correct order

Question

I have tried to simplify the code, but here I am making requests to the api, it all works fine until I try to call function 5 in function 4, where it instead puts the data received by the api into function2.

Has anyone got any idea of how to stop this happening?

var xhr = new XMLHttpRequest();
var counter = 0;

window.onload = function() {
    var add_button = document.getElementById('add_button');
    add_button.addEventListener('click', function1);


var function1 = function() {
    counter++;
    xhr.addEventListener('load', function2);
    var url = 'https://api.example.org/search';
    console.log(url);
    xhr.open("GET", url);
    xhr.send();
};

var function2 = function() {
    var data = JSON.parse(this.response);
    console.log(data);
    function3();
};

var function3 = function() {
    for (var i in searchArray) {
        var Url2 = 'https://api.example.org/search2';
        xhr.open("GET", Url2);
        xhr.addEventListener('load', function4);
        xhr.send();
    };
};

var function4 = function() {
    var data2 = JSON.parse(this.response);
    if (counter === 3) {
        var url3 = 'https://api.example.org/search3';
        xhr.open("GET", url3);
        xhr.addEventListener('load', function5);
        xhr.send();
    };
};

var function5 = function() {
    var data3 = JSON.parse(this.response);
};

Why not use Promises? That way it will be truly asynchronous. — Baruch, Dec 29 '16 at 15:03
You are reusing the xhr variable, I'm guessing both of your listeners that are attached are being called. — Gerrit0, Dec 29 '16 at 15:06
@T.J.Crowder Right, but why not use the tools provided by the language? — Baruch, Dec 29 '16 at 15:08
@Baruch: Using promises would be perfectly valid. It would also be (slightly) more complicated unless you already have a promise-ified XHR wrapper lying around (which is probably a good idea). But that's not the point. The point is that your comment makes a false equivalence. "That way it will be truly asynchronous" says, in effect, "your way isn't truly asynchronous." But it is. — T.J. Crowder, Dec 29 '16 at 15:11
Thanks for the explanation. I guess what I was thinking more about how this looks like forced asynchronicity. But then again I guess this might be another way to avoid callback hell. — Baruch, Dec 29 '16 at 15:18
@Baruch What do you mean it "looks like forced asynchronicity"? — 1252748, Dec 29 '16 at 15:29

score 2 · Accepted Answer · edited May 23 '17 at 10:30

You're reusing a single XMLHttpRequest object. Don't. Create a new one for each request (see *** lines):

var counter = 0;

window.onload = function() {
    var add_button = document.getElementById('add_button');
    add_button.addEventListener('click', function1);


var function1 = function() {
    counter++;
    var xhr = new XMLHttpRequest();                    // ***
    xhr.addEventListener('load', function2);
    var url = 'https://api.example.org/search';
    console.log(url);
    xhr.open("GET", url);
    xhr.send();
};

var function2 = function() {
    var data = JSON.parse(this.response);
    console.log(data);
    function3();
};

var function3 = function() {
    for (var i in searchArray) {
        var Url2 = 'https://api.example.org/search2';
        var xhr = new XMLHttpRequest();                // ***
        xhr.open("GET", Url2);
        xhr.addEventListener('load', function4);
        xhr.send();
    };
};

var function4 = function() {
    var data2 = JSON.parse(this.response);
    if (counter === 3) {
        var url3 = 'https://api.example.org/search3';
        var xhr = new XMLHttpRequest();                // ***
        xhr.open("GET", url3);
        xhr.addEventListener('load', function5);
        xhr.send();
    };
};

var function5 = function() {
    var data3 = JSON.parse(this.response);
};

Also note that function3 creates several overlapping requests (assuming the loop is a loop for a reason), so just beware of that.

The line

for (var i in searchArray) {

is suspicious. You haven't shown searchArray, but if it really is an array, for-in isn't usually the best way to loop through it. See For-each over an array in JavaScript? for details.

Thank you, this worked perfectly and for finding the problem in function3! — Jess, Dec 29 '16 at 15:21

score 1 · Answer 2 · answered Dec 29 '16 at 15:09

The main issue you're seeing comes from trying to reuse the same XHR object.

This XHR object already has one event listener that was added to it in function1.
Next, in function3 you add an additional event listener.
And then another event listener in function4.

Possible solution:
1. Use a different XHR object each time: fn1XHR = new XMLHttpRequest() etc..

Other than that, I think that using a Promise can be a better pattern for the scenario you're describing here.

Javascript functions not calling in correct order

2 Answers2