Returning an error message to an end user upon exception in WebApi methods

Question

I am currently building a WebApi service that connects with the backoffice of my application allowing the user to click a button an trigger a series of web service requests that trigger my Umbraco website to updated with content.

The class for my WebApi service is setup as so:

public class MyController : UmbracoApiController{
     // Global variable declarations go here

     private MyController(){
          // assign actual values to global variables here
     }

     public string GetAllUpdates(){
          try{
              GetCountries();
              GetAreas();
              GetCities();
              ...
          }
          catch (TimeoutException ex)
          {
               Log.Error("Web Service Timeout", ex);
               return "Failed to contact web services.";
          }
          catch (Exception ex)
          {
               Log.Error("Exception", ex);
               return "An error has occurred.";
          }
     }

     public string GetCountries(){
         try{
               // Main code here
         }
         catch(TimeoutException ex){
           Log.Error("Web Service Timeout", ex);
           return "Failed to contact web services.";
         }
         catch(Exception ex){
           Log.Error("Exception", ex);
           return "An error has occurred.";
         }
     }

     .....
}

My WebApi method are called through the use of AngularJS and HTML. When a user clicks the button related to a Web Api method, that method is run then a message is returned to the user upon success or failure of the method.

So, if I were to run the GetCountries() method using the relevant button a message would be displayed on the screen saying "Countries updated successfully" upon success or instead it would output one of the messages that are returned in the catch definitions.

My main problem is that the GetAllUpdates() method runs all of the other methods one after the other. This is fine and I return a message upon success saying "All content updated" however the setup really falls apart when an exception is hit in one of the other methods. For example, GetCountries() is run first. If an exception is raised here, no message is returned to the screen and the application simply moves onto the next method GetAreas(). This propagates through to the end where the success message is then displayed giving the user a false impression that the update went well.

My question therefore is how do I halt the GetAllUpdates() method from running subsequent methods in the chain if an exception occurs without modifying the returns in my other methods? The methods need to return a string so that the message is displayed on screen when they are run on their own however when ran as part of GetAllUpdates() the error messages being returned in the catch blocks are being treated as if the method was completely successful and the user is none the wiser.

Any help would be greatly appreciated.

N.B. After following one of the responses below I altered my WebApi method to do the following:

NEW CLASS

public class ResponseMessage
    {
        public bool Result { get; set; }
        public string Message { get; set; }
    }

RETURN ON METHODS

            ResponseMessage response = new ResponseMessage();
            String json = string.Empty;

            response.Result = true;
            response.Message = "Content update successful";

            json = JsonConvert.SerializeObject(response);
            return json;

My Angular code is as follows:

 angular.module("umbraco")
.controller("AxumTailorMade",
function ($scope, $http, AxumTailorMade) {
    $scope.getAll = function() {
        $scope.load = true;
        $scope.info = "Retreiving updates";
        AxumTailorMade.getAll().success(function (data) {
            var response = JSON.parse(data);
            $scope.result = response.Result;
            $scope.info = response.Message;
            $scope.load = false;
        });
    };

  });

angular.module("umbraco.services").factory("AxumTailorMade", function ($http) {
    return {
        getAll: function () {
            return $http.get("/umbraco/api/axumtailormade/getallupdates");
        },
        getRegions: function () {
            return $http.get("/umbraco/api/axumtailormade/getregions");
        },
        getCountries: function () {
            return $http.get("/umbraco/api/axumtailormade/getcountries");
        },
        getAreas: function () {
            return $http.get("/umbraco/api/axumtailormade/getareas");
        },
        getCities: function () {
            return $http.get("/umbraco/api/axumtailormade/getcities");
        },
        getCategories: function () {
            return $http.get("/umbraco/api/axumtailormade/getcategories");
        },
        getPackages: function () {
            return $http.get("/umbraco/api/axumtailormade/getpackages");
        },
        getHotels: function () {
            return $http.get("/umbraco/api/axumtailormade/gethotels");
        },
        getActivities: function () {
            return $http.get("/umbraco/api/axumtailormade/getactivities");
        }
    };
})

You should throw errors, or allow errors to propagate from your service call. Do not swallow the error and return a result. When an error is thrown, the promise is rejected, and response.data contains the exception — Michael Kang, Feb 03 '15 at 12:34
Exceptions are not end user readable and should not be returned in such a manner. Also, throwing errors and allowing them to propagate in this case would mean that the errors are thrown all the way up to the private instantiation method of the class which does not allow for any messages to be returned to the end user leaving them with no feedback — jezzipin, Feb 03 '15 at 12:36

timothyclifford · Accepted Answer · 2015-02-03T12:35:02.123

2

Rather than returning a string from your methods, could you instead return a simple object containing the result and message for client?

public class ResponseMessage
{
    public bool Result { get; set; }
    public string Message { get; set; }
}

Then when executing GetAllUpdates() you would check result of GetCountries(), GetAreas() and GetCities() and append any error messages.

public ResponseMessage GetAllUpdates()
{
    ResponseMessage response;

    response = GetCountries();
    if (!response)
    {
        return response;
    }

    response = GetAreas();
    if (!response)
    {
        return response;
    }

    response = GetCities();
    if (!response)
    {
        return response;
    }

    response.Result = true;
    response.Message = "Successful";

    return response;
}

public ResponseMessage GetCountries()
{
    var response = new ResponseMessage();

    try
    {
        // Your main code here, call services etc

        response.Result = true;
        response.Message = "GetCountries successful";
    }
    catch (TimeoutException ex)
    {
        Log.Error("Web Service Timeout", ex);
        response.Result = false;
        response.Message = "Failed to contact web services.";
    }
    catch (Exception ex)
    {
        Log.Error("Exception", ex);
        response.Result = false;
        response.Message = "An error has occurred.";
    }

    return response;
}

It is a little more effort and code but will also give you more control over the flow and what's returned to client. You could also optimise it quite a lot I'm sure, I just stubbed out a general approach for you...

edited Feb 03 '15 at 12:35

answered Feb 03 '15 at 12:20

timothyclifford

6,799
7
57
85

So far this is looking like the only viable option. – jezzipin Feb 03 '15 at 13:11
The only problem I have now is that I am returning the response object as JSON to my AngularJS code but it fails to pickup any of the values despite using JSON.parse – jezzipin Feb 03 '15 at 13:48
Can you supply your Angular code please? Also, can you see what the response is coming through as by using console.log or the Network tab in your browser dev tools? – timothyclifford Feb 03 '15 at 13:50
Please see the edit above. The console.log in my AngularJS returns undefined despite Firebug showing that the string is coming through correctly – jezzipin Feb 03 '15 at 13:59
It's difficult to tell without seeing all your Angular code sorry! Can you supply all the related code please? Otherwise I would suggest closing out this question and starting a new one related to your Angular issue. – timothyclifford Feb 03 '15 at 14:15
Okay so it turns out that I was re-encoding my JSON when it was coming through on the Javascript end. This was unneccesary. – jezzipin Feb 03 '15 at 15:05

score 0 · Answer 2 · answered Feb 03 '15 at 12:21

You can update your `GetAllUpdate` method something like this

public string GetAllUpdates(){
          try{
              var result = GetCountries();

              if(string.isNullOrEmpty(result)
               return result;


              result  = GetAreas();

              if(string.isNullOrEmpty(result)
               return result;

              result  = GetCities();

              if(string.isNullOrEmpty(result)
               return result;

              ...

              result = "All updated successfully";

              return result;

          }
          catch (TimeoutException ex)
          {
               Log.Error("Web Service Timeout", ex);
               return "Failed to contact web services.";
          }
          catch (Exception ex)
          {
               Log.Error("Exception", ex);
               return "An error has occurred.";
          }
     }

This will not work. What happens when a method completes successfully? A message is returned therefore this would always stop after the first method executes. — jezzipin, Feb 03 '15 at 12:27

score 0 · Answer 3 · edited May 23 '17 at 12:23

I would suggest you actually get rid of the GetAllUpdates() method, and call the particular Get****() methods separately from AngularJS.

This would have the following advantages:

You could execute those methods in parallel (better performance), and handle the return value (or possible error message) separately.
As I understand - the fact that GetCountries() failed does not mean GetAreas() should fail or should not be executed; it is just the GetAllUpdates() method which requires this. You can therefore handle each error independently, while letting as many of the methods succeed as possible.

If you do not want/need to call the particular methods in parallel, it would still be better, in my opinion, to call them one by one, hanging the call to every subsequent one off its predecessor's promise (see "Chaining Promises" here: https://docs.angularjs.org/api/ng/service/$q)

Finally - I would not return a string from the WebAPI methods. Rather - you should return an HTTP 200 code for success (i.e. make the methods void, and simply not do anything special), or a specific HTTP code for whatever error has happened. In Angular you can then use .catch() to handle the exceptions. This way your messages will not be decided in the WebAPI code - but on the client side.

How do you return status 401 from WebAPI to AngularJS and also include a custom message?

EDIT:

If the client-side option is not possible, I would make the calls from nested try/catch blocks in the server code:

 public string GetAllUpdates()
 {
      try
      {
          GetCountries();
          try 
          {
              GetAreas();
              // ... and so on
          }
          catch(Exception ex)
          {
              return "Error getting areas";
          }

      }
      catch(Exception ex) 
      {
          return "Error getting countries";
      }

Alternatively, you could put the calls and respective error messages into lists:

var calls = new List<Action> { GetCountries, GetAreas, Get... };
var errorMessages = new List<string> { "Error getting countries", "Error getting areas", "Error getting ..."} ;

and then call the methods in a for loop:

for(var i = 0; i < calls.Count; i++) 
{
    try 
    {
        var call = calls[i];
        call();
    }
    catch(Exception ex) 
    {
        return errorMessages[i];
    } 
}

Make sense? (Sorry if the code doesn't compile, I'm not checking it in a compiler unfortunately)

Thanks for your response. Unfortunately this will not work with this application as the methods should not be run in parallel. Areas are a part of a country in the context of the application so the get countries should always be run first when doing a complete update to ensure that the country exists before the areas are processed. If this is done in parellel then this will not be the case and the areas may fail to be updated because the necessary country was not brought through, Furthermore this WebAPI method will be used again so clientside is not an option here. — jezzipin, Feb 03 '15 at 12:50

Returning an error message to an end user upon exception in WebApi methods

3 Answers3

Linked