Ndtyjky

I've read (here) that it's not a good idea to do Try/Catch blocks and avoid exceptions in a Web API setting. However, if you want to catch and log errors as they happen in your app... wouldn't a try/catch be the best way to go about things? Here's an example of what I've done in my application - I'm curious if anyone has a better way. My ErrorHander class saves the error to the database and emails administrators the details of the error.

Controller

namespace MyApp.Controllers

{

    [Route("api/[controller]")]

    public class AuthController : Controller

    {

        private readonly IAuthRepository _repo;

        private readonly IErrorHandler _errorHandler;



        private AuthController(IAuthRepository repo, IErrorHandler errorHandler) {

            _errorHandler = errorHandler;

        }



        [Authorize]

        [HttpPut("changePassword")]

        public async Task<IActionResult> ChangePassword(

            [FromBody] UserForChangePasswordDto userForChangePasswordDto)

        {

            var userFromRepo = await _repo.Login(userForChangePasswordDto.Username,

                    userForChangePasswordDto.OldPassword, null);



            try

            {

                //logic to update user's password and return updated user



                return Ok(new { tokenString, user });

            }

            catch (Exception ex)

            {

                // emails error to Admin, saves it to DB, then returns HttpStatusCode

                return await _errorHandler.HandleError(

                    HttpStatusCode.InternalServerError, Request, new Error

                {

                    Message = ex.Message,

                    StackTrace = ex.StackTrace,

                    User = userFromRepo != null ? userFromRepo.Username : "Invalid User"

                });

            }

        }

    }

}

I have a few recommendations:

1. Avoid try..catch entirely when possible. For example, use methods like TryParse, TryCreate, etc, instead of methods that throw exceptions. The same goes with the *OrDefault LINQ methods, e.g. always use SingleOrDefault rather than Single, etc. Basically, if there's a way to avoid throwing exceptions at all, use that.




2. When you do need to handle exceptions do so in your abstractions, not your app code. For example, presumably, the //logic to update user's password and return updated user line is using your IAuthRepository. Your repo's method you're calling should not throw an exception itself. If an exception is thrown by the code inside, catch it there and handle it there. Your method itself then can return something like a boolean that you can use to determine whether the operation was successful or not and branch accordingly. Although, the actual error handling logic (email admin, etc.) is contained in your IErrorHandler abstraction, your app code still is working with that and has a dependency on that, which is unnecessary domain knowledge.



3. 
   

   When you catch exceptions, catch specific exceptions. You should know exactly what you're catching and why. Catching something as generic as Exception is generally a sign of lazy coding. You may not even know if an exception could be returned at all, but you're still using a performance-draining try..catch block. In some scenarios it might be appropriate to catch any possible exception, but then, you should always re-throw the exception. Swallowing every possible exception is a huge no-no. If you feel you cannot re-throw the exception, then you should be targeting a specific exception instead.

   
   
   

   try
   
   {
   
       // do something
   
   }
   
   catch (Exception e)
   
   {
   
       // log exception
   
       throw;
   
   }
   
   

   
   



4. Separate your error handling from your request processing. Your app simply needs to return a response for the request. It should not care about stuff like emailing your admins and such. You need some sort of record that something went wrong, understandably, but simple logging is sufficient for that, and much lighter-weight. If you want to email admins, you can set up a separate service to monitor your logs and gen emails when appropriate, which takes that process appropriately out of band.

If you want to catch exceptions and do some general things with them, like you're doing in your example, then your way is not the good way in ASP.NET :)

When you have multiple controllers and multiple actions inside each controller, you're going to have a lot of code duplication.

ASP.NET Core allows you to create a Filter where you can put all that kind of logic in. It means that you'll have only one place to handle uncaught exceptions.
You should definetely have a look at the possibilities that are provided with IExceptionFilter