fix: honor -pr http11 by disabling retryablehttp HTTP/2 fallback

[SolariSystems]

Superseded by PR #2419 — this PR is closed.

[coderabbitai]

Walkthrough

Replaces string protocol check with a constant and, when HTTP/1.1 is explicitly selected, aliases the retryablehttp fallback client (HTTPClient2) to the HTTP/1.1 client to prevent automatic HTTP/2 fallback. Tests moved to local httptest servers and add coverage for Content-Length and fallback behavior.

Changes

Cohort / File(s)	Summary
HTTP/1.1 Protocol Guard common/httpx/httpx.go	Use HTTP11 constant for protocol comparison; after building the HTTP client, when protocol is HTTP/1.1 set HTTPClient2 = HTTPClient to disable retryablehttp's HTTP/2 fallback.
Test Coverage Expansion common/httpx/httpx_test.go	Switch tests to httptest servers, add Content-Length/body cases, and add TestHTTP11DisablesHTTP2Fallback asserting HTTPClient2 is pinned to HTTPClient for HTTP/1.1 and remains independent by default.

Sequence Diagram(s)

sequenceDiagram
    participant Test as Test
    participant HTTPX as HTTPX (build client)
    participant Retryable as retryablehttp (Do)
    participant Server as Origin Server

    rect rgba(200,200,255,0.5)
    Test->>HTTPX: configure Protocol=HTTP11
    HTTPX->>Retryable: create HTTPClient (http1-only)
    HTTPX-->>Retryable: set HTTPClient2 = HTTPClient
    end

    Test->>Retryable: Do(Request)
    Retryable->>Server: send request (HTTP/1.1)
    Server-->>Retryable: respond (OK)
    Retryable-->>Test: return response (no HTTP/2 fallback)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I’m a rabbit in a curious patch of code,
I hop where protocols once freely rode,
I pin HTTP/1.1 so it won’t stray,
Tests stand guard along the way,
Hooray for steady hopping mode! 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: disabling retryablehttp's HTTP/2 fallback when HTTP/1.1 is explicitly requested via the -pr flag.
Linked Issues check	✅ Passed	The code changes directly address the core issue #2240 by setting HTTPClient2 = HTTPClient when Protocol == HTTP11, preventing silent fallback to HTTP/2 and respecting the -pr http11 flag.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the HTTP/1.1 protocol enforcement: constant replacement, fallback client aliasing, and corresponding test coverage with no extraneous modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[dogancanbakir]

Hi, thanks for your interest in contributing! Just a heads up, we ask contributors to work on 1 active issue at a time (see).

Also, we welcome AI-assisted development, but submissions must be complete, tested, and ready to merge. Please also make sure to fill out the PR template with proof that your changes work.

We're closing this PR along with your other open submissions. Once you're ready, feel free to pick one issue to focus on and resubmit; we'd be happy to review it.

Appreciate your understanding!