Exception, errors handling, best practice in WebApi Core

Question

I'm making my WebApi project.

My logic is, the controller didn't see data. It only triggers a service that returns objects to it. My simple method from TaskService looks like this:

public Task TakeTaskByUser(int taskId, string userId)
{
 var task = this.GetItem(taskId);
 if (task != null)
 {
 if (task.ApplicationUserId != null)
 {
 throw new TaskTakenByAnotherUserException();
 }
 task.ApplicationUserId = userId;
 task.StartTime = DateTime.Now;
 this.context.Entry(task).State = EntityState.Modified;
 this.context.SaveChanges();
 return task;
 }
 return null;
}

As you can see, in this method I try to find Task and attach Foreign Key to User by his UserId. If Task is already taken by another user, I throw a custom exception.

In my controller, Action looks like this:

[HttpPut("{taskId}/{userId}")]
[Authorize(Roles = "Developer, Manager")]
public IActionResult TakeTaskByUser([FromRoute] int taskId, [FromRoute] string userId)
{
 try
 {
 var task = this.taskService.TakeTaskByUser(taskId, userId.ToString());
 if (task != null)
 {
 return this.Ok(task);
 }
 return this.NotFound();
 }
 catch (TaskTakenByAnotherUserException)
 {
 return this.ValidationProblem();
 }
}

Is using a try-catch block in a controller good practice? Is using any exceptions in WebApi good, or I should use the method posted in this article (I like Level 1, seems easy and fun)?

How do I deal with errors that I know that can appear when someone sends a request?

Answer 1

Exceptions are pretty costly so as a general rule of thumb they should be avoided. Especially when we talk about REST endpoint where response time is crucial.

Also, to my taste you're messing up command-query separation principle in your code. I would have rewritten it roughly like this

[HttpPut("{taskId}/{userId}")]
[Authorize(Roles = "Developer, Manager")]
public IActionResult TakeTaskByUser([FromRoute] int taskId, [FromRoute] string userId)
{
 var task = this.taskService.GetTask(taskId);
 if (task == null)
 return this.NotFound();
 var taskValidationResult = this.taskValidator.Validate(task);
 if (!taskValidationResult.IsSuccess)
 {
 //handle validation failure
 }
 task = this.taskService.TakeTaskByUser(task, userId);
 return this.Ok(task);
}
```

• metadon789: I like that method to avoid exceptions. I will try it, thanks!
• dfhwze: @Bohdan Stupak how would you handle validation failures? Ignore silently, wrap as some kind of response or perhaps throw an exception ?
• Bohdan Stupak: I would return some kind of response to be consistent with return this.NotFound() couple lines above. I've omitted the code because the mapping of validation result to response code might be verbose and irrelevant for the case.