Skip to content

[release/9.0] [browser][http] mute JS exceptions about network errors + HEAD verb #113261

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

Merged
merged 14 commits into from
Mar 17, 2025

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Mar 7, 2025

Backport of #113014 to release/9.0

/cc @pavelsavara

Customer Impact

  1. In browser HTTP client we subscribe for rejected promises in order to suppress unhandled rejection of AbortError when we issue the abort. The current handler also handles other network errors and aborts the whole program, which is not what it should do. Instead the network error should be propagated to managed HTTP client code.

  2. in Firefox when the verb is HEAD, the body is null. We should not fail because of that.

  3. Passing Response as result of http_wasm_fetch is creating JS memory leak by marshaling the object.

  • Customer reported
  • Found internally

Fixes #112172
Contributes to #111992

Regression

  • Yes
  • No

Testing

Automated tests.

Risk

Low

Both issues are only triggered when user enabled streaming responses. That's opt-in feature in Net9 and below.

@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 7, 2025
@pavelsavara pavelsavara self-assigned this Mar 7, 2025
@pavelsavara pavelsavara added arch-wasm WebAssembly architecture area-System.Net.Http os-browser Browser variant of arch-wasm and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Mar 7, 2025
@pavelsavara pavelsavara added this to the 9.0.x milestone Mar 7, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@pavelsavara pavelsavara requested review from akoeplinger and maraf March 7, 2025 17:03
@pavelsavara
Copy link
Member

cc @campersau

@lewing lewing added the Servicing-consider Issue for next servicing release review label Mar 7, 2025
@pavelsavara pavelsavara changed the base branch from release/9.0 to release/9.0-staging March 7, 2025 17:13
@carlossanlop
Copy link
Contributor

@pavelsavara @lewing @steveisok Today is code complete for the April Release. If you want this fix included in this release, please get it approved by Tactics and merge it before 4pm PT.

@pavelsavara pavelsavara added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 11, 2025
@pavelsavara
Copy link
Member

/ba-g CI failures are unrelated

@pavelsavara pavelsavara merged commit ad3b340 into release/9.0-staging Mar 17, 2025
92 of 95 checks passed
@radderz
Copy link

radderz commented Mar 18, 2025

is this still going out in the april release or now in may release?

@pavelsavara
Copy link
Member

is this still going out in the april release or now in may release?

@radderz May, we didn't make it into the previous release train

@jkotas jkotas deleted the backport/pr-113014-to-release/9.0 branch March 23, 2025 02:42
@github-actions github-actions bot locked and limited conversation to collaborators Apr 22, 2025
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 os-browser Browser variant of arch-wasm Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants