fix: Fix installer retries for HTTP/2 download failures#1910
Conversation
📝 WalkthroughWalkthroughThis pull request adds HTTP/1.1 fallback support to the installer script to handle transient GitHub/CDN HTTP/2 framing errors. Three curl wrapper functions retry failed requests using HTTP/1.1, are integrated into metadata and asset downloads, verified by new smoke tests, and documented in CI and the release manual. ChangesHTTP/1.1 Fallback for Curl Operations
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 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. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/test_install.sh (1)
33-91: ⚡ Quick winHarden fallback tests to assert retry order, not just flag presence.
These checks should also verify exactly two curl calls and that
--http1.1is used only on the second call, so the “normal path first, fallback second” contract is protected.♻️ Suggested test hardening
+assert_retry_shape() { + local calls="$1" label="$2" + IFS='|' read -r _ first second extra <<< "${calls}" + if [[ -z "${second:-}" || -n "${extra:-}" ]]; then + echo "FAIL: ${label} should issue exactly 2 curl calls (base + http1.1 retry)" + exit 1 + fi + if [[ "${first}" == *"--http1.1"* || "${second}" != *"--http1.1"* ]]; then + echo "FAIL: ${label} should retry with --http1.1 only on the second call" + exit 1 + fi +} + ( CURL_CALLS="" curl() { @@ if ! curl_head_with_http_fallback "https://example.invalid/OpenHuman.app.tar.gz"; then echo "FAIL: reachability fallback should succeed when HTTP/1.1 retry succeeds" exit 1 fi - if [[ "${CURL_CALLS}" != *"--http1.1"* ]]; then - echo "FAIL: reachability check did not retry with --http1.1" - exit 1 - fi + assert_retry_shape "${CURL_CALLS}" "reachability check" )Apply the same
assert_retry_shapecall to the metadata and download blocks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/test_install.sh` around lines 33 - 91, The tests currently only assert that "--http1.1" appears in CURL_CALLS; update each of the three test blocks (the ones invoking curl_head_with_http_fallback, curl_get_file, and curl_download_file) to assert the retry shape: ensure CURL_CALLS contains exactly two recorded curl invocations and that only the second invocation includes "--http1.1" (i.e., the normal path is first, the fallback second). Use the existing CURL_CALLS variable and pattern checks to count entries and verify the second entry includes "--http1.1" while the first does not.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/test_install.sh`:
- Around line 33-91: The tests currently only assert that "--http1.1" appears in
CURL_CALLS; update each of the three test blocks (the ones invoking
curl_head_with_http_fallback, curl_get_file, and curl_download_file) to assert
the retry shape: ensure CURL_CALLS contains exactly two recorded curl
invocations and that only the second invocation includes "--http1.1" (i.e., the
normal path is first, the fallback second). Use the existing CURL_CALLS variable
and pattern checks to count entries and verify the second entry includes
"--http1.1" while the first does not.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 974c29d3-b7da-4d08-a34f-382ac2fc38f1
📒 Files selected for processing (4)
.github/workflows/installer-smoke.ymldocs/RELEASE-MANUAL-SMOKE.mdscripts/install.shscripts/test_install.sh
|
Completed the reinforcement of retry-shape tests as requested by CodeRabbit. What changed:
Validation run:
|
|
Heads up: the failing It fails during Appium setup before the E2E tests start:
I triggered a fresh CI run with an empty |
chiehwangs
changed the title
2 participants
Summary
Problem
Solution
scripts/install.shthat first use the existing curl behavior, then retry once with--http1.1when the initial request fails.latest.json, releases API metadata fallback, digest metadata refresh, asset HEAD reachability checks, and final asset downloads.scripts/test_install.shwith fake-curl cases for metadata fetches, HEAD checks, and asset downloads that fail once with exit 16 and then succeed with--http1.1..github/workflows/installer-smoke.ymland update the release manual smoke checklist for public installer runs through proxy/VPN networks.Submission Checklist
scripts/test_install.shadds focused failure-path coverage.docs/TEST-COVERAGE-MATRIX.md.## Related— N/A: no coverage-matrix feature IDs are affected.docs/RELEASE-MANUAL-SMOKE.md)Closes #NNNin the## Relatedsection — N/A: no upstream GitHub issue or Linear issue was provided for this contribution.Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/installer-http1-fallback28cabc62e6b9fa35208b563def3a26caa285e088Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckbash scripts/test_install.sh;bash -n scripts/install.sh && bash -n scripts/test_install.sh;bash scripts/install.sh --dry-run --verbose;git diff --check;sh .husky/pre-pushValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
--http1.1after an initial failure, addressing VPN/proxy-sensitive HTTP/2 framing failures.Parity Contract
Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Tests
Documentation
Chores