[release/8.0-staging] Catch exception when subscribing to NetworkChange #95123
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
ghost
added
the
|
Tagging subscribers to this area: @dotnet/ncl Issue DetailsBackport of #94861 to release/8.0-staging /cc @rzikm Customer ImpactTestingRiskIMPORTANT: If this backport is for a servicing release, please verify that:
|
|
cc @karelz |
|
Thanks @carlossanlop, we are not ready yet. Let's aim for Feb. Thanks! |
karelz
approved these changes
Nov 29, 2023
ManickaP
approved these changes
Nov 29, 2023
|
@artl93 this is ready for your approval - please mark it with |
artl93
approved these changes
Nov 29, 2023
leecow
added
Servicing-approved
1 task
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #94861 to release/8.0-staging
Fixes #94794
/cc @rzikm
Customer Impact
This is a regression against 7.0.
Customers impacted: Restricted environments like Google Cloud - customer reported in #94794
Whenever user sends any HTTP request (incl. HTTP 1.1 or 2.0) to a server which supports also HTTP/3 -- in specific constrained environments (like Google Cloud as reported in #94794), the user will NOT be able to read the response, because we will throw exception.
Workaround is to disable HTTP/3 via AppContext switch (or environment variable).
Customers would run into the same problem on 7.0, if they had msquic.so installed on the machine. That is not the case for customer reporting issue #94794 (they do not have msquic.so installed on their Google Cloud constrained environment).
When the msquic.so was NOT installed on 7.0, the problem didn't exist, but it DOES exist on 8.0 -- hence a regression. The reason is that the problematic check was delayed AFTER msquic.so presence check in 7.0. In 8.0 we changed the order to load msquic.so ONLY when user asks for HTTP/3 (see PR #83494), therefore doing the problematic check also for HTTP/1.1 and HTTP/2 requests, causing this regression.
Testing
Tested on simulated constrained environment (via injecting faults in Networking code). Tested both before and after change.
Risk
Low, change is only adding a catch block around code which throws only in constrained environments.