Don't return IEnumerable from action methods

December 13th 2024 ASP.NET Core LINQ

I already wrote about the potential dangers of using the IEnumerable interface in the past. But I recently encountered a bug related to it which I found worth sharing. It caused the exception filter in ASP.NET Core to not handle the exception thrown in the action method.

This is a simplified sample which reproduces the issue:

private static IEnumerable<WeatherForecast> GetForecasts()
{
    return Enumerable
        .Range(1, 5)
        .Select(index => new WeatherForecast
        {
            Date = DateOnly.FromDateTime(DateTime.Now.AddDays(index)),
            TemperatureC = Random.Shared.Next(-20, 55),
            Summary = Summaries[10] // throws IndexOutOfRangeException
        });
}

[HttpGet(Name = "GetWeatherForecastUnhandled")]
[Route("unhandled")]
public IEnumerable<WeatherForecast> GetUnhandled()
{
    return GetForecasts();
}

As marked in the code with a comment, the IEnumerable that generates the weather forecast throws an exception.

I have a simple exception filter registered to produce a custom response in case of an unhandled exception:

public class ExceptionFilter : IActionFilter
{
    public void OnActionExecuted(ActionExecutedContext context)
    {
        if (context.Exception != null)
        {
            context.Result = new ObjectResult(new { context.Exception.Message })
            {
                StatusCode = 500
            };
            context.ExceptionHandled = true;
        }
    }

    public void OnActionExecuting(ActionExecutingContext context) { }
}
builder.Services.AddControllers(options =>
{
    options.Filters.Add<ExceptionFilter>();
});

But despite that, the endpoint still returns the default plain text response, as demonstrated by the following passing test:

[Test]
public async Task CallUnhandledEndpoint()
{
    using var httpClient = factory.CreateClient();

    var response = await httpClient.GetAsync("/WeatherForecast/unhandled");

    response.Should().HaveServerError();
    response.Content.Headers.ContentType?.MediaType.Should().Be("text/plain");
}

To fix the bug, the IEnumerable sequence has to be evaluated/materialized before it is returned from the action method:

[HttpGet(Name = "GetWeatherForecastHandled")]
[Route("handled")]
public IEnumerable<WeatherForecast> GetHandled()
{
    return GetForecasts().ToArray();
}

The exception from this action method will be properly intercepted by the exception filter, and the endpoint will return a custom JSON error response, as demonstrated by the following passing test:

[Test]
public async Task CallHandledEndpoint()
{
    using var httpClient = factory.CreateClient();

    var response = await httpClient.GetAsync("/WeatherForecast/handled");

    response.Should().HaveServerError();
    response.Content.Headers.ContentType?.MediaType.Should().Be("application/json");
}

So, why is the exception not intercepted when thrown from the first action method? Because the LINQ expression returned by the GetForecasts() method isn't evaluated inside the action method. It's only evaluated during JSON serialization of the result, which happens after the action method completes. This can be clearly seen if you look at the stack trace of the exception:

at Internal.Runtime.CompilerHelpers.ThrowHelpers.ThrowIndexOutOfRangeException()
at ApiIenumerableErrors.Controllers.WeatherForecastController.<>c.<GetForecasts>b__1_0(Int32 index) in C:\Users\damir\Git\Blogposts\ApiIenumerableErrors\ApiIenumerableErrors\Controllers\WeatherForecastController.cs:line 27
at System.Linq.Enumerable.RangeSelectIterator`1.MoveNext()
at System.Text.Json.Serialization.Converters.IEnumerableDefaultConverter`2.OnWriteResume(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, WriteStack& state)
at System.Text.Json.Serialization.JsonCollectionConverter`2.OnTryWrite(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, WriteStack& state)
at System.Text.Json.Serialization.JsonConverter`1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)
at System.Text.Json.Serialization.JsonConverter`1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state)

The GetUnhandled() method is not present in the stack trace, which means that the exception wasn't really thrown from inside it. So the exception filter has already executed before the exception was thrown and therefore couldn't intercept the exception.

If you look at the stack trace of the exception throw in the second case, you will notice the GetHandled() method:

at Internal.Runtime.CompilerHelpers.ThrowHelpers.ThrowIndexOutOfRangeException()
at ApiIenumerableErrors.Controllers.WeatherForecastController.<>c.<GetForecasts>b__1_0(Int32 index) in C:\Users\damir\Git\Blogposts\ApiIenumerableErrors\ApiIenumerableErrors\Controllers\WeatherForecastController.cs:line 27
at System.Linq.Enumerable.RangeSelectIterator`1.Fill(Span`1 results, Int32 start, Func`2 func)
at System.Linq.Enumerable.RangeSelectIterator`1.ToArray()
at ApiIenumerableErrors.Controllers.WeatherForecastController.GetHandled() in C:\Users\damir\Git\Blogposts\ApiIenumerableErrors\ApiIenumerableErrors\Controllers\WeatherForecastController.cs:line 46
at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.SyncObjectResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<<InvokeActionMethodAsync>g__Logged|12_1>d.MoveNext()
at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<<InvokeNextActionFilterAsync>g__Awaited|10_0>d.MoveNext()

This time, the LINQ expression was evaluated by the ToArray() method called inside the action method. Since the exception was now actually thrown from inside the action method, it could be intercepted by the exception filter.

You can find a sample project in my GitHub repository. You can try out the behavior yourself either with the included .http file or with the integration tests in the accompanying test project.

The IEnumerable interface is great as it allows you to defer the execution of the underlying code, e.g., the LINQ expression. You can only do the potentially costly processing when (and if) you need the results. However, you need to be aware of that when writing code, otherwise you might be doing the costly evaluation multiple times or outside the code block you expect it to be happening.

Get notified when a new blog post is published (usually every Friday):

Copyright
Creative Commons License