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

[wasm][test] BrowserHttpHandler stream large responses with SetBrowserResponseStreamingEnabled #60339

Merged
merged 4 commits into from
Oct 14, 2021

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Oct 13, 2021

In order to be able to use WebAssemblyEnableStreamingResponse option, the user should also set HttpCompletionOption.ResponseHeadersRead so that the content would not be cached in memory.

This is unit test covering the user scenario.
Fixes #60287

@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Net.Http test-enhancement Improvements of test source code labels Oct 13, 2021
@pavelsavara pavelsavara added this to the 7.0.0 milestone Oct 13, 2021
@ghost
Copy link

ghost commented Oct 13, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

In order to be able to use WebAssemblyEnableStreamingResponse option, the user should also set HttpCompletionOption.ResponseHeadersRead so that the content would not be cached in memory.

This is unit test covering the customer scenario.
Fixes #60287

Author: pavelsavara
Assignees: -
Labels:

arch-wasm, area-System.Net.Http, test enhancement

Milestone: 7.0.0

@ManickaP
Copy link
Member

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

/azp run runtime-libraries-mono outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara
Copy link
Member Author

BrowserHttpHandler_Streaming passed on mono outerloop now. There are many unrelated failures otherwise.

@ManickaP
Copy link
Member

src\libraries\Common\tests\System\Net\Http\ResponseStreamTest.cs(133,61): error CS0246: (NETCORE_ENGINEERING_TELEMETRY=Build) The type or namespace name 'HttpRequestOptionsKey<>' could not be found (are you missing a using directive or an assembly reference?)

Doesn't seem like unrelated... I assume it's because HttpRequestOptionsKey type doesn't exist in net48.

Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test looks good, @pranavkm I wonder if this should be documented along with the option?

@pranavkm
Copy link
Contributor

We have a quip about it on https://docs.microsoft.com/en-us/aspnet/core/blazor/call-web-api?view=aspnetcore-5.0&pivots=webassembly#httpclient-and-httprequestmessage-with-fetch-api-request-options-1.

The HTTP response is typically buffered to enable support for synchronous reads on the response content. To enable support for response streaming, use the SetBrowserResponseStreamingEnabled extension method on the request.

@pavelsavara pavelsavara merged commit e7709b6 into dotnet:main Oct 14, 2021
@campersau
Copy link
Contributor

@pranavkm I guess it would be great to also mention the HttpCompletionOption.ResponseHeadersRead in the docs as well.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 14, 2021
@pavelsavara pavelsavara deleted the wasm_http_large_stream branch January 4, 2022 13:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Net.Http test-enhancement Improvements of test source code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BrowserHttpHandler can't stream large responses even with SetBrowserResponseStreamingEnabled
6 participants