Is throwing an Exception bad practice when returning error messages from WebApi to Client?

Question

Currently my WebApi throws an exception in order to display a message on the client side if any error occurred. This has proven convenient because I can wrap the exception in a BadRequest() response and it's nicely handled on the client side. I'm just wondering if this bad practice and weather I should be returning an object with an error flag + message or something like that instead. I would then have to change my client side code to deal with this...

Ex. Api

[HttpPost("Login")]
        [AllowAnonymous]
        public async Task<IActionResult> LoginAsync([FromBody]LoginJS credentials)
        {
            try
            {
                var result = await _accountRepository.AuthenticateAsync(credentials.Username, credentials.Password);

                if (result.Success)
                    return Ok(result);
                else
                    return BadRequest("Login Failed!");
            }
            catch (Exception e)
            {
                return BadRequest(e.Message);
            }
        }

public async Task<AuthenticationResponse> AuthenticateAsync(string username, string password)
        {
            try
            {
                var user = await _userManager.FindByNameAsync(username);
                if (user != null)
                {
                    var emailVerified = await _userManager.IsEmailConfirmedAsync(user);
                    if (!emailVerified)
                        throw new Exception("Email Address has not yet been verified!");

                    var signInResult = await _signInManager.PasswordSignInAsync(user, password, false, false);
                    if (signInResult.Succeeded)
                    {
                        var roles = await _userManager.GetRolesAsync(user);

                        if (!roles.Any())
                            throw new Exception("Sorry your account has not been setup yet.");

                        var authenticationResponse = await GenerateTokenAsync(user, roles.ToList());

                        return authenticationResponse;
                    }                       
                }

                throw new Exception("Login Failed!");

                //IS THIS THE BETTER WAY?
                //return new AuthenticationResponse
                //{
                //    Errors = new[] { "Login Failed" }
                //};
            }
            catch (Exception e)
            {
                _logger.LogError(e.Message);
                throw e;
            }
        }

score 1 · Answer 1 · answered May 11 '20 at 21:54

Exceptions should be exceptional. This particular case is not exceptional. Exceptions can be costly from a performance point of view. Here is another question that addresses this.

Returning the AuthenticationResponse in your example is a preferred approach. You can also check out the TryXXX pattern which is likely not useful here given your method is async.

Is throwing an Exception bad practice when returning error messages from WebApi to Client?

1 Answers1