3

I am using Web API 2.0 to create my own project. My Api contain ability to Add , Get and book product.

I Want to handle exception but there is some issue I little bit confused.

I have a controller: ProdctController with action method AddProdct.

   [Route("~/CKService.svc/author/add")]
    [HttpPost]
    public IHttpActionResult AddProduct(ProductRequest request)
    {
        ProductManager.AddProduct(request.ProductId, request.ProductName, 
        request.Price);

        return Ok(new AddProductResponse()
        {
            Status = AddProductStatus.Success
        };)
    }

Now, if the product already exists, so the data access layer will throw DuplicateKeyException

I want to handle this excpetion and return response like this:

  return Ok(new AddProductResponse()
    {
        Status = AddProductStatus.AlreadyExists
    };)
  1. Is this reasonable to return HTTP status code 200?
  2. How can I do this without add catch DuplicateKeyException to my controller because i think this is not the right way.

Thanks

maz
  • 515
  • 2
  • 8
  • 22
  • 3
    If it is an error you should never return HTTP 200. – Patrick Hofman May 12 '17 at 10:36
  • 2
    And there is [a lot](https://www.google.com/search?q=web+api+error+handling) on exception handling in ASP.NET Web API. – Patrick Hofman May 12 '17 at 10:37
  • 409 Conflict - would be a resonable HTTP Code [reference](http://stackoverflow.com/questions/25541203/http-response-code-when-resource-creation-post-fails-due-to-existing-matching-re) – johannespartin May 12 '17 at 10:46
  • 1
    you should return a HTTP status which reflects the error, not an arbitrary code of your own defining within a "success" method. Make use of the existing, well-defined standard codes that exist. Then your response has intrinsic semantic meaning which any generic API client can work with. Returning "OK" when there's a problem is effectively misleading the client application, even if you document it, and not how RESTful APIs are intended to work. – ADyson May 12 '17 at 10:52
  • @ADyson thank you , I still confused, for example if I have book_seat action method but airplane is already full in this case what is the right status code to be returned? – maz May 12 '17 at 11:16
  • it's somewhat up to you - as in everything there's no definitive "correct" response, a lot depends on your application and requirements. But, if I was handling that case, I might be inclined to treat it as an invalid request (i.e. requesting something impossible), so I'd return 400 (Bad Request). – ADyson May 12 '17 at 11:22
  • Obviously your response should include some information about the exact nature of the problem, so the client can take action to fix it. A common WebAPI convention with BadRequest responses is to return the ModelState, which you can customise the contents of if you need to include info such as this which isn't directly a validation error with the Model's data structure. I actually answered a similar-ish query yesterday, and the C# part of the answer would be helpful to you - see here: http://stackoverflow.com/a/43910495/5947043 – ADyson May 12 '17 at 11:25
  • @ADyson Ok I got your point, Do you have explanation why google Rest API map return Http code 200 for all request (In http level), if there is an error it will be returned as part as the json response and not http resposne?https://developers.google.com/maps/documentation/directions/intro#Waypoints – maz May 12 '17 at 12:18
  • all this stuff is somewhat a matter of opinion. Google Maps are entitled to do it their way. You can find lots of material online giving one viewpoint or another. Google's calendar API works more like the way I've described: https://developers.google.com/google-apps/calendar/v3/errors . Like I said, there are no definitive rules. I just commented on what I think is the best and most clear thing to do (hence a comment not an answer :-)), making use of existing standards and adhering to a style which is widely used and quite logical. – ADyson May 12 '17 at 12:27

2 Answers2

1

Here is one suggestion. (and I emphasis that it is just one of the many options you have available)

Refactor the manager to handle and return the status of adding the product and then have the action return the appropriate response code based on the status.

[Route("~/CKService.svc/author/add")]
[HttpPost]
public IHttpActionResult AddProduct(ProductRequest request) {
    var status = ProductManager.AddProduct(request.ProductId, request.ProductName, request.Price);
    var response = new AddProductResponse() {
        Status = status
    };
    if(status == AddProductStatus.Success) {
        return Ok(response);
    }
    return BadRequest(response);
}

Avoid return 200 OK for requests that have not completed as intended. That will mislead the client making the request.

Nkosi
  • 235,767
  • 35
  • 427
  • 472
  • can you explain why to avoid use HTTP 200? look at this: http://stackoverflow.com/questions/27921537/returning-http-200-ok-with-error-within-response-body – maz May 12 '17 at 10:57
1

HTTP 200 is not resonable - it indicates a successfull request. Use a 4xx (Error) Code instead. 409 (conflict) fits your situation of an already existing object.

See: HTTP response code when resource creation POST fails due to existing matching resource

To catch the exception is totally fine. An unhandled exception would result in a 500 error (internal server error) which is not meaningful by any means. Instead you should catch the Exception and return a HTTP Error like this (Exception Handling ASP.NET) :

 throw new HttpResponseException(
        Request.CreateErrorResponse(HttpStatusCode.Conflict, message))
Community
  • 1
  • 1
johannespartin
  • 501
  • 3
  • 15