feat: Add download retry support to startup scripts - #2461
Conversation
|
the tests are bogus - not testing it works; just testing the scripts has retry logic. |
5e3e390 to
703aa12
Compare
|
updated tests to be "real" via wiremock test that fails so can see retry work. |
c90402b to
95893e8
Compare
Add JBANG_DOWNLOAD_RETRY env var (default: 5) to control retry attempts for JDK and JBang bootstrap downloads across all platforms: - bash: curl gets --retry/--retry-delay, wget gets --tries/--waitretry - PowerShell: new Invoke-Download function with retry loop and backoff - CMD: inherits PowerShell behavior (delegates downloads to jbang.ps1) Also adds: - JBANG_DOWNLOAD_RETRY_DELAY env var (default: 1s) for retry backoff - JBANG_DOWNLOAD_URL env var to override the JBang download URL (useful for corporate mirrors and testing)
95893e8 to
60a1226
Compare
|
ok, now tests are real, use progressive backoff by default, env vars added for user and test control + documented the various env vars. @quintesse wdyt? and should all those vars be treated equally supported ? :) |
LGTM 👍
Not sure what you mean by this? |
Wether all the env vars in the script that this PR add should be considered all equally supported/stable. |
|
Well I think most of them are okay. Perhaps the USE_NATIVE one because it's supposed to be for internal testing? |
|
@quintesse any idea why powershell specifically would fail here ..just on java 11 ? |
|
No idea really |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds configurable download retry behavior to JBang startup scripts (Bash and PowerShell), exposes ChangesDownload Retry Feature
Sequence Diagram(s)sequenceDiagram
participant StartupScript as Startup Script (Bash/PowerShell)
participant DownloadHelper as download()/Invoke-Download
participant Remote as HTTP Endpoint
StartupScript->>DownloadHelper: request URL (JBANG_DOWNLOAD_URL or computed)
loop attempts up to JBANG_DOWNLOAD_RETRY
DownloadHelper->>Remote: HTTP GET
alt success (200)
Remote-->>DownloadHelper: payload (archive)
DownloadHelper-->>StartupScript: success (save/archive)
else error (5xx/timeout)
Remote-->>DownloadHelper: error
DownloadHelper->>DownloadHelper: wait (fixed or backoff via JBANG_DOWNLOAD_RETRY_DELAY)
end
end
alt exhausted without success
DownloadHelper-->>StartupScript: failure (exit 1)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 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. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/main/scripts/jbang.ps1`:
- Around line 67-89: The Invoke-Download retry loop currently uses "$attempt -ge
$downloadRetry" which makes only $downloadRetry total attempts; change the
termination check so the function allows the initial attempt plus $downloadRetry
retries (i.e. fail only when $attempt -gt ($downloadRetry + 1) or otherwise
compare against $downloadRetry + 1) to match curl's semantics; update any
retry-progress text (the Console::Error message) to reflect the total attempts
as $downloadRetry + 1 if present.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 52da3b3a-b28b-4c25-a7d1-ed69fccd9271
📒 Files selected for processing (5)
docs/modules/ROOT/pages/installation.adocdocs/modules/ROOT/pages/troubleshooting.adocsrc/main/scripts/jbangsrc/main/scripts/jbang.ps1src/test/java/dev/jbang/cli/TestScriptRetry.java
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
PowerShell's 'break' outside a loop/switch terminates the script but always exits with code 0. This meant download failures were silently swallowed — the error message was printed to stderr but the process reported success. Replace 'break' with 'exit 1' in the two download error paths (JBang archive and JDK archive) so callers and CI can detect the failure. Also remove the stale $err reference that was left over after the Invoke-WebRequest→Invoke-Download refactor (the variable was never set by Invoke-Download, so it always printed an empty line).
CI failure analysis & fixThe failures are not Java 11-specific — unit tests only run on Java 11 in the CI matrix, so that's the only place they surface. The same tests would fail on any Java version. Two bugs found and fixedBug 1: The PowerShell script uses Fix: Replaced Bug 2: stale The old code captured the exception into |
|
llms to the rescue again - dumb logic error :) tests passes. lets merge and things should hopefully still work :) |
Fixes #2459
Problem
A single transient network error during JDK or JBang bootstrap download fails the entire setup, which is particularly painful in CI environments.
Solution
Adds a
JBANG_DOWNLOAD_RETRYenvironment variable (default:5) that controls retry attempts for downloads across all platforms:curl--retry N --retry-delay 1wget--tries=N --waitretry=1Invoke-WebRequestInvoke-Downloadretry loop with 1s backoffUsage
Changes
src/main/scripts/jbang— Added retry flags tocurlandwgetin thedownload()functionsrc/main/scripts/jbang.ps1— AddedInvoke-Downloadfunction with retry loop; replaced rawInvoke-WebRequestcalls (PS5-compatible, no dependency on PS7's-MaximumRetryCount)src/test/java/dev/jbang/cli/TestScriptRetry.java— 7 unit tests validating retry support in script contentsdocs/modules/ROOT/pages/installation.adoc— New "Download Retries" sectiondocs/modules/ROOT/pages/troubleshooting.adoc— Updated "Network Issues" sectionSummary by CodeRabbit
New Features
Documentation
Tests