Exception Handling in webapi MVC.net

Question

I got an API with a controller and a service, when invoking one of the action in the controller I must apply a validation, this validation need to check the data in DB to validate that is correct.

As far as I can see there are two ways of handling this

1- Validate before calling the "update" to prevent the error

public IActionResult UpdateCustomer(CustomerDto customer)
{
   if (!customerService.Validate(customer))
   {
       return Send400BadRequest();
   }
   customerService.Update(customer);

   return Send200Ok();
}

2- Call the update validate inside and throw an Exception.

 public IActionResult UpdateCustomer(CustomerDto customer)
 {
    customerService.Update(customer);    
    return Send200Ok();
 }

In customer service

  public void Update(CustomerDto customer)
  {
     if (!Validate(customer)
        throws new CustomValidationException("Customer is not valid");
     //Apply update
  }

We already have an ActionFilter to handle the CustomValidationException so it will return a BadRequest.

1) Pros +Don't use Exception to use the running flow

Cons -Controller is more fat, has more decision on each case it will decide which is the output

2) Pros +Operation is more atomic, all logic is inside the service. +Easier to test +Every use of the method will be validated.

Cons -Use exception to manage the flow.

Which is a better solution?

I really need arguments to defend one or the other.

Answer 1

I think that you should validate in both the controller and the service, but validate slightly different things. Especially if what starts off as just an api turns into an api, with a mvc site and a wpf admin interface as often happens.

Using Validation Attributes and Model Binding gives the data a quick "once over" check. Did they supply the id for the customer to update? Did they submit foreign key value for a related entity? Is the price between 0 and 1,000,000? If you have a website then you also get client validation out of the box. Crucially there no need to connect to the database at all, as the data is "obviously" wrong.

Validation in the service is also required as you may end up with 3 or 4 applications calling this service, and you want to write the business logic once. Some validation eg. referencing foreign keys can only be done in the database, but there is nothing wrong in throwing an exception. Consumers of your api should have access to the range of valid values, users of the website should be choosing from a drop down etc, so this circumstance should be exceptional

Answer 2

If you have a Business Logic Layer and a Service Layer, I prefer to keep all Business Logic Rules including Business Logic Validations in Business Logic Layer and use Service Layer as a wrapper around Business Logic Layer and throw Exception in Business methods.

When deciding about whether to use Exception for Business Validation rules or not, you can consider:

1) It's better that your Business methods be a Unit of Work. They should perform a complete task. So it's better they contain also validation rules. This way you can reuse such Business Logic Layer across different Service Layers or use the same Unit of Work in different methods of the same Service Layer. If you throw a Business Validation Exception in Business Logic Layer, you will not face with risk of forgetting validation or using another validation rule by mistake and each service method / action will perform a single task for you and will be as lightweight as possible.

Think about when you may need to expose a WCF service for some clients or for example if you may use ASP.NET MVC without using WebAPI or iff you want to use the same logic in another Action method in the same WebAPI.

If you put Business Logic validations in Web API controller, when creating WCF service methods or creating MVC Actions or other service methods, you may forget to apply validations or may apply wrong validations with different rules in the new Service Layer.

2) Considering the first benefit, can you return a meaningful value from methods that shows success, failure, or contain suitable information about failure reason in output?

I believe it's not suitable to use out put of method for all these goals. The method output is method output, it should be data in such business application. It should not be sometimes a status, sometimes data or some times message. Throwing exception will solve this problem.

Answer 3

I am going against the other opinions on here and saying that the first method both clearly illustrates what the intent of your method is and if you ever decide to not return a 400 error, its a bit easier to pull that off in scenario #1.

Additionally, some thoughts on exceptions. Exceptions should be exceptional, meaning unexpected events that occur in your code. A company not passing a validation check is not an exception-al event, it either does pass or does not pass. Passing a User object into the ValidateCompany() method should throw an exception.

Here is a good answer on the similar topic of exception throwing. It uses an easy example problem to determine when in that case an exception should be thrown or not.

In regards to "Easier to test" - I don't see how. Your controller will have two tests with any option you choose, a valid company and an invalid company. Your service will have two tests with any option you choose, a valid company and an invalid company (obviously simplifying your service layer a bit here). In any case you want to ensure both your controller action and your service layer can handle an invalid and valid company object.

Answer 4

Receive a validatable customer model, pass this model to the data service layer, and perform exception handling in the controller if the data service layer fails.

Customer model

Implement IValidatableObject to trap business logic related errors.

public class CustomerModel : IValidatableObject
{
    public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
    {
        // example validation
        if ( Balance < 100 )
            yield return new ValidationResult("Balance >= 100 req.", new [] { nameof(Balance) });
    }
    public string Name { get; set; }
    public double Balance { get; set; }
}

API Controller

Receive a CustomerModel in your public facing UpdateCustomer API method, then reference ModelState to determine if the customer is valid.

public IActionResult UpdateCustomer(CustomerModel customer)
{
    if(ModelState.IsValid) // model validation succeeded
    {
        try
        {
            customerService.Update(customer);
        }
        catch (ServiceUpdateException svcEx)
        {
            // handled failure at the service layer
            return Conflict();
        }
        catch (Exception ex)
        {
            // unhandled error occured
            return InternalServerError(ex);
        }
        // update succeeded
        return Ok();
    }
    // invalid model state
    return BadRequest();
}

Service (Data Layer)

public void Update(Customer customer)
{
     //Apply update
     try
     {
         database.Update(customer);
     }
     catch (DataStorageException dbEx)
     {
         throw new ServiceUpdateException(dbEx);
     }
     catch
     {
         throw;//unknown exception
     }
}

Answer 5

I would do it this way:

public IActionResult UpdateCustomer(CustomerDto customer)
    {
    try
        {
        consumerService.Update (customer);
        }
    catch (CustomValidationException)
        {
        return Send400BadRequest ();
        }

    return Send200Ok ();
    }

And in your CustomerService:

   public void Update(CustomerDto customer)     
       {
       if (!Validate(customer))
           throw new CustomValidationException("Customer is not valid");
       }

This way your service has the validation logic. So any other callers to your service will also have their inputs validated before attempting an update and hence will get proper error messages. Your controller will not have any validation code. And moreover, having the try-catch block means you can gracefully handle any other errors the Update call might throw. Hope that helps.

Answer 6

i think handling exception by using httpresponse message is much better then any one else. Atleast you got the proper error response with proper http response message as output.

      public HttpResponseMessage TestException()
      {
        try
        {
            //if your code works well
            return Request.CreateResponse(HttpStatusCode.OK);
        }
        catch (Exception ex)
        {

            return  Request.CreateErrorResponse(HttpStatusCode.ExpectationFailed, ex.ToString());
        }
      }

Answer 7

I would prefer 2 :)

Because I think service might be called from another node not only the asp.net controller so it would be nice for me if all validation logic is handled in the single layer like Service layer.