Is it bad practice to use a global XMLHttpRequest?

user_19089035 0
Viewed 51

I am working on the example from this page, trying to learn some javascript: https://developer.mozilla.org/en-US/docs/Web/Guide/AJAX/Getting_Started

And I have a little web server and a status page that I made that I want to use to check the status of the web server. The way I have it built is I want to check the status of the web server onLoad rather than with a button.

HTML
Just using:     <body onload=pingSite();> to try to automatically run the check when the page loads.

Javascript
  <script>
            let xhr = new XMLHttpRequest(); <--- The question and the problem.
            function pingSite() {
                var xhr = new XMLHttpRequest();
                response = 'xyz';
                //If an XMLHTTP instance cannot be created
                if(!xhr)
                {
                    response = 'Internal error: Cannot create XMLHTTP instance.'
                    return response;
                }
                xhr.onreadystatechange = HandlePing;
                xhr.open('GET', 'mysite.com');
                xhr.send();
                
                return response;
            }

            function HandlePing() {
                    //If the request has finished
                if(xhr.readyState === XMLHttpRequest.DONE)
                {
                    try {
                        //If the status is 200 (IE we have recieved a response back indicating the server is up.)
                        if(xhr.status === 200)
                        {
                            alert("Server is up");
                        }
                        else
                        {
                            alert("There was a problem with the request");
                        }
                    //If the server is down. 
                    } catch (error) {
                        alert(`Caught Exception: ${error.description}`);
                    }
                }
                else
                {
                    response = 'Pinging...';
                }
            }

    </script>

The problem I have is two fold: 1.) The only way I can get this to work is by creating a global variable in my script above both of the functions in order to get the call to work. I have a gut feeling this is really dangerous and bad practice. Is it, and if so, what is a better way to approach this problem?

2.) The way I have it set up seems to work, but it doesn't return any indication that it worked at all. There is no alert. There is no response. The console is empty. Am I missing something? Do I need an event handler in the HTML despite the fact I am doing it onload?

javascript
1 Answers

You could dodge the global variable by creating an anonymous inline function that passes xhr as an argument:

xhr.onreadystatechange = () => HandlePing(xhr);

Or with bind:

// same as arrow function above but harder to read
xhr.onreadystatechange = HandlePing.bind(null, xhr);

Or, assuming you don't need it anywhere else, you could move the HandlePing function declaration into the pingSite function:

function pingSite() {
  function handlePing() {
    if(xhr.readyState === XMLHttpRequest.DONE)
    // ...
  }

  const xhr = new XMLHttpRequest();

  // ...other stuff...

  const xhr.onreadystatechange = HandlePing;
  // ...
}

A few additional thoughts:

  1. You should use fetch instead of XMLHttpRequest.

  2. You should use addEventListener instead of attaching an onload attribute to the body.

  3. fetch uses Promises, which make it easier to manage asynchronous behavior.

The skeletal implementation might look something like this:

window.addEventListener('load', handleLoadEvent);

function handleLoadEvent(e) {
   fetch('http://example.com')
      .then( response => {
        // do stuff with the response
      })
      .catch( error => {
        // deal with errors
      })
}

And if you didn't want to pollute the global namespace with the handleLoadEvent function, you could wrap this all in an IIFE:

(function () {
  window.addEventListener('load', handleLoadEvent);

  function handleLoadEvent(e) {
     fetch('http://example.com')
        .then( response => {
          // do stuff with the response
        })
        .catch( error => {
          // deal with errors
        })
  }
})()

Or if you prefer async/await you could write the handleLoadEvent function that way:

async function handleLoadEvent(e) {
  try {
    const response = await fetch('http://example.com');
    // do stuff with response
  }
  catch (e) {
    // deal with error
  }
}
user_6360770
Related