[release/8.0][browser][http] mute JS exceptions about network errors + HEAD verb #113271
Conversation
|
Tagging subscribers to this area: @dotnet/ncl |
|
cc @campersau |
|
@pavelsavara Thank you! I'll let you know how it works in the next release. BTW, do you guys set tickets to closed when they are released? or when should I expect the next release to happen? |
|
Tests passed on chrome, log |
There was a problem hiding this comment.
PR Overview
This PR backports fixes to address unintended exception propagation in the browser HTTP client and to handle HEAD verb responses correctly in Firefox within Net8. Key changes include adding new tests for multiple HTTP methods including abort scenarios, updating the EchoHandler to handle delays and aborts appropriately, and modifying the wasm HTTP request handling to mute specific unhandled rejections.
Reviewed Changes
| File | Description |
|---|---|
| src/libraries/Common/tests/System/Net/Http/ResponseStreamTest.cs | Added new test cases covering different HTTP methods and abort scenarios for streaming responses. |
| src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/Handlers/EchoHandler.cs | Updated the echo handler to incorporate delay, buffering control, and proper handling of the HEAD method and abort scenarios. |
| src/mono/wasm/runtime/http.ts | Modified the HTTP handling logic to improve the muting of unhandled promise rejections by refactoring error handling in abort scenarios. |
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/Handlers/EchoHandler.cs:76
- [nitpick] Consider using a pre-defined constant (such as HttpMethods.Head) instead of the string literal "HEAD" to maintain consistency with other HTTP method checks.
if(context.Request.Method == "HEAD")
It should land in Net 10 preview3 nightly in next few days. It would be awesome if you can try to use it to validate the fix and report back. That would decrease risk for the Net8 patch. https://aka.ms/dotnet/10.0.1xx/daily/dotnet-sdk-win-x64.zip
I think this will go out in 8.0.15 in April. |
|
@pavelsavara @lewing 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. |
|
/ba-g CI failures are unrelated |
5a1fc27
into
dotnet:release/8.0-staging
@pavelsavara I downloaded the dotnet sdk, however, it looks like I need to have visual studio 17.16. |
@hectorm-bmg I don't know much about VS versions, sorry. Does your repro build with |
Backport of #113014 to release/8.0 with modifications for Net8
Customer Impact
In browser HTTP client we subscribe for rejected promises in order to suppress unhandled rejection of
AbortErrorwhen 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.in Firefox when the verb is HEAD, the body is null. We should not fail because of that.
Contributes to #112172
Fixes #111992
Regression
Testing
Automated tests.
Risk
Low
Both issues are only triggered when user enabled streaming responses. That's opt-in feature in Net8.