Handle synchronous exceptions from partial #16679
Conversation
|
@Pilchie for 3.1 askmode. |
There was a problem hiding this comment.
There's a couple of additional places where MVC calls the sync dispose on these types:
- https://github.com/aspnet/AspNetCore/blob/c565386a3ed135560bc2e9017aa54a950b4e35dd/src/Mvc/Mvc.Core/src/Infrastructure/ContentResultExecutor.cs#L58,
- https://github.com/aspnet/AspNetCore/blob/acf45106fcc972d2c28f7cdfc198be7c98c8c574/src/Mvc/Mvc.ViewFeatures/src/ViewComponentResultExecutor.cs#L126
^ Not exhaustive. I just searched for CreateWriter on GitHub. We should be able to just turn them all into await usings without much ceremony. I don't think we necessarily need a test for every one of these cases.
| @@ -0,0 +1 @@ | |||
| @(new TagHelpersWebSite.MyHtmlContent(Html)) | |||
There was a problem hiding this comment.
Could this just use @Html.Partial("_Test")? I'm not sure if the extra indirection is necessary.
There was a problem hiding this comment.
The extra indirection seems to be necessary. Tested it out and we don't get the Synchronous complain otherwise.
| @@ -12,8 +12,11 @@ | |||
|
|
|||
| <ItemGroup> | |||
| <Reference Include="Microsoft.AspNetCore.Mvc" /> | |||
| <Reference Include="Microsoft.AspNetCore.Diagnostics" /> | |||
There was a problem hiding this comment.
Can we undo changes to this file and the Startup?
| @@ -233,7 +233,7 @@ public class ViewExecutor | |||
|
|
|||
| OnExecuting(viewContext); | |||
|
|
|||
| using (var writer = WriterFactory.CreateWriter(response.Body, resolvedContentTypeEncoding)) | |||
| await using (var writer = WriterFactory.CreateWriter(response.Body, resolvedContentTypeEncoding)) | |||
There was a problem hiding this comment.
It should be fairly easy to write a unit test for this too. Basically we would write a test where the body of the view in https://github.com/aspnet/AspNetCore/blob/master/src/Mvc/Mvc.ViewFeatures/test/ViewExecutorTest.cs#L94-L97 throws and the test ensures that MVC does not perform a synchronous write to the HTTP response stream.
|
Approved for 3.1.0-Preview3 |
Pilchie
added
the
area-mvc
ryanbrandenburg
force-pushed
the
rybrande/PartialSync
branch
from
e0d274e to
9e8b75d
Compare
ryanbrandenburg
force-pushed
the
rybrande/PartialSync
branch
from
9e8b75d to
cf3b303
Compare
|
🆙📅 |
Co-Authored-By: Pranav K <[email protected]>
|
This comment was made automatically. If there is a problem contact [email protected]. I've triaged the above build. I've created/commented on the following issue(s) |
Fixes #15367.