Asynchronous GET/POST function with error reporting

Question

I've been working on this asynchronous communication function for a little while as an exercise to improve my experience with Javascript, and as a tool to provide feedback from my fledgling attempts at server side scripting.

I've progressed just about as far as I can with it and I was hoping for some feedback.

I've wondered as I produced this if I should sanitize any user input on the client side if I'm already doing so on the server side? If so any suggestions for how to do this in a secure manner?

I'm also not quite happy with the .callback method ATM as I'm relying heavily on a particular configuration of the response JSON, somewhat on the markup already containing a message element to display the error, and the error trapping feels sloppy. Is there any more flexible, or generally accepted method to handle reporting server side errors to the client?

Any additional style/optimization/security criticisms you all could offer would be appreciated as well.

function callAJAX()
{//async communication to server, sends query string(s), and expects a JSON response
 this.delay = null;
 this.buildXHR = function()
 {//overkill?
 var xhr = false;
 var xhrFactories = [
 function(){return new XMLHttpRequest()},
 function(){return new ActiveXObject("Msxml2.XMLHTTP")},
 function(){return new ActiveXObject("Msxml3.XMLHTTP")},
 function(){return new ActiveXObject("Microsoft.XMLHTTP")}
 ];
 for (var i=0;i<xhrFactories.length;i++)
 {//iterate through request factories, break on success
 try 
 { xhr = xhrFactories[i]();
 }catch (e){
 continue;
 }
 break;
 }
 return xhr;
 };
 this.xhr = this.buildXHR();
 this.callback = function(context, responseStr, fn)
 {//handle callback from GET/POST, and client side error handling
 //could benefit from a more general error reporting solution
 var args = [], responseObj = {}, responseStatusStr = "", dataKeys = [];
 try
 {//trap invalid JSON
 responseObj = JSON.parse(responseStr);
 dataKeys = Object.keys(responseObj);
 } catch(e) {
 responseObj = [{"statusMsg": "Error: Response format not recognized."}];
 }
 if(responseStr !== "[]" && responseStr.indexOf("Error:") === -1 && responseStr.indexOf("<!DOCTYPE") === -1)
 {
 for(var j=responseObj.length;j;j--)
 { for(var i=dataKeys.length;i;i--)
 { if(responseObj[j-1][dataKeys[i-1]])
 {//decode response values
 responseObj[j-1][dataKeys[i-1]] = decodeURIComponent(responseObj[j-1][responseObj[dataKeys[i-1]]]);
 }
 }
 }
 if(responseObj[0].statusMsg)
 {//handle status message
 responseStatusStr = decodeURIComponent(responseObj[0].statusMsg);
 if(responseStatusStr !== "Thread was being aborted." && responseStatusStr != "0")
 if(document.getElementById("message_cb"))
 {//detect if message popup exists in markup
 document.querySelector(".messageTextWrap > span").innerText = responseStatusStr;
 document.getElementById("message_cb").checked = true;
 } else {
 alert(responseStatusStr);
 }
 }
 //push parameters into fn, and execute
 args.push(context);
 args.push(responseObj);
 fn.apply(context, args);
 } else {//more traps
 if(responseStr == "[]")
 { responseObj = [{"statusMsg": "Error: Null response."}];
 } else if(responseStr.indexOf("<!DOCTYPE")) {
 responseObj = [{"statusMsg": "Error: Server response invalid."}];
 }
 //throw error
 if(document.getElementById("message_cb"))
 {//detect if message popup exists in markup
 document.querySelector(".messageTextWrap > span").innerText = responseObj[0].statusMsg;
 document.getElementById("message_cb").checked = true;
 } else {
 alert(responseObj[0].statusMsg);
 }
 }
 };
 this.get = function(self, callUrl, data, fn, context, delay)
 { clearTimeout(self.delay);
 self.delay = setTimeout(function()
 {//prevent incessant calls
 if(self.xhr)
 {// if browser supports XmlHTTPRequest object
 var query = [];
 var dataKeys = Object.keys(data);
 for(var i=dataKeys.length;i;i--)
 {//parse dataObj to queryString
 if(data[dataKeys[i-1]])
 { query.push(encodeURIComponent(dataKeys[i-1]) + '=' + encodeURIComponent(data[dataKeys[i-1]]));
 }
 }
 self.xhr.open("GET", callUrl + (query.length ? "?" + query.join("&") : ""), true);
 self.xhr.setRequestHeader("Content-Type", "application/json; charset=UTF-8");
 self.xhr.send();
 self.xhr.onreadystatechange = function()
 {//set event handler for response
 if(self.xhr.readyState == 4)
 { if(self.xhr.status == 200)
 {//if response is valid make fn call
 self.callback(context, self.xhr.responseText, fn);
 } else {
 self.callback(context, '[{"statusMsg": "Error: ('+ self.xhr.status +') Server request failed."}]', fn);
 }
 }
 };
 }
 }, delay);
 };
 this.post = function(self, callUrl, data, fn, context)
 { if(self.xhr)// check for AJAX compatability
 { var query = [];
 var params = "";
 var dataKeys = Object.keys(data);
 for(var i=dataKeys.length;i;i--)
 {//parse dataObj to queryString
 if(data[dataKeys[i-1]])
 { query.push(encodeURIComponent(dataKeys[i-1]) + '=' + encodeURIComponent(data[dataKeys[i-1]]));
 }
 }
 params = query.length ? query.join("&") : query;
 self.xhr.open("POST", callUrl, true);
 self.xhr.setRequestHeader("Content-Type", "application/x-www-form-urlencoded; charset=UTF-8");
 self.xhr.setRequestHeader("Content-length", params.length);
 self.xhr.setRequestHeader("Connection", "close");
 self.xhr.onreadystatechange = function()
 {//set event handler for response
 if(self.xhr.readyState == 4)
 { if(self.xhr.status == 200)
 {//if response is valid make fn call
 self.callBack(document, self.xhr.responseText, fn);
 } else {
 self.callBack(context, '[{"statusMsg": "Error: ('+ self.xhr.status +') Server request failed."}]', fn);
 }
 }
 };
 self.xhr.send(params);
 }
 };
}

• Sᴀᴍ Onᴇᴌᴀ: That xhrFactory code looks like it came from a quirksmode page that has existed for ~11 years... Is the goal to still support browsers that old?
• Jim: @Sam Onela I've gone back and forth on that topic myself when I started here a few years ago the majority of users were using IE9 in compatability mode, we've since upgraded to IE11, and I've figured out how to force clients out of compatability mode. When I came across that xhrFactory code I thought it was pretty elegant, but probably overkill.
• I wrestled a bear once.: i often still support active x when writing xhr stuff without jquery but the impression i've gotten from other coders is that yea, it's overkill. if you support activex it basically means you can't use es6, etc. in my opinion there's still enough people using older browsers to justify writing code like this if you're writing code for the wild although i haven't run into many people who agree with me.
• Joseph: "if you support activex it basically means you can't use es6," - there are these things called transpilers, polyfills and shims. Writing in runtime JS, no you cannot. But with the right tooling, you can.
• I wrestled a bear once.: @Joseph - if you're transpiling your es6 to es5 then you're not really using es6 are you.. ? besides some things cannot be polyfilled. i can't imagine how you could polyfill a const but I might be wrong. what do i know, i still support active x.

Answer 1

• Your use case, I think, will determine whether or not your XHR factory is overkill. If you're writing a site that sells knitting equipment and potentially have a lot of 70+ year old visitors who are less likely to have updated their browsers then, sure, keep it. If you're selling motherboards to IT people then they probably have pretty recent software. To be safe, consider setting up Google Analytics or similar and after some time see what browsers your visitors are using and then make that decision. (Transpilers like Babel are also an option)
• You shouldn't have to try/catch the JSON response from the server. If there is a problem with the server then you should handle it on the server side. You should be confident enough in the server code not to have to do this. When you start unit testing you'll never never be able to get to 100% code coverage unless you intentionally break your server code. If you try to catch every single thing that could possibly go wrong you will waste a lot of time writing code that will never be used and you will have a lot of untested code in your code base.
• Your success test (responseStr !== "[]" && responseStr.indexOf("Error:") === -1 && responseStr.indexOf("<!DOCTYPE") === -1) is really specific, so much so that it may not be very future proof. Wouldn't it work just to check and see if the response object is a non-empty array whos first item is an object? (Array.isArray(responseObj) && responseObj.length && "object" === typeof responseObj[0]).
• args seems like an unnecessary array. Why not just fn.apply(context, [context, responseObj]); - also, when you write it out like that it becomes apparent that your context is also being passed as the first argument to the function, which is probably not necessary.
• consider using a for..in loop instead of looping the Object.keys()
• Your backwards loops could be shortened.. for(var i=dataKeys.length;i;i--) is equivalent to for(var i=dataKeys.length; i--;) .. you don't have to use all three statements in the for loop.