Throwing Exceptions Correctly

I have the following code

    public HttpResponseMessage AddDataToDatabase([FromBody] Data data)
    {
        try
        {
            var token = _tokenService.GetToken(Request.Headers.Authorization);

            if (_pService.Permission(_tokenService.GetUserId(token), "Enable_Data"))
            {
                _noteService.AddData(data, _tokenService.GetUserId(token));
                return Request.CreateResponse(HttpStatusCode.OK, "Data has been added to the case.");
            }

            throw new HttpResponseException(
                Request.CreateErrorResponse(HttpStatusCode.Forbidden, "Cannot add data because you don't have permission."));

        }
        catch (Exception exception)
        {
            if (exception is SqlException)
            {
                throw new HttpResponseException(
                    Request.CreateErrorResponse(HttpStatusCode.ServiceUnavailable, exception));
            }

            if (exception is ArgumentException)
            {
                throw new HttpResponseException(
                    Request.CreateErrorResponse(HttpStatusCode.Conflict, exception.Message));
            }

            throw;
        }
    }

I want to catch the Forbidden Exception like other exception in the catch block but not sure how to do that. The way I am returning Forbidden exception right now breaks unit tests which return SqlException and ArgumentExpception.

How can I throw exceptions correctly?

2 answers

  • answered 2018-01-11 19:40 Jonathan Wood

    By far, the easiest way is to simply do it like this.

    catch (Exception exception)
    {
        throw;
    }
    

    Unlike your code, this code also handles unanticipated exceptions instead of just SqlException and ArgumentException.

    But why are you doing this? If you want the exception to propagate out to the caller, then simply remove the try...catch block in this method. Then just throw the desired exception in your code.

    If you are capturing and converting third-party exceptions, the recommended approach is as follows.

    catch (SqlException exception)
    {
        throw new HttpResponseException(
            Request.CreateErrorResponse(HttpStatusCode.ServiceUnavailable, exception));
    }
    catch (ArgumentException exception)
    {
        throw new HttpResponseException(
            Request.CreateErrorResponse(HttpStatusCode.Conflict, exception.Message));
    }
    

  • answered 2018-01-11 19:40 Pac0

    you should probably replace your if logic in your catch by using more selective catch :

        catch (SqlException exception)
        {
            throw new HttpResponseException(
                Request.CreateErrorResponse(HttpStatusCode.ServiceUnavailable, exception));
        }
        catch (ArgumentException exception) 
        {
            throw new HttpResponseException(
                Request.CreateErrorResponse(HttpStatusCode.Conflict, exception.Message));
        }
    

    The uncaught exceptions will simplypass through (as when you you use throw;).

    If you want to catch another type of exception, just add it to the list.

    However, it is not very clear how your tests are broken. You should provide more details at this level if you want more help.