Skip to content

Returning TypedResult.Redirect become EmptyHttpResult #56714

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
1 task done
andrew-vdb opened this issue Jul 10, 2024 · 13 comments
Closed
1 task done

Returning TypedResult.Redirect become EmptyHttpResult #56714

andrew-vdb opened this issue Jul 10, 2024 · 13 comments
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc

Comments

@andrew-vdb
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I have simple minimal api that signout and doing redirect, it always return EmptyHttpResult for no reason
It is funny because i have login endpoint with minimal api that do RedirectResult but it works

Expected Behavior

returning TypedResult.Redirect should just work

Steps To Reproduce

internal static async Task Post(HttpContext httpContext)
{
if (httpContext.User.Identity.IsAuthenticated)
{
await httpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);
}
return TypedResults.Redirect("/");
}

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

@ghost ghost added the area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc label Jul 10, 2024
@andrew-vdb
Copy link
Author

Workaround (No idea why i need to do this work around, but it works)

internal static async Task<RedirectHttpResult> Post(HttpContext httpContext)
{
    if (httpContext.User.Identity.IsAuthenticated)
    {
        await httpContext.SignOutAsync(CookieAuthenticationDefaults.AuthenticationScheme);
        httpContext.Response.Redirect("/");
    }
    return TypedResults.Redirect("/");
}

@BrennanConroy
Copy link
Member

You're returning a Task<T> in the first case, but because you set the return type to Task minimal APIs will just see Task and have no idea that you actually returned an IResult type. In the second example, you changed the return type so now minimal APIs will see your IResult and handle it properly.

@andrew-vdb
Copy link
Author

Sorry, first sample will compile error, wrong copy paste

What make it work in workaround is response redirect, without that line, its not redirecting

@andrew-vdb
Copy link
Author

Screenshot

As you can see in debugger, the delegate return RedirectHttpResult but when it goes to dynamic, it become EmptyHttpResult

image

image

@BrennanConroy
Copy link
Member

Can you please share a minimal repro, because trying to reproduce this shows it working as expected.

app.MapPost("/p", async Task<RedirectHttpResult> (HttpContext httpContext) =>
{
    await Task.Yield();
    return TypedResults.Redirect("/");
}).AddEndpointFilter(async (c, n) =>
{
    var res = await n(c);
    return res;
});

@andrew-vdb
Copy link
Author

I will try, im on holiday at this moment, i have no access to laptop for a week something

@andrew-vdb
Copy link
Author

@BrennanConroy for some reason, that endpoint always running in this delegate, do you maybe know why the filter running on that delegate?
image

@BrennanConroy
Copy link
Member

Again, please share a minimal repro, just showing screenshots of our code isn't helpful.

@andrew-vdb
Copy link
Author

Took a while to create the repro but here you go
AspNet.EmptyEndpointRepro.zip

image

@BrennanConroy
Copy link
Member

Simplified repro:

var g = app.MapGroup("/test").AddEndpointFilter(async (c, next) =>
{
    var res = await next(c);
    return res;
});
g.MapGet("/red", H.Redirect);

public static class H
{
    public static async Task<RedirectHttpResult> Redirect(HttpContext h)
    {
        await Task.Delay(100);
        return TypedResults.Redirect("/"); //This redirect is the cause of EmptyHttpResult
    }
}

This is the same as #44316 where we wrote an analyzer that should be warning you about this pattern
image

TLDR; if you want the Task<T> FuncName(HttpContext) method format, you need to cast it to Delegate when passing it to Map* methods.

@andrew-vdb
Copy link
Author

Existing issue 2 years ago
#39956

PR that fixed it
#40235

PR that revert the fix
#42519

Another issue that discuss the revert
#45373

The reason to revert the change is to make it "reflection free"

Tbh, i do not get the "reflection free" in minimal api since AddEndpointFilterFactory use reflection, does this mean using AddEndpointFilterFactory is discouraged? (Tried to ask this in discussion but no one answer) if yes what is the alternative solution?

But
https://stackoverflow.com/questions/73426685/how-is-delegate-being-cast-to-requestdelegate-in-asp-net-core
All delegate in the end converted to RequestDelegate during "runtime"
Is this why first request very slow even using minimal api?
Then should we somehow write minimal api with RequestDelegate to avoid conversion during runtime which will make thing fast somehow

@BrennanConroy
Copy link
Member

"reflection free" is incorrect wording. They meant non-trim safe reflection which Map(..., Delegate) has.

Is this why first request very slow even using minimal api?

No, the first request is probably slow because all the generated code is created on first request.
#46313 talks about some of this.

@andrew-vdb
Copy link
Author

Thank you for your reply, i think there is not much we can do in this issue... I will see if I can create endpoint directly with RequestDelegate as its fast start up, the only problem is that all of my filter basically broken with RequestDelegate...

#46313 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Projects
None yet
Development

No branches or pull requests

2 participants