-
Notifications
You must be signed in to change notification settings - Fork 21
[CFX-5789] [CFX-5790] ToolRegistry & Surface environment-aware failure messaging
#577
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
4d5d244
29b422a
5786675
42b40d6
6c10ba7
3742576
ff5cca2
e1fec45
dc1017f
a4cf6c2
43727f4
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 |
|---|---|---|
|
|
@@ -15,10 +15,13 @@ | |
| package dependencies | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "os/exec" | ||
| "runtime" | ||
| "strings" | ||
|
|
||
| "github.com/datarobot/cli/internal/log" | ||
| "github.com/datarobot/cli/internal/repo" | ||
|
|
@@ -45,7 +48,9 @@ func InstallPrerequisites(w io.Writer, prerequisites []tools.Prerequisite) ([]st | |
|
|
||
| fmt.Fprintf(w, "📦 Installing %s...\n", prerequisite.Name) | ||
|
|
||
| exitCode, err := ExecuteShLine(installCmd, w) | ||
| var cmdBuf bytes.Buffer | ||
|
|
||
| exitCode, err := ExecuteShLine(installCmd, io.MultiWriter(w, &cmdBuf)) | ||
| if err != nil { | ||
| log.Debug("deps: install failed to start", "name", prerequisite.Name, "err", err) | ||
|
|
||
|
|
@@ -55,7 +60,13 @@ func InstallPrerequisites(w io.Writer, prerequisites []tools.Prerequisite) ([]st | |
| if exitCode != 0 { | ||
| log.Debug("deps: install exited non-zero", "name", prerequisite.Name, "exit_code", exitCode) | ||
|
|
||
| return installed, fmt.Errorf("install failed for %q (exit code %d)\n Please run manually: %s\n Or check %s", prerequisite.Name, exitCode, installCmd, prerequisite.URL) | ||
| env := DetectEnvironment() | ||
| permDenied := isPermissionDenied(exitCode, cmdBuf.String()) | ||
| msg := buildInstallFailureMsg(prerequisite, exitCode, permDenied, env, runtime.GOOS) | ||
|
|
||
| fmt.Fprint(w, msg) | ||
|
|
||
| return installed, fmt.Errorf("install failed for %q (exit code %d)", prerequisite.Name, exitCode) | ||
| } | ||
|
|
||
| log.Debug("deps: tool installed", "name", prerequisite.Name) | ||
|
|
@@ -81,6 +92,102 @@ func InstallPrerequisites(w io.Writer, prerequisites []tools.Prerequisite) ([]st | |
| return installed, nil | ||
| } | ||
|
|
||
| // isPermissionDenied inspects exit codes and stderr text across OS types. | ||
| func isPermissionDenied(exitCode int, stderr string) bool { | ||
|
ajalon1 marked this conversation as resolved.
|
||
| stderrLower := strings.ToLower(stderr) | ||
|
|
||
| if strings.Contains(stderrLower, "permission denied") || | ||
| strings.Contains(stderrLower, "operation not permitted") || | ||
| strings.Contains(stderrLower, "access is denied") || | ||
| strings.Contains(stderrLower, "requires root privileges") || | ||
| strings.Contains(stderrLower, "unauthorizedaccessexception") { | ||
| return true | ||
| } | ||
|
|
||
| // Unix-like systems (Linux & macOS) return 126 when a file lacks execute permissions | ||
| if (runtime.GOOS == "linux" || runtime.GOOS == "darwin") && exitCode == 126 { | ||
| return true | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exit 126 ignored on WindowsMedium Severity Permission-denied handling treats exit code Additional Locations (1)Triggered by project rule: Bugbot Rules for DataRobot CLI Reviewed by Cursor Bugbot for commit 6c10ba7. Configure here. |
||
|
|
||
| return false | ||
| } | ||
|
|
||
| // extractFailedManager heuristically identifies the package/version manager | ||
| // referenced in cmd (e.g. "brew" in "brew install uv"). Returns "" if none found. | ||
| func extractFailedManager(cmd string) string { | ||
| for _, m := range knownManagers { | ||
| if strings.Contains(cmd, m) { | ||
| return m | ||
| } | ||
| } | ||
|
|
||
| return "" | ||
|
ajalon1 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // buildInstallFailureMsg composes the user-facing failure message for a failed install. | ||
| // env and goos are injectable for testing. | ||
| func buildInstallFailureMsg(prerequisite tools.Prerequisite, exitCode int, permDenied bool, env map[string]bool, goos string) string { | ||
| toolName := prerequisite.Name | ||
|
|
||
| installCmd, _ := prerequisite.PlatformInstallCommand() | ||
|
|
||
| var sb strings.Builder | ||
|
|
||
| if permDenied { | ||
| fmt.Fprintf(&sb, "✗ %s install failed (exit code %d — permission denied)\n", toolName, exitCode) | ||
| } else { | ||
| fmt.Fprintf(&sb, "✗ %s install failed (exit code %d)\n", toolName, exitCode) | ||
| } | ||
|
|
||
| fmt.Fprintf(&sb, TAB+"Tried: %s\n", installCmd) | ||
|
|
||
| if tip := buildInstallTip(prerequisite, permDenied, env, goos); tip != "" { | ||
| fmt.Fprintf(&sb, "%s\n", tip) | ||
| } | ||
|
|
||
| fmt.Fprintf(&sb, TAB+"Raw command if you want to retry: %s\n", installCmd) | ||
|
|
||
| if prerequisite.URL != "" { | ||
| fmt.Fprintf(&sb, TAB+"Refer to %s for manual installation instructions.\n", prerequisite.URL) | ||
| } | ||
|
|
||
| return sb.String() | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // buildInstallTip returns the optional tip line for buildInstallFailureMsg, or "" when | ||
| // no actionable suggestion is available. | ||
| func buildInstallTip(prerequisite tools.Prerequisite, permDenied bool, env map[string]bool, goos string) string { | ||
| if permDenied { | ||
| switch goos { | ||
| case "windows": | ||
| return TAB + "Tip: This action requires Administrator privileges. Please restart your terminal/tool as Administrator." | ||
| case "darwin", "linux": | ||
| return TAB + "Tip: This action requires root privileges. Please re-run this tool using 'sudo'." | ||
| default: | ||
| return TAB + "Tip: Administrative or root privileges are required to perform this action." | ||
| } | ||
|
cursor[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| toolKey := prerequisite.Key | ||
| if toolKey == "" { | ||
| toolKey = NormalizeToolName(prerequisite.Name) | ||
| } | ||
|
|
||
| if toolKey == "" { | ||
| return "" | ||
| } | ||
|
|
||
| installCmd, _ := prerequisite.PlatformInstallCommand() | ||
| failedMgr := extractFailedManager(installCmd) | ||
|
|
||
| strategy := selectInstallStrategy(toolKey, failedMgr, env) | ||
| if strategy == nil { | ||
| return "" | ||
| } | ||
|
|
||
| return strategy.withVersion(prerequisite.MinimumVersion).getStrategyTip(goos) | ||
|
ajalon1 marked this conversation as resolved.
|
||
| } | ||
|
|
||
| // ExecuteShLine executes shellCmd via sh -c, streaming stdout and stderr | ||
| // to w in real time. Handles pipe-based commands (e.g. curl ... | sh). | ||
| // Returns the process exit code; a non-nil error indicates the process could | ||
|
|
||


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.
Start flow hides install failures
High Severity
Detailed install failure text is written only to the
io.Writer, while the returnederroris a short exit-code message. The quickstart TUI installs deps into a buffer and on failure displays onlym.err, so users lose tips, the tried command, and streamed command output that this change added.Triggered by project rule: Bugbot Rules for DataRobot CLI
Reviewed by Cursor Bugbot for commit 29b422a. Configure here.