-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
.Net 6 Preview 7 - Cannot set reponse headers before returning IAsyncEnumerable in controller #35364
Comments
Thanks for filing this issue because I got to learn something here today as well 😄. When you make an IAsyncEnumerable method your method doesn't run until the consumer calls using System.Text.Json;
await JsonSerializer.SerializeAsync(Console.OpenStandardOutput(), AsyncRange0(0, 1000));
async IAsyncEnumerable<int> AsyncRange0(int start, int end)
{
for (int i = start; i < end; i++)
{
yield return i;
await Task.Delay(500);
}
} If you set a break point on the first line of AsyncRange0 and observe the callstack you'll see that we're deep in the middleware of the serializer.
Now consider what happens when we change method to be not async: using System.Text.Json;
await JsonSerializer.SerializeAsync(Console.OpenStandardOutput(), AsyncRange0(0, 1000));
IAsyncEnumerable<int> AsyncRange0(int start, int end)
{
async IAsyncEnumerable<int> Range()
{
for (int i = start; i < end; i++)
{
yield return i;
await Task.Delay(500);
}
}
return Range();
} Now we set a break point at the entry, we're being called before the call to SerializeAsync
This is by design. |
@pranavkm I don't think this has been answered |
@arjennienhuis didn't #35364 (comment) have a sufficient explanation of this? |
@pranavkm I'm confused as well. I still don't see why the response headers shouldn't be editable before yielding the result back. |
I think that comment is an explanation of why it's broken. It doesn't explain how to work around the problem. |
@davidfowl I think the state machine for a Task or an IAsyncEnumerable are very similar. The middleware of asp.net that handles a Task calls into the state machine before flushing the headers. I think that should also be done for IAsyncEnumerable. If this is the application: async Task<(string h, string b1, string b2)> ExternalApi()
{
await Task.Delay(50);
return ("header=value", "bo", "dy");
}
async Task<string> M1()
{
var (header, b1, b2) = await ExternalApi();
WriteLine($"Set headers: {header}");
return b1 + b2;
}
async IAsyncEnumerable<string> M2()
{
var (header, b1, b2) = await ExternalApi();
WriteLine($"Set headers: {header}");
yield return b1;
yield return b2;
} I think M1 and M2 should be handled the same by the middleware. So instead of doing this: async Task HandleTask_1(Func<Task<string>> m)
{
WriteLine("Flush headers");
WriteLine(await m());
}
async Task HandleIAE_1(Func<IAsyncEnumerable<string>> m)
{
WriteLine("Flush headers");
await foreach (var s in m())
Write(s);
WriteLine();
} the middleware should do this: async Task HandleTask_2(Func<Task<string>> m)
{
var s = await m();
WriteLine("Flush headers");
WriteLine(s);
}
async Task HandleIAE_2(Func<IAsyncEnumerable<string>> m)
{
bool flushed = false;
void flush()
{
if (flushed) return;
WriteLine("Flush headers");
flushed = true;
}
await foreach (var s in m())
{
flush();
Write(s);
}
flush();
WriteLine();
} |
The second code snippet is the workaround. In order for this to make sense you have to understand how an AsyncEnumerable works. A bit like an Enumerable, it will yield (hence the keyword) each element when called. You won't know how many elements there even are before calling the method, otherwise it would just be a list. The use case is to stream elements and if you think of it as a stream then it might also make more sense than thinking of it as an "enumerable" (the naming is a bit confusing in .NET). So, if you have an async IAsyncEnumerable then the internal code of that method will not be executed/materialised before it's actually being called, otherwise it would behave like a list and not like a stream. Does that make sense? That's what David was trying to explain, the compiler creates a state machine and executes the code when it's actually being requested during runtime. So if that is being called from a method which writes the elements straight to the response stream then at that point the headers have already been flushed and you won't be able to set/edit them anymore. In the workaround you extract your async enumerable generator into a nested method and then return that method from within the parent method which isn't an async method and doesn't await the IAsyncEnumerable itself. You'll be able to do whatever you want before the return statement: |
I think the example would be even clearer if the for loop was replaced with a while loop, so it looks even less like a list but more like a stream. Example: using System.Text.Json;
await JsonSerializer.SerializeAsync(Console.OpenStandardOutput(), AsyncRange0(0, 1000));
IAsyncEnumerable<int> AsyncRange0(int start, int end)
{
async IAsyncEnumerable<int> Range()
{
while (DateTime.Now < new DateTime(2021, 8, 20))
{
yield return i;
await Task.Delay(500);
}
}
return Range();
} An Enumerable or IAsyncEnumerbale could theoretically return you a never ending stream. In this case it would return you data for 3 days at which point it would stop. This example really demonstrates that the compiler has to create a state machine for the generator and only execute it when invoked during runtime. So, if the generator gets called from another method which writes each element directly to a HTTP response stream then you can see how this would naturally happen AFTER headers were flushed, which is what is the case in the original issue here. |
@dustinmoris Thanks for the explanation, I understand the issue now 👍 |
@dustinmoris that workaround doesn't work if the headers need to be set after an async call:
It's also strange that that workaround is not required for a normal async method. It shouldn't be required for an IAsyncEnumerbale either. |
This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes. See our Issue Management Policies for more information. |
@pranavkm would you consider reopening this issue? I don't see a proper workaround. |
The workaround was posted by me and @dustinmoris. Make another method that isn't async, use that as your controller method. Then it'll give you a chance to set headers: [HttpGet]
public IAsyncEnumerable<WeatherForecastData> Get(CancellationToken cancellationToken)
{
Response.Headers.Add("Test", results.Name);
IAsyncEnumerable<WeatherForecastData> Enumerate()
{
var results = await _dataService.GetData();
await foreach (var result in results.Data.WithCancellation(cancellationToken))
{
yield return result;
}
}
return Enumerate();
} |
That doesn't compile. results is out of scope there. Even if it was in scope, it's impossible to get a value there that needs await. A workaround would be to await it "synchronously". But that would be less than ideal. |
Everything is a normal async method. The difference is in the return type. What is strange is that you want it to behave like a List but keep using an AsyncEnumerable. Please read the explanation above again. An IAsyncEnumerable is essentially a generator. If you don't want a generator then you should change your code to something else, but if you want a generator then you hopefully agree that a generator will behave like a generator.
David didn't write your entire application for you. How is anyone supposed to know what you need? The workaround is there, now you need to compose an application with this knowledge that fits your use case. If you want to await something then you need to call it from an async method. Look: using System.Text.Json;
IAsyncEnumerable<int> GetAsyncEnumerable(DateTime endDate)
{
async IAsyncEnumerable<int> Range()
{
while (DateTime.Now < endDate)
{
yield return i;
await Task.Delay(500);
}
}
return Range();
}
public async Task SomeOtherMethodAsync()
{
// Do whatever pleases you here
Response.Headers.Add("foo", "bar");
var foo = await BarAsync(); // <-- Awaiting works as normal
// Set as many headers as you want, do with the Response object whatever you want
Response.Headers.Add("x-setting-custom-header", foo.SomeValue);
// Here you call a method that will flush the headers and start writing to the response body.
// After this you cannot change headers anymore.
await JsonSerializer.SerializeAsync(
Console.OpenStandardOutput(),
GetAsyncEnumerable(foo.EndDate)); // <-- Here you call your generator now, it will get materialised and will start streaming to the response body
} |
That workaround actually works. Thanks! I improved it a bit. This way the workaround is generic and the code in the original function can stay the same. I DO think this should be done in the framework. async IAsyncEnumerable<int> Range(HttpContext context)
{
var endDate = DateTime.Now.AddSeconds(1000);
var foo = await BarAsync();
// only set headers before yielding the first value
context.Response.Headers.Add("x-setting-custom-header", foo);
int i = 0;
while (DateTime.Now < endDate)
{
yield return i++;
await Task.Delay(1);
}
}
async Task SomeOtherMethodAsync(HttpContext context)
{
var g = Range(context).GetAsyncEnumerator();
var hasValues = await g.MoveNextAsync(); // now all the headers shuld be set
static async IAsyncEnumerable<int> _inner(bool hasValues, IAsyncEnumerator<int> g)
{
if (hasValues)
{
yield return g.Current;
while (await g.MoveNextAsync())
{
yield return g.Current;
}
}
}
// Here you call a method that will flush the headers and start writing to the response body.
// After this you cannot change headers anymore.
await JsonSerializer.SerializeAsync(context.Response.Body, _inner(hasValues, g));
} |
What exactly should be done by the framework? |
Calling into the method at least once before flushing the headers. |
@arjennienhuis if you can outline how this would work I'd be happy to indulge. |
You can also return a public async Task<IAsyncEnumerable<WeatherForecastData>> Get()
{
var results = await _dataService.GetData();
Response.Headers.Add("Test", results.Name);
return Enumerate();
async IAsyncEnumerable<WeatherForecastData> Enumerate()
{
await foreach (var result in results)
{
yield return result;
}
}
} |
I'm not sure if this will work, but it might: The framework used to buffer the the whole steam. I believe that was here:
Don't buffer the whole stream, just "peek" a single element and do something like: var value = result.Value;
if (_isIAsyncEnumerable(value)) {
...
var g = value.GetAsyncEnumerator();
var hasValue = await g.MoveNextAsync(); // now all the headers shuld be set
static async IAsyncEnumerable _inner(...) {... };
value = _inner(...);
)
var objectType = value?.GetType() ?? typeof(object); (I believe this also allows sending error messages or a non 200 status code to the client if an exception occurs early enough.) |
The framework is no longer responsible for dealing with IAsyncEnumerable, it delegates to the serializer. |
My suggestion is to let the framework do just a little more work, and then delegate it to the serializer. That way, existing code breaks a little less, while still not materializing the whole stream. |
I don't think the framework should do anything here. |
Could you elaborate? The framework used to do a lot more (i.e. buffering). Removing that code broke some stuff. Putting a little bit of the code back might fix it. |
The buffering was removed because it was the wrong thing to do (hence this behavior change). The serialization was pushed down into the serializer, where it belongs, and we don't want to special case IEnumerable or IAsyncEnumerable. The workarounds above actually look pretty reasonable, the best one so far being the |
@campersau solution works perfectly. I understand the design is based on separation of concerns or maybe performance implications. If not going to be addressed by the framework itself then some documentation on the quirks of IAsyncEnumerable would be helpful, right now it's a bit confusing as to why it wouldn't work without the behavior explained in this thread. |
Documentation would be great! We will do that. |
Describe the bug
After updating from .Net 5 to .Net 6 Preview 7, I'm having an exception "System.InvalidOperationException: Headers are read-only, response has already started." when trying to setup a response header before returning an IAsyncEnumerable. This error only occurs when making another async call in the process.
My use case is that for pagination purposes before returning the IAsyncEnumerable I perform a query to get the total amount of data in the db, then place that pagination information as a response header for the client.
To Reproduce
Use this webapi minimal repository to reproduce
https://github.com/gabynevada/.net6-iasync-enumerable-set-header-error
Exceptions (if any)
Click to expand exception message
System.InvalidOperationException: Headers are read-only, response has already started.
at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpHeaders.ThrowHeadersReadOnlyException() in Microsoft.AspNetCore.Server.Kestrel.Core.dll:token 0x6000a43+0xa
at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpHeaders.System.Collections.Generic.IDictionary<System.String,Microsoft.Extensions.Primitives.StringValues>.Add(String key, StringValues value) in Microsoft.AspNetCore.Server.Kestrel.Core.dll:token 0x6000a5a+0x8
at System.Collections.Generic.CollectionExtensions.TryAdd[TKey,TValue](IDictionary
2 dictionary, TKey key, TValue value) in System.Collections.dll:token 0x600005b+0x17 at SetResponseHeaders.Controllers.WeatherForecastController.Get(CancellationToken cancellationToken)+MoveNext() in /Users/elvis/Downloads/SetResponseHeaders/Controllers/WeatherForecastController.cs:line 27 at SetResponseHeaders.Controllers.WeatherForecastController.Get(CancellationToken cancellationToken)+System.Threading.Tasks.Sources.IValueTaskSource<System.Boolean>.GetResult() in SetResponseHeaders.dll:token 0x6000014+0x0 at System.Threading.Tasks.ValueTask
1.ValueTaskSourceAsTask.<>c.<.cctor>b__4_0(Object state) in System.Private.CoreLib.dll:token 0x60033c6+0x23--- End of stack trace from previous location ---
at System.Text.Json.Serialization.Converters.IAsyncEnumerableOfTConverter
2.OnWriteResume(Utf8JsonWriter writer, TAsyncEnumerable value, JsonSerializerOptions options, WriteStack& state) in System.Text.Json.dll:token 0x6000970+0x95 at System.Text.Json.Serialization.Converters.IEnumerableDefaultConverter
2.OnTryWrite(Utf8JsonWriter writer, TCollection value, JsonSerializerOptions options, WriteStack& state) in System.Text.Json.dll:token 0x600099e+0x86at System.Text.Json.Serialization.Converters.IAsyncEnumerableOfTConverter
2.OnTryWrite(Utf8JsonWriter writer, TAsyncEnumerable value, JsonSerializerOptions options, WriteStack& state) in System.Text.Json.dll:token 0x600096f+0x14 at System.Text.Json.Serialization.JsonConverter
1.TryWrite(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state) in System.Text.Json.dll:token 0x6000790+0x1f4at System.Text.Json.Serialization.JsonConverter
1.WriteCore(Utf8JsonWriter writer, T& value, JsonSerializerOptions options, WriteStack& state) in System.Text.Json.dll:token 0x600077b+0x0 at System.Text.Json.Serialization.JsonConverter
1.WriteCoreAsObject(Utf8JsonWriter writer, Object value, JsonSerializerOptions options, WriteStack& state) in System.Text.Json.dll:token 0x600077a+0x49at System.Text.Json.JsonSerializer.WriteCore[TValue](JsonConverter jsonConverter, Utf8JsonWriter writer, TValue& value, JsonSerializerOptions options, WriteStack& state) in System.Text.Json.dll:token 0x60003c6+0x18
at System.Text.Json.JsonSerializer.WriteAsyncCore[TValue](Stream utf8Json, TValue value, JsonTypeInfo jsonTypeInfo, CancellationToken cancellationToken) in System.Text.Json.dll:token 0x60003d4+0xd4
at System.Text.Json.JsonSerializer.WriteAsyncCore[TValue](Stream utf8Json, TValue value, JsonTypeInfo jsonTypeInfo, CancellationToken cancellationToken) in System.Text.Json.dll:token 0x60003d4+0x1b5
at System.Text.Json.JsonSerializer.WriteAsyncCore[TValue](Stream utf8Json, TValue value, JsonTypeInfo jsonTypeInfo, CancellationToken cancellationToken) in System.Text.Json.dll:token 0x60003d4+0x38b
at Microsoft.AspNetCore.Mvc.Formatters.SystemTextJsonOutputFormatter.WriteResponseBodyAsync(OutputFormatterWriteContext context, Encoding selectedEncoding) in Microsoft.AspNetCore.Mvc.Core.dll:token 0x6000b14+0x132
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|30_0[TFilter,TFilterAsync](ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted) in Microsoft.AspNetCore.Mvc.Core.dll:token 0x6000a89+0x6a
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ResultExecutedContextSealed context) in Microsoft.AspNetCore.Mvc.Core.dll:token 0x6000a7c+0x15
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.ResultNext[TFilter,TFilterAsync](State& next, Scope& scope, Object& state, Boolean& isCompleted) in Microsoft.AspNetCore.Mvc.Core.dll:token 0x6000a77+0x3dc
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|28_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted) in Microsoft.AspNetCore.Mvc.Core.dll:token 0x6000a88+0x6e
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|20_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted) in Microsoft.AspNetCore.Mvc.Core.dll:token 0x6000a81+0x65
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope) in Microsoft.AspNetCore.Mvc.Core.dll:token 0x6000a7d+0x77
at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope) in Microsoft.AspNetCore.Mvc.Core.dll:token 0x6000a7d+0xfb
at Microsoft.AspNetCore.Routing.EndpointMiddleware.g__AwaitRequestTask|6_0(Endpoint endpoint, Task requestTask, ILogger logger) in Microsoft.AspNetCore.Routing.dll:token 0x60000ab+0x5e
at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context) in Microsoft.AspNetCore.Authorization.Policy.dll:token 0x6000013+0x16b
at Swashbuckle.AspNetCore.SwaggerUI.SwaggerUIMiddleware.Invoke(HttpContext httpContext) in Swashbuckle.AspNetCore.SwaggerUI.dll:token 0x6000002+0x1ce
at Swashbuckle.AspNetCore.Swagger.SwaggerMiddleware.Invoke(HttpContext httpContext, ISwaggerProvider swaggerProvider) in Swashbuckle.AspNetCore.Swagger.dll:token 0x6000009+0x8e
at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context) in Microsoft.AspNetCore.Diagnostics.dll:token 0x60000aa+0x82
warn: Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware[2]
The response has already started, the error page middleware will not be executed.
fail: Microsoft.AspNetCore.Server.Kestrel[13]
Further technical details
dotnet --info
Click to view output
.NET SDK (reflecting any global.json):
Version: 6.0.100-preview.7.21379.14
Commit: 22d70b47bc
Runtime Environment:
OS Name: Mac OS X
OS Version: 11.5
OS Platform: Darwin
RID: osx.11.0-x64
Base Path: /usr/local/share/dotnet/sdk/6.0.100-preview.7.21379.14/
Host (useful for support):
Version: 6.0.0-preview.7.21377.19
Commit: 91ba01788d
.NET SDKs installed:
5.0.100 [/usr/local/share/dotnet/sdk]
5.0.101 [/usr/local/share/dotnet/sdk]
5.0.102 [/usr/local/share/dotnet/sdk]
5.0.103 [/usr/local/share/dotnet/sdk]
5.0.202 [/usr/local/share/dotnet/sdk]
6.0.100-preview.7.21379.14 [/usr/local/share/dotnet/sdk]
.NET runtimes installed:
Microsoft.AspNetCore.App 5.0.0 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.1 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.2 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.3 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.5 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.0-preview.7.21378.6 [/usr/local/share/dotnet/shared/Microsoft.AspNetCore.App]
Microsoft.NETCore.App 5.0.0 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.1 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.2 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.3 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.5 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.0-preview.7.21377.19 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
The text was updated successfully, but these errors were encountered: