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

[release/7.0] [browser] fix asset counting after download retry #82617

Conversation

pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Feb 24, 2023

This is simplified version of the https://github.com/dotnet/runtime/pull/81886/files

Fixes #82271

Customer Impact

Internally detected problem on CI.

The issue is caused by counting download retry multiple times.
Only when the retry is caused by download fail during await response.arrayBuffer() but after HTTP headers are already fetched.
That could be TCP failure and similar issues.

Testing

Added sample code with test for aborted HTTP stream and HTTP code 500 as well.
Manual testing

Risk

The download retry logic is complex.
This code path is relevant for WASM application which don’t use blazor (small audience).

@pavelsavara pavelsavara added this to the 7.0.x milestone Feb 24, 2023
@pavelsavara pavelsavara self-assigned this Feb 24, 2023
@ghost
Copy link

ghost commented Feb 24, 2023

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

Issue Details

This is simplified version of the https://github.com/dotnet/runtime/pull/81886/files

Fixes #82271

Customer Impact

Internally detected problem on CI

Testing

Manual testing

Risk

TBD

Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript

Milestone: 7.0.x

@lewing lewing changed the title [release/7.0] [browser] fix asset counting after downloa retry [release/7.0] [browser] fix asset counting after download retry Feb 25, 2023
@lewing lewing added Servicing-consider Issue for next servicing release review and removed Servicing-consider Issue for next servicing release review labels Feb 26, 2023
src/mono/wasm/runtime/assets.ts Show resolved Hide resolved
@ilonatommy
Copy link
Member

✅Manual tests work.

@lewing lewing added the Servicing-consider Issue for next servicing release review label Feb 28, 2023
@rbhanda rbhanda added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Feb 28, 2023
@carlossanlop carlossanlop modified the milestones: 7.0.x, 7.0.5 Mar 8, 2023
@carlossanlop
Copy link
Member

Approved by Tactics.
Signed-off by area owners.
No OOB changes needed (browser).
No related test failures, and manual testing passed.
Ready to merge. :shipit:

@carlossanlop carlossanlop merged commit b524e93 into dotnet:release/7.0 Mar 8, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Apr 7, 2023
@pavelsavara pavelsavara deleted the pavelsavara_backport/pr-81886-to-release/7.0 branch September 2, 2024 15:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants