Skip to content

Comments

[wasm] fix error cases in assets loading#73702

Merged
pavelsavara merged 10 commits intodotnet:mainfrom
pavelsavara:wasm_fix_optional_assets
Aug 11, 2022
Merged

[wasm] fix error cases in assets loading#73702
pavelsavara merged 10 commits intodotnet:mainfrom
pavelsavara:wasm_fix_optional_assets

Conversation

@pavelsavara
Copy link
Member

@pavelsavara pavelsavara commented Aug 10, 2022 •
edited
Loading

  • fix bad rename of maxParallelDownloads - throttling
  • fix re-try logic
  • fixed counting of loaded assets
  • catching more error cases
  • fix blazor detection

Fixes #73673

@pavelsavara pavelsavara added this to the 7.0.0 milestone Aug 10, 2022
@pavelsavara pavelsavara requested a review from maraf August 10, 2022 17:03
@pavelsavara pavelsavara self-assigned this Aug 10, 2022
@ghost
Copy link

ghost commented Aug 10, 2022

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

Issue Details
  • fix bad rename of maxParallelDownloads - fixes throttling
Author: pavelsavara
Assignees: pavelsavara
Labels:

arch-wasm, area-System.Runtime.InteropServices.JavaScript
Milestone: 7.0.0

@pavelsavara
Copy link
Member Author

pavelsavara commented Aug 10, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 10, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara changed the title Wasm fix optional assets [wasm] fix error cases in assets loading Aug 10, 2022
This was referenced Aug 10, 2022
Closed
Closed
@pavelsavara
Copy link
Member Author

pavelsavara commented Aug 10, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 10, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@pavelsavara pavelsavara marked this pull request as ready for review August 10, 2022 18:09
@pavelsavara pavelsavara requested a review from lewing as a code owner August 10, 2022 18:09
kg
kg reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
kg
kg reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
kg
kg reviewed Aug 11, 2022
View reviewed changes
kg
kg reviewed Aug 11, 2022
View reviewed changes
kg
kg reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical reviewed Aug 11, 2022
View reviewed changes
radical
radical approved these changes Aug 11, 2022
View reviewed changes
Copy link
Member

@radical radical left a comment

Choose a reason for hiding this comment

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

I'm no expert on JS/TS, and reviewed whatever I could. Other than my comments, it looks good to me.

@pavelsavara
Copy link
Member Author

pavelsavara commented Aug 11, 2022

thanks @kg !

@pavelsavara
Copy link
Member Author

pavelsavara commented Aug 11, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 11, 2022

Azure Pipelines successfully started running 1 pipeline(s).

maraf
maraf reviewed Aug 11, 2022
View reviewed changes
@pavelsavara
Copy link
Member Author

pavelsavara commented Aug 11, 2022

/azp run runtime-wasm

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 11, 2022

Azure Pipelines successfully started running 1 pipeline(s).

thaystg
thaystg approved these changes Aug 11, 2022
View reviewed changes
Copy link
Member

@thaystg thaystg left a comment

Choose a reason for hiding this comment

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

LGTM. I also tested it on codespaces :)

@pavelsavara
Copy link
Member Author

pavelsavara commented Aug 11, 2022 •
edited
Loading

There was some crypto timeouts on this PR, but it went away on re-run.
All the other failures are #73721

@pavelsavara pavelsavara merged commit 478a8a2 into dotnet:main Aug 11, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2022
@pavelsavara pavelsavara deleted the wasm_fix_optional_assets branch November 29, 2022 16:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@radical radical radical approved these changes

@kg kg kg left review comments

@thaystg thaystg thaystg approved these changes

@maraf maraf maraf approved these changes

@lewing lewing Awaiting requested review from lewing lewing is a code owner

Assignees

@pavelsavara pavelsavara

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

[wasm] Regression: Crash on running debugger tests on Codespaces

5 participants

@pavelsavara @radical @kg @thaystg @maraf