[release/7.0][browser][http] Fix blocking of streaming response and abort #81092

pavelsavara · 2023-01-24T15:35:33Z

Backport of #80693

return bytes of streaming response as soon as available
fix unhandled error in reader.cancel() promise
return cancelable promise from http_wasm_get_streamed_response_bytes
unit test for slowly streamed chunks
unit test for streaming and default cancellation

Customer Impact

Fixes #79238
Fixes #80696

Testing

unit tests
manual test with customers

Risk

Impact is limited to

users who use HTTP client with WebAssemblyEnableStreamingResponse request option opt-in
users who pass cancellation token

- return bytes of streaming response as soon as available - fix unhandled error in reader.cancel() promise - return cancelable promise from http_wasm_get_streamed_response_bytes - unit test for slowly streamed chunks - unit test for streaming and default cancellation

ghost · 2023-01-24T15:35:48Z

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

Issue Details

Backport of #80693

return bytes of streaming response as soon as available
fix unhandled error in reader.cancel() promise
return cancelable promise from http_wasm_get_streamed_response_bytes
unit test for slowly streamed chunks
unit test for streaming and default cancellation

Customer Impact

Fixes #79238
Fixes #80696

Testing

unit tests
manual test

Risk

Author:	pavelsavara
Assignees:	pavelsavara
Labels:	`arch-wasm`, `area-System.Runtime.InteropServices.JavaScript`
Milestone:	7.0.x

pavelsavara · 2023-01-24T15:55:54Z

please help me to check that the fix works for you @hakenr @oleneveu

how to test it

check out this Net7 branch
make sure you delete artifacts directory, if you have any previous builds
build it with build.cmd -bl -os Browser -subset mono+libs -c Release
edit your blazor client project, for example BlazorGrpcWebCodeFirst\Client\BlazorGrpcWebCodeFirst.Client.csproj in your repro app
add into client .csproj

  <Target Name="UpdateRuntimePack" AfterTargets="ResolveFrameworkReferences">
    <ItemGroup>
      <ResolvedRuntimePack PackageDirectory="c:\Dev\runtime\artifacts\bin\microsoft.netcore.app.runtime.browser-wasm\Release"
                           Condition="'%(ResolvedRuntimePack.FrameworkName)' == 'Microsoft.NETCore.App'" />
    </ItemGroup>
  </Target>

replace c:\Dev\runtime\ with your own path to the runtime

I already tested it with

VirtualizeGrpcRepro
BlazorStreamReaderIssue with AddInterceptor<CancellationWorkaroundGrpcClientInterceptor>(); commented out

pavelsavara · 2023-01-24T15:58:03Z

/azp run runtime-wasm

azure-pipelines · 2023-01-24T15:58:14Z

Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara · 2023-01-24T20:41:58Z

I think that's unhandled at https://github.com/protobuf-net/protobuf-net.Grpc/blob/8e89902f8a900fb2f603ea5d628e8a0b19de9d21/src/protobuf-net.Grpc/Internal/Reshape.cs#L554
And then in the code of the sample.

@hakenr could you please comment ?

Is that GRPC issue which is solved by your CancellationWorkaroundGrpcClientInterceptor ?

hakenr · 2023-01-24T23:13:00Z

@pavelsavara Yes, this RpcException is expected and handled by the CancellationWorkaroundGrpcClientInterceptor (which is there as workaround for grpc/grpc-dotnet#2014).

I will try this net7 branch with my repro later this week and confirm, but it seems the issue is fixed now ❤️.

oleneveu · 2023-01-26T08:04:32Z

@pavelsavara One of my colleagues has tested the fix on our project were we had an issue with it and it works. Thanks!

pavelsavara · 2023-02-01T16:13:47Z

I will try this net7 branch with my repro later this week and confirm, but it seems the issue is fixed now ❤️.

ping @hakenr, I would appreciate the confirmation.

pavelsavara · 2023-02-02T08:46:38Z

@hakenr if you are interested to make it work, please ping me at https://aka.ms/dotnet-discord and we could do it more interactively. If it's not something which seems like fun, no worries.

I realized that since this is just change in dotnet.js I could give you just that file.

I zipped dotnet.7.0.2.2w7arw7yw0.js for you
in bin\Release\net7.0\wwwroot\_framework\ of your blazor project (or server which is hosting it).
- delete any other dotnet*.js files and their .gz and .br alternatives
- you need to unpack my dotnet.7.0.2.2w7arw7yw0.js into _framework\ instead of the old ones
- The file has new hash so, edit your blazor.boot.json
- use "dotnet.7.0.2.2w7arw7yw0.js": "sha256-4fyIoT2cFX0RlAXX93ZDbT6aBTBv5g6QB0b9M2Sy8l8=",
- I attached mine blazor.boot.json just for comparison, but you need yours for your specific project's resources

It should be largely compatible with 7.0.2 runtime and just work 🤞.

81092.zip

hakenr · 2023-02-03T01:14:17Z

@pavelsavara Many thanks for providing a much easier way to test the fix.
I can confirm that after replacing the JS with dotnet.7.0.2.2w7arw7yw0.js I'm no longer able to reproduce the issue within my repo: https://www.screencast.com/t/h3vInxlQ

pavelsavara · 2023-02-03T12:13:17Z

The CI failures are on optional multi-threaded pipelines.

carlossanlop · 2023-02-08T23:42:02Z

@pavelsavara @maraf I am seeing many wasm failures in this PR's CI that I'm not seeing in other 7.0 PRs. Can you please investigate them and determine if they're related to this change?

carlossanlop · 2023-02-08T23:50:05Z

I just noticed the CI logs are gone (they were too old). So I'm closing and reopening the PR to run it again, and preferably on top of the latest fixes we merged for arm64 helix queues.

maraf · 2023-02-09T11:22:48Z

@pavelsavara Pavel Savara FTE @maraf Marek Fisera FTE I am seeing many wasm failures in this PR's CI that I'm not seeing in other 7.0 PRs. Can you please investigate them and determine if they're related to this change?

The last runtime pipeline run looks good. The errors visible on github from runtime-wasm pipeline are not actual errors, as they come from multi-threaded lines which are expected to fail.

carlossanlop · 2023-02-09T17:59:18Z

@dotnet/dnceng I closed and reopened this PR last night to get a fresh set of CI results, and today I noticed the build results from the runtime-wasm queue are non-existent. Is this a known issue? https://github.com/dotnet/runtime/pull/81092/checks?check_run_id=10860243431

I clicked on "21 errors / 3 warnings" link.

garath · 2023-02-09T18:36:52Z

I closed and reopened this PR last night to get a fresh set of CI results, and today I noticed the build results from the runtime-wasm queue are non-existent.

Looks like that did not cause the runtime-wasm pipeline to re-run. The results shown in GitHub are from two weeks ago.

You can manually rerun from within the GitHub Checks UI, from the UI in Azure Pipelines, or using the /azp run ... in-comment commands.

carlossanlop · 2023-02-09T18:43:09Z

Ah, thanks @garath for the clarification.

As it was pointed out, the failures are not concerning, they are the same set that's considered unrelated to this change.
Approved by Tactics for 7.0.4 via email.
Signed off by area owners.
No OOB changes needed.
Ready to merge.

pavelsavara added arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript labels Jan 24, 2023

pavelsavara added this to the 7.0.x milestone Jan 24, 2023

pavelsavara requested a review from maraf January 24, 2023 15:35

pavelsavara self-assigned this Jan 24, 2023

build-analysis bot mentioned this pull request Jan 24, 2023

Tracking issue for CI build timeouts #76454

Closed

This comment was marked as resolved.

Sign in to view

pavelsavara marked this pull request as ready for review February 3, 2023 11:27

pavelsavara requested a review from lewing as a code owner February 3, 2023 11:27

pavelsavara added the Servicing-consider Issue for next servicing release review label Feb 3, 2023

maraf approved these changes Feb 3, 2023

View reviewed changes

pavelsavara added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Feb 6, 2023

carlossanlop closed this Feb 8, 2023

carlossanlop reopened this Feb 8, 2023

carlossanlop modified the milestones: 7.0.x, 7.0.4 Feb 9, 2023

carlossanlop merged commit e9d6af0 into dotnet:release/7.0 Feb 9, 2023

ghost locked as resolved and limited conversation to collaborators Mar 11, 2023

pavelsavara deleted the pavelsavara_backport/pr-80693-to-release/7.0 branch September 2, 2024 15:29

[release/7.0][browser][http] Fix blocking of streaming response and abort #81092

[release/7.0][browser][http] Fix blocking of streaming response and abort #81092

Uh oh!

Conversation

pavelsavara commented Jan 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Customer Impact

Testing

Risk

Uh oh!

ghost commented Jan 24, 2023

Customer Impact

Testing

Risk

Uh oh!

pavelsavara commented Jan 24, 2023

Uh oh!

pavelsavara commented Jan 24, 2023

Uh oh!

azure-pipelines bot commented Jan 24, 2023

Uh oh!

This comment was marked as resolved.

pavelsavara commented Jan 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hakenr commented Jan 24, 2023

Uh oh!

oleneveu commented Jan 26, 2023

Uh oh!

pavelsavara commented Feb 1, 2023

Uh oh!

This comment was marked as resolved.

pavelsavara commented Feb 2, 2023

Uh oh!

hakenr commented Feb 3, 2023

Uh oh!

pavelsavara commented Feb 3, 2023

Uh oh!

carlossanlop commented Feb 8, 2023

Uh oh!

carlossanlop commented Feb 8, 2023

Uh oh!

maraf commented Feb 9, 2023

Uh oh!

carlossanlop commented Feb 9, 2023

Uh oh!

garath commented Feb 9, 2023

Uh oh!

carlossanlop commented Feb 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pavelsavara commented Jan 24, 2023 •

edited

Loading

pavelsavara commented Jan 24, 2023 •

edited

Loading