0

This is very similar, but my question is different: Return content with IHttpActionResult for non-OK response

Considering the question is different, I'm asking for a more concise answer, if it exists.

My architecture is the following:

  1. Javascript/jQuery call to a backend controller
  2. Backend controller calls a WebAPI service
  3. WebAPI service queries db (etc) and returns data

I have the following simplified code (Web API)...

Example 1 return error if product id doesn't exist:

public IHttpActionResult GetProduct(int id)
{
    var product = products.FirstOrDefault((p) => p.Id == id);
    if (product == null)
    {
        return NotFound();
    }
    return Ok(product);
}

Example 2 return empty data if product id doesn't exist:

public IHttpActionResult GetProduct(int id)
{
    var product = products.FirstOrDefault((p) => p.Id == id);
    return Ok(product);
}

Client side JS:

$.getJSON("example.json", function() {
  alert("success");
})
.done(function() { alert('Product retrieved'); })
.fail(function() { alert('Product doesn't exist. '); })
.always(function() { ... });

I've read many many times that using exceptions to control flow is bad practice, which is in effect what will happen if I use NotFound() since it will hit the .fail function, suggesting there was an error (which there wasn't).

In another case when the comment has to be approved by someone other than the person who inserted the comment:

public IHttpActionResult ApproveComment(int rowId, string userName)
{
    try {
        return Ok(BusinessLogicLayer.ApproveComment(rowId, userName));
    }
    catch(Exception ex)
    { 
        // elmah.logerr...
        throw new HttpResponseException(Request.CreateErrorResponse(HttpStatusCode.InternalServerError, ex.InnerException == null ? ex.Message : ex.InnerException.Message));
    }
}

BusinessLogicLayer:
public string ApproveComment(int rowId, string userName)
{
    if (userName == _repository.GetInsertedCommentUserName()) {
        return "You cannot approve your own comment.";
    }
    if(_repository.ApproveComment(rowId, userName)){
        return "Comment approved";
    }
}

OR

public string ApproveComment(int rowId, string userName)
{
    if (userName == _repository.GetInsertedCommentUserName()) {
        throw new Exception("You cannot approve your own comment.");
    }
    if(_repository.ApproveComment(rowId, userName)){
        return "Comment approved";
    }
}

What is a clean and elegant way to return an appropriate message to the user, without using exceptions?

Or is my thinking wrong, is the 'exceptional' circumstance from the users point of view? IE., "I am expecting to get a product returned when I pass in this ID, but alas it doesn't exist!" From a developers/testers point of view I don't think this would be an exceptional case but from an end user's point of view - maybe.

cdsln
  • 830
  • 1
  • 11
  • 33
  • If you look at https://en.wikipedia.org/wiki/List_of_HTTP_status_codes there is a Success status of 204 - No Content, would this resolve your issue? – uk2k05 Jun 07 '19 at 10:14

3 Answers3

2

You're asking two different questions. So let's answer them one by one.

The 'not found' problem

In the first case, the client is trying to access a product that does not exist. The appropriate status code to return from the server is 404, not found. For the client, this is an exceptional case actually, because it is probably trying to access an existing resource. In the 'fail' part of your client side javascript you can then check why the request failed (4xx range = client side error, 5xx range = server side error), and show appropriate messages to the user.

The 'approve' problem

For the approve problem you are returning unuseful statuscodes. You should check whether the resource the client is trying to approve exists before approving. If the resource does not exist, this is a client side error and you should return 404, not found. If somehow the updating of the resource by the business logic layer still fails, and this is a server issue, you should return 500, internal server error. For any situation where it is the client's fault the update fails, return a statuscode in the 4xx range. (like 403, unauthorized, if the client is not allowed to approve its own comments)

Jesse de Wit
  • 3,867
  • 1
  • 20
  • 41
  • I just want to link to the wiki page for [http status codes](https://en.wikipedia.org/wiki/List_of_HTTP_status_codes) for future reference – MindSwipe Jun 07 '19 at 10:43
1

Approving your own comments - This seems like an invalid operation. There is an exception for that.

And it is a bad request from the client then. There is a response status code for that.

There is nothing wrong with having Exceptions in your code, if you handle them consequentially. And there is nothing wrong with giving out 4xx status codes either, if you handle them gracefully on client side. It is a request the client should be unallowed to make, so it should fail, shouldn't it?

Your second example is not equivalent of the first. In the second example, you certainly will fail to achieve something. This is a Bad Request.

The first example, well, you try to achieve something that is not forbidden by default - getting a record with a given id. And it happens to be the case that this record does not exist. This is a case where your item was Not Found.

iSpain17
  • 2,502
  • 3
  • 17
  • 26
0

You can use HttpResponseMessage to customize the response message. It allows setting status code, reason phrase, message, and many other things.