-
Notifications
You must be signed in to change notification settings - Fork 25
feat: add retry logic with exponential backoff for URL timeout checks #237
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,8 +40,8 @@ function extractUrls(obj, path = "") { | |||||||||||||||||||||||||||||||||
| return urls; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Check if URL is accessible | ||||||||||||||||||||||||||||||||||
| function checkUrl(urlInfo) { | ||||||||||||||||||||||||||||||||||
| // Check if URL is accessible (single attempt) | ||||||||||||||||||||||||||||||||||
| function checkUrlOnce(urlInfo) { | ||||||||||||||||||||||||||||||||||
| return new Promise((resolve) => { | ||||||||||||||||||||||||||||||||||
| const url = new URL(urlInfo.url); | ||||||||||||||||||||||||||||||||||
| const protocol = url.protocol === "https:" ? https : http; | ||||||||||||||||||||||||||||||||||
|
|
@@ -76,6 +76,58 @@ function checkUrl(urlInfo) { | |||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Retry timed out URLs with exponential backoff | ||||||||||||||||||||||||||||||||||
| async function retryTimeouts(timedOutResults) { | ||||||||||||||||||||||||||||||||||
| if (timedOutResults.length === 0) return []; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const maxRetries = 10; | ||||||||||||||||||||||||||||||||||
| const baseDelay = 1000; // Start with 1 second | ||||||||||||||||||||||||||||||||||
| let remaining = [...timedOutResults]; | ||||||||||||||||||||||||||||||||||
| const successful = []; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| for ( | ||||||||||||||||||||||||||||||||||
| let attempt = 1; | ||||||||||||||||||||||||||||||||||
| attempt <= maxRetries && remaining.length > 0; | ||||||||||||||||||||||||||||||||||
| attempt++ | ||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||
| // Exponential backoff: 1s, 2s, 4s, 8s, 16s, 32s, 60s (capped), 60s, 60s, 60s | ||||||||||||||||||||||||||||||||||
| const delay = Math.min(baseDelay * Math.pow(2, attempt - 1), 60000); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| console.log( | ||||||||||||||||||||||||||||||||||
| `\n${colors.yellow}Retry attempt ${attempt}/${maxRetries} for ${remaining.length} timed out URL(s) (waiting ${delay / 1000}s)...${colors.reset}` | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| await new Promise((resolve) => setTimeout(resolve, delay)); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const retryResults = await Promise.all( | ||||||||||||||||||||||||||||||||||
| remaining.map((r) => checkUrlOnce({ url: r.url, path: r.path })) | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| 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}` | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+112
to
+114
|
||||||||||||||||||||||||||||||||||
| 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}` | |
| ); | |
| } |
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Copilot
AI
Nov 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading variable name:
successfulcontains all non-timeout results (including "FAILED" and "ERROR" statuses), not just successful results. Consider renaming toretriedorcompletedto better reflect its actual content.