Skip to content

Support for route handler filters in minimal APIs issue #41188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
axzxs2001 opened this issue Apr 14, 2022 · 2 comments · Fixed by #41253
Closed

Support for route handler filters in minimal APIs issue #41188

axzxs2001 opened this issue Apr 14, 2022 · 2 comments · Fixed by #41253
Assignees
Labels
bug This issue describes a behavior which is not expected - a bug. old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Priority:0 Work that we can't release without
Milestone

Comments

@axzxs2001
Copy link

axzxs2001 commented Apr 14, 2022

.net version is 7.0.100-preview.3.22179.4

The following code will work:

string SayHello(string name) => $"Hello, {name}!.";
app.MapGet("/hello/{name}", SayHello)
    .AddFilter(async (RouteHandlerInvocationContext context, RouteHandlerFilterDelegate next) =>
    {
        return await next(context);
    });

But the following code doesn't work:

app.MapGet("/hello/{name}", (string name) => $"Hello, {name}!.")
    .AddFilter(async (RouteHandlerInvocationContext context, RouteHandlerFilterDelegate next) =>
    {
        return await next(context);
    });

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

@TanayParikh TanayParikh added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Apr 14, 2022
@BrennanConroy BrennanConroy added the bug This issue describes a behavior which is not expected - a bug. label Apr 14, 2022
@BrennanConroy BrennanConroy added this to the 7.0-preview4 milestone Apr 14, 2022
@BrennanConroy BrennanConroy self-assigned this Apr 14, 2022
@martincostello
Copy link
Member

I've just come across this too, but with .AddFilter<T>(). You can repro using the unit tests of the aspnetcore-41188 branch of this repo.

For example, the filter is added here:

// Get all Todo items
builder.MapGet("/api/items", async (
    ClaimsPrincipal user,
    ITodoService service,
    CancellationToken cancellationToken) =>
    {
        return Results.Ok(await service.GetListAsync(user.GetUserId(), cancellationToken));
    })
    .AddFilter<RateLimitFilter>()
    .ProducesProblem(StatusCodes.Status429TooManyRequests)
    .RequireAuthorization();

The NullReferenceException is thrown when next is awaited here:

var rateLimit = GetRateLimitPolicy(userId, operation);

try
{
    return await rateLimit.ExecuteAsync(async () => await next(context));
}
catch (RateLimitRejectedException ex)
{
    return Results.Extensions.RateLimited(ex.RetryAfter);
}
 Can_Manage_Todo_Items_With_Api
   Source: ApiTests.cs line 46
   Duration: 14 ms

  Message: 
System.Net.Http.HttpRequestException : Response status code does not indicate success: 500 (Internal Server Error).

  Stack Trace: 
HttpResponseMessage.EnsureSuccessStatusCode()
HttpClientJsonExtensions.GetFromJsonAsyncCore[T](Task`1 taskResponse, JsonSerializerOptions options, CancellationToken cancellationToken)
ApiTests.Can_Manage_Todo_Items_With_Api() line 52
--- End of stack trace from previous location ---

  Standard Output: 
[2022-04-15 14:04:00Z] dbug: AspNet.Security.OAuth.GitHub.GitHubAuthenticationHandler[1]
      HandleChallenge with Location: https://localhost:443/sign-in-github?code=3dc3e33a-c5d8-4413-9f7b-cb546fcab89b&state=CfDJ8E9WBL1MnW1Nn2fpw_o_MnEBnqeKTHVhuSSQawRf_Ipcj9vIb8GU5SbWI9heULZrYC-ccdmtJwWVo4v9JjSS-Sg1f62oqJu4KB7ZQ4SwQNJOLP0lmLLkgeYeaaS6m1sZcFcdXfrRwUZ_Q6jtMtNhnXlUVXI7hBMnZfb2oxKpnw9_Vl5KsMIYsgUGzdLqaL6RpH_ZattA2dJsd_GLT3PU4yI; and Set-Cookie: .AspNetCore.Correlation.FLc18UATvq8WEyiZUwuuOYK1E8Zz8J1WJLHJvXdBRdg=N; expires=Fri, 15 Apr 2022 14:19:00 GMT; path=/sign-in-github; secure; samesite=none; httponly.
[2022-04-15 14:04:00Z] info: AspNet.Security.OAuth.GitHub.GitHubAuthenticationHandler[12]
      AuthenticationScheme: GitHub was challenged.
[2022-04-15 14:04:00Z] fail: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[1]
      An unhandled exception has occurred while executing the request.
System.NullReferenceException: Object reference not set to an instance of an object.
   at lambda_method81(Closure, RouteHandlerInvocationContext)
   at TodoApp.RateLimitFilter.<>c__DisplayClass7_0.<<InvokeAsync>b__0>d.MoveNext() in C:\Coding\martincostello\polly-rate-limiting\src\TodoApp\RateLimitFilter.cs:line 31
--- End of stack trace from previous location ---
   at Polly.RateLimit.AsyncRateLimitEngine.ImplementationAsync[TResult](IRateLimiter rateLimiter, Func`3 retryAfterFactory, Func`3 action, Context context, CancellationToken cancellationToken, Boolean continueOnCapturedContext)
   at Polly.AsyncPolicy.ExecuteAsync[TResult](Func`3 action, Context context, CancellationToken cancellationToken, Boolean continueOnCapturedContext)
   at TodoApp.RateLimitFilter.InvokeAsync(RouteHandlerInvocationContext context, RouteHandlerFilterDelegate next) in C:\Coding\martincostello\polly-rate-limiting\src\TodoApp\RateLimitFilter.cs:line 31
   at Microsoft.AspNetCore.Http.RequestDelegateFactory.ExecuteObjectReturn(Object obj, HttpContext httpContext)
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.<Invoke>g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger)
   at Microsoft.AspNetCore.Authorization.Policy.AuthorizationMiddlewareResultHandler.HandleAsync(RequestDelegate next, HttpContext context, AuthorizationPolicy policy, PolicyAuthorizationResult authorizeResult)
   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.StatusCodePagesMiddleware.Invoke(HttpContext context)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

@BrennanConroy
Copy link
Member

I've just come across this too, but with .AddFilter<T>().

Right, because the issue is with the generated code and how we handle inline delegates vs. methods. So the workaround until preivew4 is to move your handler code into a method.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue describes a behavior which is not expected - a bug. old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels Priority:0 Work that we can't release without
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants