3
\$\begingroup\$

I have the following ajax call and if it errors, I have a default error message stored in a constant which I'll use if the response doesn't contain a custom message. When I look at this, I keep thinking it could be done better. Can anyone suggest how I can do it better?

function requestService(){
            $.ajax({
                dataType: 'json',
                type: 'POST',
                url: '/myurl',
                contentType: CONTENT_TYPE,
                timeout: AJAX_TIMEOUT,
                success: function(data, textStatus, jqXHR){
                    populateData(JSON.parse(jqXHR.responseText))
                },
                error: function(err){
                    var errorMessage = AJAX_ERROR;
                    try {
                        errorMessage = JSON.parse(err.responseText).messages.messages[0].description;
                    }
                    catch(error) {
                        //There was an problem finding an error message from the server. The default message will be used.
                    }
                    displayError(errorMessage)
                },
                complete: function(){
                    console.log("complete");
                }
            });
        };
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! I have changed your title to better say what the code does, rather than that you'd like to improve it (that's the whole intent of the site, after all). I hope you get some great answers! \$\endgroup\$ – Phrancis May 7 '15 at 0:11
2
\$\begingroup\$

I'd say it's pretty much by-the-book already, but you could use the catch block to set the default:

error: function(err){
  var errorMessage;
  try {
    errorMessage = JSON.parse(err.responseText).messages.messages[0].description;
  } catch(exception) {
    errorMessage = AJAX_ERROR;
  }
  displayError(errorMessage);
}

Or, you can use the full try...catch...finally formulation, if you want:

error: function(err){
  var errorMessage;
  try {
    errorMessage = JSON.parse(err.responseText).messages.messages[0].description;
  } catch(exception) {
    errorMessage = AJAX_ERROR;
  } finally {
    displayError(errorMessage);
  }
}

Or you can try getting the custom error message, and only defaulting at the last moment:

error: function(err){
  var errorMessage;
  try {
    errorMessage = JSON.parse(err.responseText).messages.messages[0].description;
  } catch(exception) {
    // ignored
  }
  displayError(errorMessage || AJAX_ERROR);
}

The possible advantage of this, is that if the server for any reason sends a blank (false'y) error message the above will still default to the generic error message, even though no exceptions were thrown.

In any event the overall approach seems OK to me. There's not really a cleaner way of getting the server's error message, since you have to parse the JSON and dig deep into the structure. Either thing may throw exceptions, so try...catch is the simplest way of handling that. At minimum you need the try..catch for JSON.parse anyway, so you might as well use it for both things.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks, these are some helpful suggestions and I feel more confident having had another pair of eyes on it too! \$\endgroup\$ – Leo Farmer May 7 '15 at 1:56

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.