1
\$\begingroup\$

The intention is to implement an xmlHttpRequest in plain vanilla js while considering all possible errors and problem situations without crashing in the browser. Result and faults are to be properly communicated back to the caller.

function get() {
    return new Promise(function(resolve, reject){
        var xmlHttpRequest = new XMLHttpRequest();
        xmlHttpRequest.open("GET", "http://localhost/products", true);
        xmlHttpRequest.onreadystatechange = function() {
            if (xmlHttpRequest.readyState === 4) {
                try { 
                    var data = JSON.parse(xmlHttpRequest.response);
                } catch (error) { // in case parser error
                    reject(error);
                    return;
                }
                if(xmlHttpRequest.status === 200) {
                    resolve(data);
                } else {
                    reject(data);
                }
            }
        };
        xmlHttpRequest.addEventListener("error", function(error){
            reject(error);
        });
        xmlHttpRequest.send();
    });
}
\$\endgroup\$
  • \$\begingroup\$ I'd also like to mention that there's a status 0 condition, it happens when browser is offline, that means no json response etc. \$\endgroup\$ – Developer Feb 22 at 4:36
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Simon Forsberg Feb 25 at 8:58
2
\$\begingroup\$

Here are a few issues with your code

  • If older browsers aren't a concern, you can just use fetch instead of XHR. It's built-in and uses promises.
  • HTTP 200 isn't the only "successful" response status. See https://en.wikipedia.org/wiki/List_of_HTTP_status_codes.
  • Status "0" is a generic network error, not just being offline. It can also be caused by the browser suppressing the request due to some restriction, an extension blocking the request, an insecure connection/invalid certificate.
  • Send an Accept header with application/json as value. This is because, while the server might support JSON, it might not respond with it by default.
  • The third argument of xhr.open is by default true. You may omit that third argument.
  • You can use readystatechange with addEventListener instead of the onreadystatechange property to assign your callback.
\$\endgroup\$
  • \$\begingroup\$ Thanks Joseph for review, I've edited with version 2 of my code. It'd be very helpful to know the exact error message for your point #3, is there a property that I can read on xhr. REST contract dictates 200 for the success in this case. I've incorporated the rest of the feedback. please review. \$\endgroup\$ – Developer Feb 25 at 6:10
  • \$\begingroup\$ Hi Joseph, they didn't let me update the code, can you please let me know how to get specific error details when status is 0 from xhr object? \$\endgroup\$ – Developer Feb 25 at 9:01
  • \$\begingroup\$ hi joseoph, reaching out to you again, can you please explain how to read such kind of errors from xhr object, I've tried statusText but it's empty string. \$\endgroup\$ – Developer Mar 1 at 0:23
  • \$\begingroup\$ @Developer Status 0, as far as I know, does not provide any textual response. This behavior may be vendor-specific, but I wouldn't count on it to return anything meaningful cross-browser. Hence "generic network error". \$\endgroup\$ – Joseph Mar 1 at 13:25
  • \$\begingroup\$ Thanks Joseph. I've searched a lot and I'm totally blocked on this. So there's no description on xhr, that's disappointing that API is designed like this. In other words it's an error with no details at the code level, and I'll be forced to open developer tools and see what went wrong. (it's harder on mobile apps) \$\endgroup\$ – Developer Mar 1 at 13:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.