1720221 - add direct failover for system principal requests

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect, P1)

Description

[Shane Caraveo (:mixedpuppy)]

When a configured proxy fails to work, fallback to a direct connect for system requests. This ensures various update requests have the best chance of succeeding.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1720221 proxy failover to direct for system requests

Comment 2

[Alexandru Michis [:malexandru]]

Backed out changeset 2307d0bebc26 (Bug 1720221) for causing python failures in configure/lint.py
Backout link: https://hg.mozilla.org/integration/autoland/rev/6def2109b05afdfa6d771a3d1dc2ae9c8a1fb444
Push with failures, failure log.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 9230835 [details]
Bug 1720221 proxy failover to direct for system requests

Beta/Release Uplift Approval Request

• User impact if declined: updates may become unavailable
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: PI request has details
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): Change in behavior with proxies, though it can be compiled out or pref'd off if necessary.
• String changes made/needed: none

Comment 4

[Shane Caraveo (:mixedpuppy)]

That is an early request since this is still in autoland, but wanted to get visibility to others.

Comment 5

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

proxy failover to direct for system requests r=kershaw,necko-reviewers,robwu
https://hg.mozilla.org/integration/autoland/rev/b633563955ad437d61cb5ddb11b39074764c8e6d
https://hg.mozilla.org/mozilla-central/rev/b633563955ad

Comment 6

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9230835 [details]
Bug 1720221 proxy failover to direct for system requests

Approved for 91 beta 5 and let's have it manually verified on beta as well, thanks.

Comment 7

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/1b4a1f57a080

Comment 8

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 9230835 [details]
Bug 1720221 proxy failover to direct for system requests

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: per above beta request, but additionally this would be necessary if we pursue some AMO changes for further mitigation.
• User impact if declined: per above beta request
• Fix Landed on Version: 91
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): There are a number of changes in the proxy service since 78. I've looked through the code and feel that this particular change can apply and work just the same. There area couple merge conflicts that are simple to address by whoever would merge.
• String or UUID changes made by this patch: none

Comment 9

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1720221 use failover proxy for NET_RESET errors

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

use failover proxy for NET_RESET errors r=kershaw,necko-reviewers
https://hg.mozilla.org/integration/autoland/rev/179464e879dd2f92854d93c806e6cfa312ef6d7f
https://hg.mozilla.org/mozilla-central/rev/179464e879dd

Comment 11

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 9232468 [details]
Bug 1720221 use failover proxy for NET_RESET errors

Beta/Release Uplift Approval Request

• User impact if declined: per prior uplift requests
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: QA has steps in jira
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): this only adds checking for proxy failover on socket connection failures to a proxy
• String changes made/needed: none

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: If we uplift the prior patch to esr then this patch needs to go as well.
• User impact if declined:
• Fix Landed on Version: 91
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky):
• String or UUID changes made by this patch: none

Comment 12

[Julien Cristau [:jcristau]]

Resetting status so this doesn't fall off the uplift radar for the followup patch.

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/11f552e39cbf

Comment 14

[Alex Cornestean]

Hello,

Verified the patch on the latest Nightly (92.0a1/20210722214815) and Beta (91.0b6/20210722185635) under Windows 10 x64, Ubuntu 16.04 LTS and macOS 11.3.1.

With the proxy test add-on installed:

• browser update checks are performed and work as intended (initial aus5 requests fail as a result of the proxy add-on but the browser will immediately send successful duplicate requests);
• telemetry requests are sent (initial requests fail as a result of the proxy add-on but the browser will immediately send successful duplicate requests;
• add-on updates work as intended i.e. add-ons are successfully updated;
• recommendations are shown in Add-ons Manager;
• remote tools shows list of data and no errors are present

, thus confirming the fix.

Comment 15

[Shane Caraveo (:mixedpuppy)]

[Tracking Requested - why for this release]:

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

Shane, how important is it to have it on the esr 78 given that we have a new esr 91 branch that is replacing it soon? That patch would ship August 10 and the end of life of the esr78 branch is October 5.

Comment 17

[Shane Caraveo (:mixedpuppy)]

IIUC from the slack chat, we'll skip on esr78

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9230835 [details]
Bug 1720221 proxy failover to direct for system requests

Per comment 17.

Comment 19

[Shane Caraveo (:mixedpuppy)]

Unhiding to provide context, this has been out for ages.