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/8.0-staging] Catch exception when subscribing to NetworkChange #95123

Merged
merged 2 commits into from
Dec 6, 2023

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 22, 2023

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.

@ghost
Copy link

ghost commented Nov 22, 2023

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

Issue Details

Backport of #94861 to release/8.0-staging

/cc @rzikm

Customer Impact

Testing

Risk

IMPORTANT: If this backport is for a servicing release, please verify that:

  • The PR target branch is release/X.0-staging, not release/X.0.

  • If the change touches code that ships in a NuGet package, you have added the necessary package authoring and gotten it explicitly reviewed.

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@rzikm
Copy link
Member

rzikm commented Nov 22, 2023

cc @karelz

@rzikm rzikm self-assigned this Nov 22, 2023
@carlossanlop
Copy link
Member

Today is code complete for the January Release. Please get this approved and merged by 4pm PT at the latest if you want this fix shipped there. Otherwise, this will have to wait for the February Release.
@rzikm @karelz

@karelz
Copy link
Member

karelz commented Nov 22, 2023

Thanks @carlossanlop, we are not ready yet. Let's aim for Feb. Thanks!

@karelz karelz added this to the 8.0.x milestone Nov 28, 2023
@karelz karelz requested a review from ManickaP November 29, 2023 14:25
@karelz
Copy link
Member

karelz commented Nov 29, 2023

@artl93 this is ready for your approval - please mark it with Servicing-consider if you agree. Thanks!

@artl93 artl93 added the Servicing-consider Issue for next servicing release review label Nov 29, 2023
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Dec 5, 2023
@leecow leecow modified the milestones: 8.0.x, 8.0.2 Dec 5, 2023
@rzikm rzikm merged commit 26e7d2d into release/8.0-staging Dec 6, 2023
109 of 117 checks passed
@jkotas jkotas deleted the backport/pr-94861-to-release/8.0-staging branch December 8, 2023 02:44
@github-actions github-actions bot locked and limited conversation to collaborators Jan 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants