dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.46k stars 10.03k forks source link

Response buffering #27470

Open BrennanConroy opened 4 years ago

BrennanConroy commented 4 years ago
ghost commented 4 years ago

Thanks for contacting us. We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

halter73 commented 4 years ago

We released a preview package that does this which was last updated in 2017. It has over 500,000 downloads. https://www.nuget.org/packages/Microsoft.AspNetCore.Buffering/

dionrhys commented 3 years ago

I'd appreciate this feature. At the moment it's not straightforward to deal with exceptions that happen during serializing responses, e.g. exceptions that may happen while enumerating an IEnumerable<T> collection that's present on a response object's type (in an API controller).

Ideally we wouldn't have these exceptions, but mistakes happen. Right now an exception that occurs during response formatting/serialization will cause JSON output to be truncated mid-response and the 200 OK status code kept (as the response has already begun writing).

Here's an example of how that manifested for me today: ``` 2021-04-12 12:50:52.252 +01:00 [ERR] An unhandled exception has occurred while executing the request. System.NullReferenceException: Object reference not set to an instance of an object. at *****.Web.Misc.*****.GetUserDefinedFields(Product product, IEnumerable`1 polygonList) in C:\Projects\*****\src\*****\Misc\*****.cs:line 163 at *****.Web.Controllers.EventsController.<>c__DisplayClass4_0.b__3(ValueTuple`2 result) in C:\Projects\*****\src\*****\Controllers\EventsController.cs:line 266 at System.Linq.Enumerable.WhereSelectEnumerableIterator`2.MoveNext() at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeList(JsonWriter writer, IEnumerable values, JsonArrayContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty) at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeObject(JsonWriter writer, Object value, JsonObjectContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty) at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType) at Newtonsoft.Json.JsonSerializer.SerializeInternal(JsonWriter jsonWriter, Object value, Type objectType) at Microsoft.AspNetCore.Mvc.Formatters.JsonOutputFormatter.WriteObject(TextWriter writer, Object value) at Microsoft.AspNetCore.Mvc.Formatters.JsonOutputFormatter.d__11.MoveNext() ``` ``` 2021-04-12 12:50:52.288 +01:00 [WRN] The response has already started, the error page middleware will not be executed. ```

In order to be able to deal with unexpected exceptions during serialization right now, I have to pre-serialize the response object to JSON before returning it in a ContentResult:

return Content(JsonHelper.Serialize(response).ToString(), "application/json");

This sidesteps using the built-in output formatting in ASP.NET Core, which isn't ideal from a dev perspective.

I think this is a good workaround for now, but I think the right solution would be to add buffering, possibly. In this instance, I know I'll be sacrificing response writing performance (and extra memory usage) for correctness, and I'm ok with that.

I could just avoid having any IEnumerable<T> in my response objects and ensure everything is fully-enumerated into List<T> or something, however any dev mistake here would still cause the issue of a response being truncated mid-content while keeping 200 OK. Some people apparently experienced this issue if they had reference loops in their response objects for example: https://stackoverflow.com/questions/47419907/net-core-incomplete-json-response

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.