foreach only sends last xhr request

Question

I have a textarea where users can enter multiple URLs which in return will be used for an API request.

The issue I run into is that only the last URLs API request gets captured (sometimes multiple times).

$('.start').on('click',function()
{
    var url_list = $("#url-list").val();
    var urls = url_list.split("\n");

    for (var i = 0, len = urls.length; i < len; i++) {
        console.log("i is "+i)
        var xhr = new XMLHttpRequest();
        xhr.open('GET', urls[i], true);
        xhr.send();

        xhr.onreadystatechange = processRequest;

        // send API request
        function processRequest() {
            if (xhr.readyState == 4 && xhr.status == 200) {
                var response = JSON.parse(xhr.responseText);
                console.log(response);
            }
        }
    }
});

I don't see exactly where I am doing something wrong, I might be blind to it or just don't know any better. Any help would be appreciated. PS. fairly new to making API requests.

Why are you using jQuery for element selection, but not for the AJAX request… that's really one of the only benefits it offers. Also, you're in a race condition, you need a closure — vol7ron, Mar 22 '17 at 02:16
Possible duplicate of [dealing with loops in javascript, only last item gets affected?](http://stackoverflow.com/questions/22438002/dealing-with-loops-in-javascript-only-last-item-gets-affected) — vault, Mar 22 '17 at 02:18
@vol7ron thanks for the note. The only reason I didn't is because I read only that just for one simple GET request it would be on overkill. After re-reading the article I realized that only using the jQuery library for one request is on overkill but not if you already use it. Thanks! — Chris Palmer Breuer, Mar 22 '17 at 02:24
Correct, if you're going to load it, you might as well use it, unless you really need performance outside of what it offers. — vol7ron, Mar 22 '17 at 02:26

score 3 · Accepted Answer · edited May 23 '17 at 12:32

3

Welcome to closures.

The problems here are:

The loop could be finished before the first request starts, so it takes the last url in the array
In the callback you are referencing the same xhr object

You can try one of the different solutions here.

Replacing xhr with this in the callback is the fastest fix:

function processRequest() {
  if (this.readyState == 4 && this.status == 200) {
    var response = JSON.parse(this.responseText);
    console.log(response);
  }
}

See this fiddle for a running example.

edited May 23 '17 at 12:32

Community

1
1

answered Mar 22 '17 at 02:21

vault

3,930
1
35
46

Thank you so much. I put `(function (i) { }` inside the loop and it started working correctly. – Chris Palmer Breuer Mar 22 '17 at 02:23
*`urls[i]`* - No, the `i` variable is only used by synchronous code that creates the requests, there is no code using it after the loop has completed. I think the real problem is similar though: the ready state change handler uses the `xhr` variable. – nnnnnn Mar 22 '17 at 02:27
@ChrisPalmerBreuer instead of creating an anonymous function, it might be better to use `bind()` – vol7ron Mar 22 '17 at 02:35

Trung Duong · Answer 2 · 2017-03-23T07:32:45.037

You could update your code like this

var requestIndex = 0;
var urls = [];

$('.start').on('click',function()
{
    // Reset request index
    requestIndex = 0;

    var url_list = $("#url-list").val();
    urls = url_list.split("\n");

    // Send Http request    
    sendRequest(urls, requestIndex);

});

// Send Http request
function sendRequest(urls, index) {

    // Send API request 
    var xhr = new XMLHttpRequest();
    xhr.open('GET', urls[index], true);
    xhr.onreadystatechange = processRequest;
    xhr.send();

}

// Process API request
function processRequest(e) {

    if (e.target.readyState == 4 && e.target.status == 200) {
        var response = JSON.parse(e.target.responseText);
        console.log(response);
    }

    requestIndex++;
    if (requestIndex < urls.length) {
       sendRequest(urls, requestIndex);
    }

}

there are no problems in sending multiple requests at once. and previous requests are not canceled. — vault, Mar 22 '17 at 03:07

foreach only sends last xhr request

2 Answers2