feat: add retry logic with exponential backoff for URL timeout checks #237
Conversation
When URLs timeout during the download check, the script now retries up to 10 times with exponential backoff (1s, 2s, 4s, 8s... capped at 60s). This helps avoid false failures from transient network issues while not retrying actual 404s or other real errors. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds retry logic with exponential backoff specifically for URL timeout failures during download URL validation. The implementation separates timeout errors from other failures and retries only timeouts, helping avoid false CI failures from transient network issues while maintaining quick failure for actual broken links.
Key changes:
- Refactored the original
checkUrlfunction intocheckUrlOncefor single attempts - Added
retryTimeoutsfunction with exponential backoff (up to 10 retries, capped at 60s delay) - Modified main logic to separate timeouts from other failures and retry only timeouts
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses Copilot review feedback - removed the misleading wrapper function and call checkUrlOnce directly. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const stillTimedOut = []; | ||
| for (const result of retryResults) { | ||
| if (result.status === "TIMEOUT") { | ||
| stillTimedOut.push(result); | ||
| } else { | ||
| successful.push(result); | ||
| console.log( | ||
| ` ${colors.green}✓${colors.reset} ${result.url} - ${result.status === "OK" ? "OK" : result.status}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Bug: Non-timeout failures during retries are incorrectly treated as successful. When a previously timed-out URL is retried and returns "FAILED" or "ERROR" status (not "TIMEOUT"), it gets added to the successful array and is logged with a success checkmark. This means these failed URLs won't be reported as failures in the final results.
The logic should only add results with status === "OK" to the successful array. Failed and error results should be tracked separately and returned as failures.
| const maxRetries = 10; | ||
| const baseDelay = 1000; // Start with 1 second | ||
| let remaining = [...timedOutResults]; | ||
| const successful = []; |
There was a problem hiding this comment.
Misleading variable name: successful contains all non-timeout results (including "FAILED" and "ERROR" statuses), not just successful results. Consider renaming to retried or completed to better reflect its actual content.
| console.log( | ||
| `${colors.green}All timed out URLs recovered after ${attempt} retry attempt(s)!${colors.reset}` | ||
| ); |
There was a problem hiding this comment.
Misleading success message: The message claims "All timed out URLs recovered" but this isn't accurate. A URL that was retried and returned "FAILED" or "ERROR" status hasn't truly "recovered" - it just didn't timeout again. The message should distinguish between URLs that became accessible (status "OK") versus those that simply returned a different failure status.
| console.log( | |
| `${colors.green}All timed out URLs recovered after ${attempt} retry attempt(s)!${colors.reset}` | |
| ); | |
| const okCount = successful.filter(r => r.status === "OK").length; | |
| const nonOkCount = successful.length - okCount; | |
| let msg = `${colors.green}${okCount} timed out URL(s) recovered (status OK) after ${attempt} retry attempt(s).${colors.reset}`; | |
| if (nonOkCount > 0) { | |
| msg += ` ${colors.yellow}${nonOkCount} URL(s) returned non-timeout failure status (e.g. FAILED, ERROR).${colors.reset}`; | |
| } | |
| console.log(msg); |
| console.log( | ||
| ` ${colors.green}✓${colors.reset} ${result.url} - ${result.status === "OK" ? "OK" : result.status}` | ||
| ); |
There was a problem hiding this comment.
Misleading success indicator: A green checkmark is displayed for retried URLs even when they return "FAILED" or "ERROR" status. This gives the false impression that these URLs are now working. Only URLs with status === "OK" should receive a success checkmark.
| console.log( | |
| ` ${colors.green}✓${colors.reset} ${result.url} - ${result.status === "OK" ? "OK" : result.status}` | |
| ); | |
| if (result.status === "OK") { | |
| console.log( | |
| ` ${colors.green}✓${colors.reset} ${result.url} - OK` | |
| ); | |
| } else if (result.status === "FAILED" || result.status === "ERROR") { | |
| console.log( | |
| ` ${colors.red}✗${colors.reset} ${result.url} - ${result.status}` | |
| ); | |
| } else { | |
| console.log( | |
| ` ${colors.yellow}!${colors.reset} ${result.url} - ${result.status}` | |
| ); | |
| } |
Summary
This helps avoid false CI failures from transient network issues while still catching real broken links.
Test plan
npm run check:downloadslocally to verify script works🤖 Generated with Claude Code