fix(executor): strip k3d's logrus INFO noise from user-facing errors#220
Conversation
k3d logs every step to stderr in logrus format, so a failed k3d command embedded a wall of INFO[0000] progress lines into the error text — raw lines outside the CLI's formatting that drowned the one ERRO/FATA line explaining the failure. CommandError and WSLError now filter informational logrus/logfmt records (INFO/DEBU/TRAC, level=info/debug/trace) before embedding stderr; warnings, errors, fatals, and non-logrus output pass through, with a raw-text fallback when nothing survives. The full stderr stays on the field for verbose paths. Helm/docker output is unaffected (they don't use logrus).
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds Helm values preflight validation, fail-fast handling for persistent deterministic Argo CD manifest errors, and filtering of informational subprocess log lines from executor error messages. ChangesHelm values preflight
Fatal deterministic manifest handling
Executor error detail filtering
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Bootstrap
participant ValuesPreflight
participant HelmManager
participant Cluster
Bootstrap->>ValuesPreflight: validate Helm values
ValuesPreflight-->>Bootstrap: validation result
Bootstrap->>Cluster: create cluster after success
HelmManager->>ValuesPreflight: validate installation values
ValuesPreflight-->>HelmManager: validation result
HelmManager->>HelmManager: execute Helm upgrade when valid
sequenceDiagram
participant WaitForApplications
participant fatalManifestTracker
participant ArgoCD
WaitForApplications->>ArgoCD: read application conditions
WaitForApplications->>fatalManifestTracker: observe deterministic failures
fatalManifestTracker-->>WaitForApplications: fatal result after thresholds
WaitForApplications-->>WaitForApplications: return aggregated fatal error
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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)
internal/shared/executor/executor.go (1)
38-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winApply stderr truncation to
WSLErrorfor consistency.
CommandErrortruncates its stderr detail usingmaxStderrInErrorto prevent massive log walls from overwhelming the terminal. SinceWSLErrorwraps similar executions (like k3d/helm) and can produce similarly huge stderr outputs on failure, consider applying the same truncation here for consistency and better UX.♻️ Proposed refactor to truncate WSL error detail
// Same log-noise strip as CommandError: k3d/helm run via WSL on Windows, // so this error carries the same logrus progress wall on failure. if detail := errorDetail(e.Stderr); detail != "" { + if len(detail) > maxStderrInError { + detail = "..." + detail[len(detail)-maxStderrInError:] + } msg += fmt.Sprintf(": %s", detail) }🤖 Prompt for 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. In `@internal/shared/executor/executor.go` around lines 38 - 42, Update the WSLError stderr-detail handling around errorDetail(e.Stderr) to apply the same maxStderrInError truncation used by CommandError before appending the detail to msg. Preserve the existing empty-detail check and message formatting while ensuring large stderr output cannot overwhelm logs.
🤖 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.
Nitpick comments:
In `@internal/shared/executor/executor.go`:
- Around line 38-42: Update the WSLError stderr-detail handling around
errorDetail(e.Stderr) to apply the same maxStderrInError truncation used by
CommandError before appending the detail to msg. Preserve the existing
empty-detail check and message formatting while ensuring large stderr output
cannot overwhelm logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: eb40db35-8871-4540-8c14-78739d5faf78
📒 Files selected for processing (14)
internal/bootstrap/service.gointernal/chart/providers/argocd/assess.gointernal/chart/providers/argocd/fatalmanifest.gointernal/chart/providers/argocd/fatalmanifest_test.gointernal/chart/providers/argocd/values.gointernal/chart/providers/argocd/values_preflight_test.gointernal/chart/providers/argocd/wait.gointernal/chart/providers/helm/manager.gointernal/chart/providers/helm/uservalues_test.gointernal/chart/services/chart_service.gointernal/chart/services/preflight.gointernal/shared/executor/executor.gointernal/shared/executor/lognoise.gointernal/shared/executor/lognoise_test.go
WSLError embedded unbounded stderr into its message; the maxStderrInError tail-truncation now lives in the shared errorDetail helper, deduplicating CommandError's inline copy.
2 participants
k3d logs every step to stderr in logrus format, so a failed k3d command embedded a wall of INFO[0000] progress lines into the error text — raw lines outside the CLI's formatting that drowned the one ERRO/FATA line explaining the failure. CommandError and WSLError now filter informational logrus/logfmt records (INFO/DEBU/TRAC, level=info/debug/trace) before embedding stderr; warnings, errors, fatals, and non-logrus output pass through, with a raw-text fallback when nothing survives. The full stderr stays on the field for verbose paths. Helm/docker output is unaffected (they don't use logrus).
Summary by CodeRabbit
Bug Fixes
Reliability
Tests