1816677 - HTTP/3 won't be retried if first destination is unreachable

Status: RESOLVED FIXED

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

Description

[Raul Bucata]

From github: https://github.com/webcompat/web-bugs/issues/118205.

<!-- @browser: Firefox 109.0 -->
<!-- @ua_header: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/109.0 -->
<!-- @reported_with: unknown -->
<!-- @public_url: https://github.com/webcompat/web-bugs/issues/118205 -->

URL: https://cloudflare-quic.com/

Browser / Version: Firefox 109.0
Operating System: Windows 10
Tested Another Browser: Yes Chrome

Problem type: Something else
Description: no quic protocol used in private browsing mode
Steps to Reproduce:
firefox doesn`t use http/3 in private browsing mode but chrome use even in private mode and i tested in youtube and https://cloudflare-quic.com/

<details>
<summary>Browser Configuration</summary>
<ul>
<li>None</li>
</ul>
</details>

From webcompat.com with ❤️

Change performed by the Move to Bugzilla add-on.

Comment 1

[Tim Huang[:timhuang]]

Hello Valentin, has this something to do with Necko? Do we do differently in private windows for quic?

Comment 2

[Valentin Gosu [:valentin] (he/him)]

So, if you load https://cloudflare-quic.com/ in the browser and don't have DNS over HTTPS (network.trr.mode is not 2 or 3), then the first time we would use HTTP/2 and only use HTTP/3 when we get the "alt-svc" header (subsequent loads)
This happens in both regular and private browser (but may be more evident in PB because when you open it there is a fresh cache).
When DNS over HTTPS is enabled, we always get HTTP/3

Raul, can you confirm this behaviour?

Comment 3

[Raul Bucata]

Valetin, if I enable the setting via Firefox from about:preferences#general, in the Network Settings section, the page still returns that HTTP/2 is being used, in both Normal and Private Mode, after refreshing the page multiple times. The same applies if I change network.trr.mode pref from 0 to either 2 or 3. Is there somewhere else that I should be looking to see if HTTP/3 is being used, beside the message displayed on the page?

Comment 4

[Valentin Gosu [:valentin] (he/him)]

I was able to reproduce this.
It seems apart from the first issue with alt-svc, we sometimes fail to connect to the http/3 endpoint, so we add the host to the excluded list, so it keeps using http/2 regardless of how often we refresh. Executing Services.obs.notifyObservers(null, "network:reset-http3-excluded-list", "") in the browser console and refreshing gets me http/3 again, so this is definitely the problem.
This seems to happen regardless if we're using TRR or not.

Kershaw, could you take a look? Let me know if you can't reproduce it, maybe I can capture some logs for you.

Comment 5

[Valentin Gosu [:valentin] (he/him)]

Kershaw and I debugged this yesterday. It seems the failure usually only happens when TRR is turned on. And that's because the first HTTP/3 tries are with an IPv6 address, and when you don't have IPv6 that fails. We need to also add a retry with IPv4 logic to UDP connections.

Comment 6

[Kershaw Chang [:kershaw]]

Attachment: Bug 1816677 - Allow to retry diffrent IP family for Http/3, r=#necko

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9ccaad4630db
Allow to retry diffrent IP family for Http/3, r=necko-reviewers,valentin

Comment 8

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/9ccaad4630db

Comment 9

[Valentin Gosu [:valentin] (he/him)]

This issue affects our ability to use HTTP/3. If we don't see any regressions from this patch we should consider uplifting to esr115

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Right now, that just means requesting Beta uplift :-)

Comment 11

[Atila Butkovits]

Backed out for causing DNS related crashes bug 1837252.

Comment 12

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/20d429c3c3eb
Backed out changeset 9ccaad4630db for causing DNS related crashes (bug 1837252). a=backout

Comment 13

[Kershaw Chang [:kershaw]]

Attachment: Bug 1816677 - Allow to retry diffrent IP family for Http/3, r=#necko

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dfe0fed4de68
Allow to retry diffrent IP family for Http/3, r=necko-reviewers,valentin

Comment 15

[Narcis Beleuzu [:NarcisB]]

Backed out for xpcshell failure on test_http3_dns_retry.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/334db4415e2ea6ff6c3ff98355db7e3c3c0b5845
Log link: https://treeherder.mozilla.org/logviewer?job_id=420491123&repo=autoland&lineNumber=2239

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0ea0074ef6d0
Allow to retry diffrent IP family for Http/3, r=necko-reviewers,valentin

Comment 17

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/0ea0074ef6d0

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Is this something we should consider uplifting to ESR115?

Comment 19

[Kershaw Chang [:kershaw]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)

Is this something we should consider uplifting to ESR115?

Yes, I'll request an uplift.

Comment 20

[Kershaw Chang [:kershaw]]

Comment on attachment 9338618 [details]
Bug 1816677 - Allow to retry diffrent IP family for Http/3, r=#necko

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Users could still use HTTP/2 to connect, but the server does support HTTP/3.
• User impact if declined: The same as the above reason.
• Fix Landed on Version: 116
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The previous version of this patch caused some crashes before and this change is not straightforward, so I think the risk is between low and medium.
  Note that this feature can be disabled via a pref and we do have a unit test for this.

Comment 21

[Dianna Smith [:diannaS]]

Comment on attachment 9338618 [details]
Bug 1816677 - Allow to retry diffrent IP family for Http/3, r=#necko

Approved for 115.1esr

Comment 22

[Dianna Smith [:diannaS]]

:kershaw before uplifting this, should we be concerned with bug 1840981 ?

Comment 23

[Kershaw Chang [:kershaw]]

(In reply to Dianna Smith [:diannaS] from comment #22)

:kershaw before uplifting this, should we be concerned with bug 1840981 ?

I think it's fine, since all failures happened when the socket process is enabled, and the socket process is not enabled by default.

Comment 24

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/9de50c08d971

Comment 25

[Dianna Smith [:diannaS]]

network.http.http3.retry_different_ip_family was disabled in 116.0.3 in bug 1847066
netwerk/test/unit/test_http3_dns_retry.js was disabled in bug 1840981

debated setting 116 to disabled since there was more on this bug. :kershaw should we reopen this?

Comment 26

[Kershaw Chang [:kershaw]]

Yeah, let's reopen this.
Thanks.

Comment 27

[Kershaw Chang [:kershaw]]

Attachment: Bug 1816677 - Make sure we rewind the request stream when retrying a transaction for different IP address, r=#necko

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aba9f42d60b3
Make sure we rewind the request stream when retrying a transaction for different IP address, r=necko-reviewers,jesup

Comment 29

[Sandor Molnar[:smolnar]]

Backed out for causing xpc failures on netwerk/test/unit/test_http3_dns_retry.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/5e0e44d9e024c68de16f25cdffae9b7a16599fef

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | netwerk/test/unit/test_http3_dns_retry.js | xpcshell return code: 0

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4cbbadf528f7
Make sure we rewind the request stream when retrying a transaction for different IP address, r=necko-reviewers,jesup

Comment 31

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/4cbbadf528f7