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");
}
});
};
-
\$\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– Phrancis2015年05月07日 00:11:26 +00:00Commented May 7, 2015 at 0:11
1 Answer 1
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.
-
\$\begingroup\$ Thanks, these are some helpful suggestions and I feel more confident having had another pair of eyes on it too! \$\endgroup\$Leo Farmer– Leo Farmer2015年05月07日 01:56:21 +00:00Commented May 7, 2015 at 1:56