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");
 }
 });
 };
Phrancis
20.5k6 gold badges69 silver badges155 bronze badges
asked May 7, 2015 at 0:04
\$\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\$ Commented May 7, 2015 at 0:11

1 Answer 1

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.

answered May 7, 2015 at 1:29
\$\endgroup\$
1
  • \$\begingroup\$ Thanks, these are some helpful suggestions and I feel more confident having had another pair of eyes on it too! \$\endgroup\$ Commented May 7, 2015 at 1:56

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.