-
Notifications
You must be signed in to change notification settings - Fork 7
Add more debugging around new project to debug failure #430
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughIncrease command execution timeout to 2 minutes; trim and conditionally log command stdout; log exit code and elapsed duration when available; detect deadline exceeded and log timeout with elapsed; on errors log concrete error type; for Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as TemplateStep
participant Runner as steps.go
participant OS as OS/Process
Caller->>Runner: Execute command (timeout = 2m)
Runner->>Runner: record startTime
Runner->>OS: start process
OS-->>Runner: stdout, exit status, process state
Runner->>Runner: buf = trim(stdout)
alt ProcessState present
Runner->>Runner: log exit code and elapsed
end
alt buf non-empty
Runner->>Caller: log "Command output: <buf>"
end
alt success
Runner->>Caller: return success
else timeout
Runner->>Caller: log timeout with elapsed, return error (uses buf)
else error
Runner->>Caller: log error value and concrete type
alt command == "uv add" and exit code == 130
Runner->>OS: run "uv pip install" (Stdin=nil, timeout)
OS-->>Runner: result
alt failed
Runner->>OS: run "pip install" (Stdin=nil, timeout)
OS-->>Runner: result
end
end
Runner->>Caller: return error (trimmed buf or aggregated error)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/templates/steps.go (3)
46-46: Make timeout configurable; avoid hard-coded magic numberBumping to 2 minutes is reasonable for debugging, but encode this as a constant or a configurable value to avoid scattered magic numbers and to tune without code changes.
Apply within the selected line range:
-timeoutCtx, cancel := context.WithTimeout(ctx.Context, 2*time.Minute) +timeoutCtx, cancel := context.WithTimeout(ctx.Context, stepCmdTimeout)Add this near the top of the file (outside the selected range):
const stepCmdTimeout = 2 * time.Minute
54-60: Cap and sanitize logged command output to prevent excessive logs and accidental secret leakageLogging the full CombinedOutput can flood logs for verbose commands. Truncate to a safe upper bound.
Apply this diff inside the block:
buf := strings.TrimSpace(string(out)) if buf != "" { + const maxLogBytes = 32 * 1024 + if len(buf) > maxLogBytes { + buf = buf[:maxLogBytes] + fmt.Sprintf("... [truncated %d bytes]", len(buf)-maxLogBytes) + } ctx.Logger.Debug("Command output: %s", buf) if cmd.ProcessState != nil { ctx.Logger.Debug("command exit code %d", cmd.ProcessState.ExitCode()) } }
98-98: Include exit code in returned error for quicker diagnosisYou already log exit code in debug; include it in the returned error to surface the signal/status in upstream logs and UIs.
- return fmt.Errorf("failed to run command: %s with args: %s: %w (%s)", s.Command, strings.Join(s.Args, " "), err, buf) + exitCode := -1 + if cmd.ProcessState != nil { + exitCode = cmd.ProcessState.ExitCode() + } + return fmt.Errorf("failed to run command: %s with args: %s: %w (exit=%d, out=%s)", + s.Command, strings.Join(s.Args, " "), err, exitCode, buf)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/templates/steps.go(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Analyze (go)
🔇 Additional comments (1)
internal/templates/steps.go (1)
63-63: Nice: log error type alongside valueIncluding the concrete error type greatly aids debugging. Good addition.
Summary by CodeRabbit
Bug Fixes
Chores