Skip to content
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

IResponseNegotiator not terminating response and falling through to ExecuteReturnAsync.WriteJsonResponseAsync #370

Open
toddsmith-adsk opened this issue Nov 23, 2024 · 8 comments

Comments

@toddsmith-adsk
Copy link

I found that the default middleware in .NET 9 Minimal API is executing the following code below. When using IResponseNegotiator if I don't return an IResult object from my method it falls through to the WriteJsonResponseAsync call and then throws an exception about headers being read-only.

https://github.com/dotnet/aspnetcore/blob/1770dcf4e81872395cc4d3b3b3efbaef91f8020a/src/Shared/RouteHandlers/ExecuteHandlerHelper.cs#L27

   public static Task ExecuteReturnAsync(object obj, HttpContext httpContext, JsonTypeInfo<object> jsonTypeInfo)
    {
        // Terminal built ins
        if (obj is IResult result)
        {
            return result.ExecuteAsync(httpContext);
        }
        else if (obj is string stringValue)
        {
            SetPlaintextContentType(httpContext);
            return httpContext.Response.WriteAsync(stringValue);
        }
        else
        {
            // Otherwise, we JSON serialize when we reach the terminal state
            return WriteJsonResponseAsync(httpContext.Response, obj, jsonTypeInfo);
        }
    }

So instead of returning

return response.Negotiate(data);

I have to wrap the response in an IResult wrapper so that it hits the 1st conditional block in ExecuteReturnAsync.

return Results.Extensions.Negotiated(response.Negotiate(data));
public static class NegotiatedResultExtensions
{
    public static IResult Negotiated(this IResultExtensions _, Task obj)
    {
        return new NegotiatedResult(obj);
    }

    private class NegotiatedResult : IResult
    {
        private readonly Task _item;
    
        public NegotiatedResult(Task item)
        {
            _item = item;
        }

        public Task ExecuteAsync(HttpContext httpContext)
        {
            return _item;
        }
    }
@jchannon
Copy link
Member

jchannon commented Nov 23, 2024 via email

@toddsmith-adsk
Copy link
Author

I assume this is an issue only for IAsyncEnumerable

This is for a regular non-IAsyncEnumerable response.

@jchannon
Copy link
Member

As far as I can see non IAsyncEnumerable is working fine.

For IAsyncEnumerable, Negotiate will just work with the default json negotiator, for example:

app.MapGet("/asyncenumerable", (HttpResponse res) =>
            {
                var enumerable = RangeAsync(10, 3);
                
                return res.Negotiate(enumerable);
            });

 static async IAsyncEnumerable<int> RangeAsync(int start, int count)
        {
            for (var i = 0; i < count; i++)
            {
                await Task.Delay(i);
                yield return start + i;
            }
        }

The above will return "[10,11,12]" as the body, if you want a negotiator to do something else then you can choose to do that yourself, for example,

public class AsyncEnumerableResponseNegotiator : IResponseNegotiator
{
    public bool CanHandle(MediaTypeHeaderValue accept)
    {
        return true;
    }

    public async Task Handle<T>(HttpRequest req, HttpResponse res, T model, CancellationToken cancellationToken)
    {
        var enumerable = (IAsyncEnumerable<int>)model;

        await foreach (var item in enumerable)
        {
            await res.WriteAsync(item.ToString(), cancellationToken);
        }
    }
}

@toddsmith-adsk
Copy link
Author

I'm using Negotiate to return MemoryPack serialization and not json. I'll try and make a small demo.

@toddsmith-adsk
Copy link
Author

Here is a demo of the issue

carter-memorypack-api

@jchannon
Copy link
Member

jchannon commented Nov 26, 2024 via email

@toddsmith-adsk
Copy link
Author

You tested both the / and /result endpoints and got the same result? The / endpoint is the one that should generate an error on my windows box running Visual Studio 2022.

@jchannon
Copy link
Member

jchannon commented Nov 27, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants