fix: log errors in GitHub workflow connection retry loop#663
fix: log errors in GitHub workflow connection retry loop#663dev-punia-altimate wants to merge 1 commit intomainfrom
Conversation
Category: silent-catch Severity: high Repo: altimate-code
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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)
github/index.ts (1)
284-286: Good improvement - adds visibility to silent retry failures.The logging provides helpful debugging context. One minor optional improvement: consider using 1-based indexing (
retry + 1) for the attempt number in logs, as operators may find "attempt 1/30" more intuitive than "retry 0/30":💡 Optional: Use 1-based attempt numbering
} catch (e) { - console.warn(`[assertOpencodeConnected] retry ${retry}/30 failed:`, e instanceof Error ? e.message : e) + console.warn(`[assertOpencodeConnected] attempt ${retry + 1}/30 failed:`, e instanceof Error ? e.message : e) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@github/index.ts` around lines 284 - 286, The log inside the catch block of assertOpencodeConnected currently prints the retry variable (0-based), update the message to display 1-based attempt numbering by using (retry + 1) so logs read "attempt 1/30" instead of "retry 0/30"; locate the catch in github/index.ts where the warn is emitted (the line with console.warn(`[assertOpencodeConnected] retry ${retry}/30 failed:`...)) and change the formatted attempt count to use retry + 1 while keeping the rest of the message and error handling intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@github/index.ts`:
- Around line 284-286: The log inside the catch block of assertOpencodeConnected
currently prints the retry variable (0-based), update the message to display
1-based attempt numbering by using (retry + 1) so logs read "attempt 1/30"
instead of "retry 0/30"; locate the catch in github/index.ts where the warn is
emitted (the line with console.warn(`[assertOpencodeConnected] retry ${retry}/30
failed:`...)) and change the formatted attempt count to use retry + 1 while
keeping the rest of the message and error handling intact.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="github/index.ts">
<violation number="1" location="github/index.ts:285">
P2: Retry warning counter is off-by-one versus loop behavior, producing misleading attempt telemetry.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| break | ||
| } catch (e) {} | ||
| } catch (e) { | ||
| console.warn(`[assertOpencodeConnected] retry ${retry}/30 failed:`, e instanceof Error ? e.message : e) |
There was a problem hiding this comment.
P2: Retry warning counter is off-by-one versus loop behavior, producing misleading attempt telemetry.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At github/index.ts, line 285:
<comment>Retry warning counter is off-by-one versus loop behavior, producing misleading attempt telemetry.</comment>
<file context>
@@ -281,7 +281,9 @@ async function assertOpencodeConnected() {
break
- } catch (e) {}
+ } catch (e) {
+ console.warn(`[assertOpencodeConnected] retry ${retry}/30 failed:`, e instanceof Error ? e.message : e)
+ }
await sleep(300)
</file context>
1 participant
Proposal
Repo: altimate-code
Category: silent-catch
Severity: high
Files:
github/index.tsWhat I Found
In
assertOpencodeConnected()(line 284), the retry loop attempts to connect to the Altimate Code server up to 30 times with a 300ms delay, but thecatch (e) {}block silently swallows all errors. This means:Fix
Added
console.warn()inside the catch block to log the retry attempt number and error message. The retry logic is preserved unchanged — this only adds visibility into failures.Auto-generated by QA Autopilot Proposal Monitor. Open for 30-day human review.
Summary by cubic
Add warning logs in
assertOpencodeConnected()so connection retries in GitHub workflows show error context instead of silently failing. Retry behavior is unchanged; each failed attempt now logs retry N/30 and the error message.Written for commit 12c78f6. Summary will update on new commits.
Summary by CodeRabbit