Error handling in a REST API

Question

I have implemented a small REST API using WebApi 2 and NHibernate for ORM.

When non-existent id is supplied, NHibernate will throw an InvalidOperationException. So I thought I would rethrow a custom exception and then handle all the error logging and returning (there is some logic to different errors/environments) to the client in one place - the ApiExceptionFilterAttribute.

Basically the question is : is this okay or I'm using the custom exception to handle normal flow? How can I improve this?

Example EntityController method:

// GET: api/Entity/5
[ApiExceptionFilter]
public IHttpActionResult Get(int id)
{
    try
    {
        return Ok(
            _mapper.Map<Entity, EntityDTO>(
                _repository.GetById<Entity>(id)));
    }
    catch (InvalidOperationException ex)
    {
        throw new NotFoundException(ex);
    }
}

Repository

public T GetById<T>(int id) where T : Entity
{
    return _session.Value.Query<T>().First(x => x.Id == id);
}

NotFoundException:

public class NotFoundException : ApiException
{
    private static readonly string CustomMessage = "Nothing found! :( Try refining the arguments";

    public NotFoundException(System.Exception ex)
        : base(HttpStatusCode.NotFound, CustomMessage, ex)
    { }
}

And finally, ApiExceptionFilterAttribute

public class ApiExceptionFilterAttribute : ExceptionFilterAttribute
{
    public override void OnException(HttpActionExecutedContext context)
    {
        _logger.Log(context.Exception);
        var exception = context.Exception as ApiException;
        if (exception != null)
        {
            context.Response = context.Request.CreateErrorResponse(exception.StatusCode, exception.Message);
        }
// if not dev or test environment, non-user-friendly exceptions will be returned as generic 500 error
#if !(DEBUG || TEST) 
        else
        {
            context.Response = context.Request.CreateErrorResponse(HttpStatusCode.InternalServerError, "Something went wrong");
        }
#endif
    }
}

Answer 1

You're really dragging your Web API through a performance drag by throwing an exception -- especially for something as trivial as a GetById returning a null. You could deal with the non-existence of the id upfront and save yourself the trouble...

public IHttpActionResult Get(int id)
{
    var entity = _repository.GetById<Entity>(id);
    if (entity == null)
    {
        // log here...
        // then I would recommend you return a meaningful statuscode of 4xx instead
        // of the 500 (internal error) you are doing at the moment.
    }
    else
    {
        // do your mapping and return the 200 (OK) status code on a success
    }
}

then change your repository to look like this:

public class Repository<T>
    where T : IEntity
{   
    public T GetById(int id)
    {   
        return _session.Value.Where(x => x.Id == id).FirstOrDefault();
    }
}