-
Notifications
You must be signed in to change notification settings - Fork 1k
fix(httpx): honor http11 by disabling retryablehttp http2 fallback #2398
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
base: dev
Are you sure you want to change the base?
fix(httpx): honor http11 by disabling retryablehttp http2 fallback #2398
Conversation
WalkthroughThe pull request fixes an issue where the HTTP/1.1 protocol flag was being bypassed by retryablehttp's HTTP/2 fallback mechanism. When HTTP/1.1 is configured, the code now assigns the HTTPClient2 fallback client to the same underlying client as HTTPClient, preventing unintended protocol switching. Tests are added to verify this behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Extended local test report (macOS arm64):\n\n- Full suite: FAIL ./... [setup failed] |
|
Extended local test report (macOS arm64):
Proof log (full command outputs):
Note: the race failure appears unrelated to this PR’s http11/http2 fallback change, but included for completeness. |
|
Additional details (excerpted from local test run): Commands:
Results highlights:
Race failure ends with: (Full stack trace available if needed.) |
|
Added stress testing (local):\n- go test ./common/httpx -run TestHTTP11DisablesHTTP2Fallback -count=100 ✅\n- go test ./common/httpx -run TestDo -count=50 ✅\n- go test -race ./common/httpx -count=10 ✅\n\nProof appended to: bounties/httpx/proof/proof_2240_extensive_tests.txt |
Problem
/claim #2240
When running httpx with
-pr http11, the transport is configured for HTTP/1.1, but retryablehttp-go can still fall back to an HTTP/2 client on certain error paths. That bypasses the user’s explicit protocol selection and makes-pr http11unreliable.Proposed Changes
Options.Protocol == http11, setretryablehttpClient.HTTPClient2 = retryablehttpClient.HTTPClientso the library cannot switch to a different protocol.httptestservers (no external network).Proof
GOCACHE=/tmp/gocache-httpx go test ./common/httpx -count=1Checklist
dev)