Returning an error message to an end user upon exc

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");
        }
    };
})

Answer 1

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...

Answer 2

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.";
          }
     }

Answer 3

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:

1. You could execute those methods in parallel (better performance), and handle the return value (or possible error message) separately.
2. 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)