How to handle server errors with layered architecture in web API controller

Question

EDIT:

I just realized that SaveChangesAsync returning 0 doesn't mean that it failed, entity framework will always throw exception when something fails so the check if SaveChanges == 0 is redundant! Save changes should always return 1 in the example below, if something fails then exception will be thrown.

However there are cases when something else is used and it's not entity framework so is this question for.

---

Servers can fail, when putting all my data access code in controllers I can handle it this way:

[HttpPost]
public async Task<ActionResult<Item>> CreateAsync([FromBody] Item item)
{
    await _dbContext.AddAsync(item);
    if (await _dbContext.SaveChangesAsync() == 0)
    {
        return StatusCode(StatusCodes.Status500InternalServerError);
    }
    return CreatedAtAction(nameof(GetAsync), new { id = item.Id }, item);
}

How should I handle it when my data access is encapsulated in a service layer?

public class ItemsService
{
    public async Task<Item> CreateAsync(Item item)
    {
        await _dbContext.AddAsync(item);
        if (await _dbContext.SaveChangesAsync() == 0)
        {
            return null;
        }
        return item;
    }
}

Then it would be used like that:

[HttpPost]
public async Task<ActionResult<Item>> CreateAsync([FromBody] Item item)
{
   // model state validation skipped for the sake of simplicity,
   // that would return BadRequest or some more valuable information
    var item = await _itemsService.CreateAsync(item);
    if (item == null)
    {
        return StatusCode(StatusCodes.Status500InternalServerError);
    }
    return CreatedAtAction(nameof(GetAsync), new { id = item.Id }, item);
}

Maybe this works for fine creating because there are only 2 status codes but let's consider updating where there can be more than 2 possible errors like:

• Not found (404)
• Internal server error(500)
• Ok (200)

Code without services:

[HttpPut("{id}")]
public async Task<ActionResult<Item>> UpdateAsync(int id, [FromBody] Item itemToUpdate)
{
    var item = await _dbContext.Items.FindAsync(id);
    if (item == null)
    {
        return NotFound();
    }

    // update item with itemToUpdate
    //...
    await _dbContext.Update(item);
    if (await _dbContext.SaveChangesAsync() == 0)
    {
        return StatusCode(StatusCodes.Status500InternalServerError);
    }
    return item;
}

Now with services this can't be properly handled:

public class ItemsService
{
    public async Task<Item> UpdateAsync(Item updateItem)
    {
        var item = await _dbContext.Items.FindAsync(id);
        if (item == null)
        {
            return null;
        }
        //change some properties and update
        //...
        _dbContext.Items.Update(item);
        if (await _dbContext.SaveChangesAsync() == 0)
        {
            // what now?
        }
        return item;
    }
}

because it always returns null and there's no way to tell if the item was not found or the saving failed.

How can I handle it properly?

Note: I didn't add DTO or anything like that to keep this example simple.

Answer 1

Your service is responsible for catching all the exceptions it knows how to handle and handles those. All the other exceptions should be reported throught some kind of IServiceResult<T> providing both bool IsSuccessful and AggregatedException Exceptions { get; }. So rather than throw you let the highest level to decide how to react. It's actually the best approach from many points of view, including: purity of your functions (which is a key concept for easy parallezation, isn't it important for server?), clean and maintanable code (consumers of the service should not know/care that much: they just verify whether result is successful and consume the result; otherwise dig into aggreaged exception; might make sense to implement your own type of exception providing methods suitable for needs of your project), ability to procceed on higher-order-functions, which are responsible for actual error handling; take rX as a popular example: .OnError(Action<Exception> handler). There are much more than this, however I don't like to make the answer longer that it needs to be.

As a side note. Make sure you read an introductory lever articles about Maybe and Either monads from Haskell programming language; in case you'd prefer F# more, you can try to seek for Some and Either respectively. There you can find useful methods like .Map(...) and see nice ways to combine/aggregate those in efficient way.

Good luck.

Answer 2

The code with the service is far better. The controller sould delegate business logic to the service layer.

To make your implementation of the service layer more better, you should:

• throw custom exceptions when something wrong happened in your service. Example : throw ItemNotFoundExcption instead of returning null. ItemUpdateException when updates failed.
• in your controller you need to catch all possible custom exceptions that might happenned and do what need to be done like returning NotFound() for ItemNotFoundException and BadRequest() for ItemUpdateException.
• the latter point can be made through custom action filter which helps you make your controller's actions more lean.