You are using a single global variable for your xmlhttp
and trying to run multiple ajax calls at the same time. As such each successive ajax call will overwrite the previous ajax object.
I'd suggest adding var
in front of the xmlhttp
declaration to make it a local variable in your function so each ajax request can have its own separate state.
function articleLinkClickAction(guid,callback){
var host = window.location.hostname;
var action = 'http://localhost:7070/assets/find';
var url = action + '?listOfGUID=' + guid.nodeValue;
console.log("URL "+url);
// add var in front of xmlhttp here to make it a local variable
var xmlhttp = getAjaxInstance();
xmlhttp.onreadystatechange = function()
{
if (xmlhttp.readyState == 4 && xmlhttp.status == 200) {
var response = JSON.parse(xmlhttp.responseText);
console.log(response);
console.log(xmlhttp.responseText);
callback(null, xmlhttp.responseText);// this is line causing error
}
else{
callback(xmlhttp.statusText);// this is line causing error
}
};
xmlhttp.open("GET", url, true);
xmlhttp.send(null);
}
In the future, you should consider using Javascript's strict
mode because these "accidental" global variables are not allowed in strict mode and will report an error to make you explicitly declare all variables as local or global (whichever you intend).
I can't say if this is the only error stopping your code from working, but it is certainly a significant error that is in the way of proper operation.
Here's another significant issue. In your real code (seen in a private chat), you are using:
document.body.innerHTML += html
in the middle of the iteration of an HTMLCollection obtained like this:
var anchors = document.getElementsByTagName("a");
In this code, anchors
will be a live HTMLCollection. That means it will change dynamically anytime an anchor element is added or removed from the document. But, each time you do document.body.innerHTML += html
that recreates the entire body elements from scratch and thus completely changes the anchors
HTMLCollection
. Doing document.body.innerHTML += html
in the first place is just a bad practice. Instead, you should append new elements to the existing DOM. I don't know exactly what's in that html, but you should probably just create a div, put the HTML in it and append the div like this:
var div = document.createElement("div");
div.innerHTML = html;
document.body.appendChild(div);
But, this isn't quite all yet because if this new HTML contains more <a>
tags, then your live HTMLCollection in anchors
will still change.
I'd suggestion changing this code block:
var anchors = document.getElementsByTagName("a");
var result = '';
for(var i = 0; i < anchors.length; i++) {
var anchor = anchors[i];
var guid = anchor.attributes.getNamedItem('GUID');
if(guid)
{
articleLinkClickAction(guid,function(err, response) { // pass an anonymous function
if (err) {
return "";
} else {
var res = response;
html = new EJS({url:'http://' + host + ':1010/OtherDomain/article-popup.ejs'}).render({price:res.content[i].price});
document.body.innerHTML += html;
}
});
}
}
to this:
(function() {
// get static copy of anchors that won't change as document is modified
var anchors = Array.prototype.slice.call(document.getElementsByTagName("a"));
var result = '';
for (var i = 0; i < anchors.length; i++) {
var anchor = anchors[i];
var guid = anchor.attributes.getNamedItem('GUID');
if (guid) {
articleLinkClickAction(guid, function (err, response) { // pass an anonymous function
if (err) {
//return "";
console.log('error : ' + err);
} else {
var res = response;
var html = new EJS({
url: 'http://' + host + ':1010/OtherDomain/article-popup.ejs'
}).render({
price: res.content[i].price
});
var div = document.createElement("div");
div.innerHTML = html;
document.body.appendChild(html);
}
});
}
}
})();
This makes the following changes:
Encloses the code in an IIFE (self executing function) so the variables declared in the code block are not global.
Changes from document.body.innerHTML += html
to use document.body.appendChild()
to avoid recreating all the DOM elements every time.
Declares var html
so it's a local variable, not another accidental global.
Makes a copy of the result from document.getElementsByTagName("a")
using Array.prototype.slice.call()
so the array will not change as the document is modified, allowing us to accurately iterate it.