4

I have a C# WebAPI project with controller/service/repository structure. Most of my controller methods call a service method which then calls a repository method to access the DB. If one of the parameters from the controller method is null, then an ArgumentNullException is thrown which returns the following from my HttpGlobalExceptionFilter.cs:

{
    "code": 400,
    "messages": [
        "Value cannot be null. (Parameter 'year')"
    ],
    "developerMessage": null
}

However, what about exceptions thrown at the controller level? For example, this is my controller method:

public async Task<ActionResult<Data>> GetDataByYearAsync([FromRoute] string DataId)
{
    int dId;
    DataType dType;
    try
    {
        string[] dataParts = DataId.Split('-');
        dType = (DataType)Enum.Parse(typeof(DataType), dataParts[0]);
        dId = int.Parse(dataParts[1]);
    }
    catch (Exception ex)
    {
        return BadRequest();
    }

    Data result = await _dataService.FindData(dType, dId).ConfigureAwait(false);

    if (result == null)
        return NotFound();

    return result;
}

If the DataId parameter is null then I get the following exception:

System.NullReferenceException: 'Object reference not set to an instance of an object.'

thrown at this line:

string[] dataParts = DataId.Split('-');

This leads to the following:

{
    "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
    "title": "Bad Request",
    "status": 400,
    "traceId": "00-a0aaaa0aa000000aa00a00a00000a0a0-a0a000000000a000-00"
}

How should I return the error/exception at the controller level?

1
  • The BadRequest() method can, depending on your .Net Core version, accept various objects (2.2, for example, can accept a string message). Are you able to use that to provide a more readable error message? Commented Sep 8, 2022 at 18:43

3 Answers 3

1

You can throw the exception with the bad Request

Return BadRequest(ex.Message)

Sign up to request clarification or add additional context in comments.

1 Comment

Your answer could be improved with additional supporting information. Please edit to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers in the help center.
1

Seems the difference is that your attempt to get data from the database is not wrapped in a try/catch block so any exceptions go straight to the global handler. But the NullReferenceException on string[] dataParts = DataId.Split('-'); is being caught and handled by returning BadRequest.

So you could just remove the try/catch and allow any exception to be handled globally:

public async Task<ActionResult<Data>> GetDataByYearAsync([FromRoute] string DataId)
{
    int dId;
    DataType dType;

    string[] dataParts = DataId.Split('-');
    dType = (DataType)Enum.Parse(typeof(DataType), dataParts[0]);
    dId = int.Parse(dataParts[1]);

    Data result = await _dataService.FindData(dType, dId).ConfigureAwait(false);

    if (result == null)
        return NotFound();

    return result;
}

Or you include the message in the returned BadRequest:

public async Task<ActionResult<Data>> GetDataByYearAsync([FromRoute] string DataId)
{
    int dId;
    DataType dType;
    try
    {
        string[] dataParts = DataId.Split('-');
        dType = (DataType)Enum.Parse(typeof(DataType), dataParts[0]);
        dId = int.Parse(dataParts[1]);
    }
    catch (Exception ex)
    {
        return BadRequest(ex.Message);
    }

    Data result = await _dataService.FindData(dType, dId).ConfigureAwait(false);

    if (result == null)
        return NotFound();

    return result;
}

You could also clean up a little by handling your data errors in the controller too:

public async Task<ActionResult<Data>> GetDataByYearAsync([FromRoute] string DataId)
{
    try
    {
        string[] dataParts = dataId.Split('-');
        DataType dType = (DataType)Enum.Parse(typeof(DataType), dataParts[0]);
        int dId = int.Parse(dataParts[1]);

        Data result = await _dataService.FindData(dType, dId).ConfigureAwait(false);

        if (result == null)
            return NotFound();

        return result;
    }
    catch (Exception ex)
    {
        return BadRequest(ex.Message);
    }
}

But you also have various potential issues with parsing the passed string that are well-known so you could reduce potential exceptions by ensuring you are checking the input correctly:

Note: method arguments should typically be camelCase.

public async Task<ActionResult<Data>> GetDataByYearAsync([FromRoute] string dataId)
{
    // check whether the argument is null before trying to use it.
    if (string.IsNullOrEmpty(dataId))
        return BadRequest($"Value cannot be null: {nameof(dataId)}");

    string[] dataParts = DataId.Split('-');
    // ensure that the string is in the expected format before progressing.
    if (dataParts.Length != 2)
        return BadRequest($"{nameof(dataId)} is invalid");

    // Use Enum.TryParse to ensure no exception is thrown for a non-matching string.
    if (!Enum.TryParse(dataParts[0], out DataType dType))
        return BadRequest($"{nameof(dataId)} unknown DataType");

    // Use int.TryParse to elegantly return if the value is invalid.
    if (!int.TryParse(dataParts[1], out int dId))
        return BadRequest($"{nameof(dataId)} could not parse integer value");

    // this will still throw and be handled globally if there are any issues.
    Data result = await _dataService.FindData(dType, dId).ConfigureAwait(false);

    if (result == null)
        return NotFound();

    return result;
}

Comments

0

Assume that you use ASP.NET 6, the non-nullable property must be required in ASP.NET 6, otherwise the model validation will fail.

Just make your parameter nullable by using ?:

public async Task<ActionResult<Data>> GetDataByYearAsync([FromRoute] string? DataId)

Or another way, remove <Nullable>enable</Nullable> from your project file to globally ignore Required validation.

If your parameter contains model data and in this model contains other model validation, you can remove [ApiController] to skip validation before hitting the action.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.