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?

Bohdan Stupak · Accepted Answer · 2019-06-05 08:36:45Z

3

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);
}
```

answered Jun 5, 2019 at 8:36

Bohdan Stupak

5583 silver badges10 bronze badges

\$\begingroup\$ I like that method to avoid exceptions. I will try it, thanks! \$\endgroup\$

metadon789
– metadon789

2019-06-05 10:21:17 +00:00
Commented Jun 5, 2019 at 10:21
\$\begingroup\$ @Bohdan Stupak how would you handle validation failures? Ignore silently, wrap as some kind of response or perhaps throw an exception ? \$\endgroup\$

dfhwze
– dfhwze

2019-06-05 11:59:59 +00:00
Commented Jun 5, 2019 at 11:59
\$\begingroup\$ 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. \$\endgroup\$

Bohdan Stupak
– Bohdan Stupak

2019-06-05 12:51:31 +00:00
Commented Jun 5, 2019 at 12:51

Add a comment |

Stack Exchange Network

Exception, errors handling, best practice in WebApi Core

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Exception, errors handling, best practice in WebApi Core

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions