Fix error handling for exec of "launch" command

Makefile

@@ -18,18 +18,15 @@ export GOPROXY=https://proxy.golang.org
 # Disable CGO so that we always generate static binaries:
 export CGO_ENABLED=0
 
+default: build
 
 .PHONY: build
 build:
-	goreleaser build --snapshot  --clean --single-target
+	$(GOENV) go build -o build/$(PROJECT_NAME) .
 
 .PHONY: install
-install:
-	go build -o ${GOPATH}/bin/srepd .
-
-.PHONY: install-local
-install-local: build
-	cp dist/*/srepd ~/.local/bin/srepd
+install: build
+	cp build/srepd ~/.local/bin/$(PROJECT_NAME)
 
 .PHONY: mod
 mod:
@@ -52,9 +49,15 @@ getlint:
 lint: getlint
 	$(GOPATH)/bin/golangci-lint run --timeout 5m
 
+.PHONY: ensure-goreleaser
 ensure-goreleaser:
 	@ls $(GOPATH)/bin/goreleaser 1>/dev/null || go install github.com/goreleaser/goreleaser@${GORELEASER_VERSION}
 
+.PHONY: goreleaser-build
+goreleaser-build: ensure-goreleaser
+	goreleaser build --snapshot  --clean --single-target
+
+.PHONY: release
 release: ensure-goreleaser
 	goreleaser release --clean
 

README.md

@@ -39,10 +39,10 @@ Configuration variables have the following precedence:
 
 * ignoredusers: A list of PagerDuty user IDs to exclude from retrieved incident lists.  It's recommended that the "silentuser" ID is in this list.
 * editor: Your choice of editor.  Defaults to the `$EDITOR` environment variable.
-* cluster_login_cmd: Command used to login to a cluster from SREPD.  Defaults to `/usr/local/bin/ocm backplane login`
+* cluster_login_command: Command used to login to a cluster from SREPD.  Defaults to `/usr/local/bin/ocm backplane login`
 * terminal: Your choice of terminal to use when launching external commands. Defaults to `/usr/bin/gnome-terminal`.
 
-__NOTE:__ The cluster_login_cmd and terminal accept a variable for `%%CLUSTER_ID%%` to stand in for the Cluster ID in the command. At least one of the two, most likely `cluster_login_cmd` MUST have the `%%CLUSTER_ID%%` placeholder set. See [AUTOMATIC LOGIN FEATURES](#automatic-login-features) for more details about config variables.
+__NOTE:__ The cluster_login_command and terminal accept a variable for `%%CLUSTER_ID%%` to stand in for the Cluster ID in the command. At least one of the two, most likely `cluster_login_command` MUST have the `%%CLUSTER_ID%%` placeholder set. See [AUTOMATIC LOGIN FEATURES](#automatic-login-features) for more details about config variables.
 
 An example srepd.yaml file might look like so:
 
@@ -56,7 +56,7 @@ editor: vim
 # Note the trailing `--` is necessary for gnome-terminal and may be necessary
 # for other terminals as well
 terminal: /usr/bin/gnome-terminal --
-cluster_login_cmd: ocm-container --clusterid %%CLUSTER_ID%%
+cluster_login_command: ocm-container --clusterid %%CLUSTER_ID%%
 
 # Note that aliases, etc, are not sourced by the shell command when launching.
 # This means, for example, that `ocm-container`, as normally setup using an

cmd/root.go

@@ -69,7 +69,8 @@ but rather a simple tool to make on-call tasks easier.`,
 
 		launcher, err := launcher.NewClusterLauncher(viper.GetString("terminal"), viper.GetString("cluster_login_command"))
 		if err != nil {
-			log.Warn(err)
+			fmt.Println(err)
+			log.Fatal(err)
 		}
 
 		m, _ := tui.InitialModel(
@@ -93,6 +94,7 @@ but rather a simple tool to make on-call tasks easier.`,
 
 		_, err = p.Run()
 		if err != nil {
+			fmt.Println(err)
 			log.Fatal(err)
 		}
 	},

pkg/launcher/launcher.go

@@ -2,10 +2,14 @@ package launcher
 
 import (
 	"fmt"
-	"github.com/charmbracelet/log"
 	"strings"
+
+	"github.com/charmbracelet/log"
 )
 
+const clusterLoginCommandFlag = "cluster_login_command"
+const terminalFlag = "terminal"
+
 type ClusterLauncher struct {
 	Enabled             bool
 	terminal            []string
@@ -37,19 +41,19 @@ func (l *ClusterLauncher) validate() error {
 	errs := []error{}
 
 	if l.terminal == nil || l.terminal[0] == "" {
-		errs = append(errs, fmt.Errorf("terminal is not set"))
+		errs = append(errs, fmt.Errorf("%s is not set", terminalFlag))
 	}
 
 	if l.clusterLoginCommand == nil || l.clusterLoginCommand[0] == "" {
-		errs = append(errs, fmt.Errorf("clusterLoginCommand is not set"))
+		errs = append(errs, fmt.Errorf("%s is not set", clusterLoginCommandFlag))
 	}
 
 	if len(l.terminal) > 0 && strings.Contains(l.terminal[0], "%%") {
-		errs = append(errs, fmt.Errorf("first terminal argument cannot have a replaceable"))
+		errs = append(errs, fmt.Errorf("first terminal argument cannot have a replaceable value"))
 	}
 
 	if (!strings.Contains(strings.Join(l.clusterLoginCommand, " "), "%%CLUSTER_ID%%")) && (!strings.Contains(strings.Join(l.terminal, " "), "%%CLUSTER_ID%%")) {
-		errs = append(errs, fmt.Errorf("clusterLoginCommand must contain %%CLUSTER_ID%%"))
+		errs = append(errs, fmt.Errorf("%s must contain %%CLUSTER_ID%%", clusterLoginCommandFlag))
 	}
 
 	if len(errs) > 0 {
@@ -67,7 +71,7 @@ func (l *ClusterLauncher) BuildLoginCommand(vars map[string]string) []string {
 	// Handle the Terminal command
 	// The first arg should not be something replaceable, as checked in the
 	// validate function
-	log.Debug("launcher.ClusterLauncher():", "Building command from terminal", "terminal", l.terminal[0])
+	log.Debug("launcher.ClusterLauncher(): building command from terminal", "terminal", l.terminal[0])
 	command = append(command, l.terminal[0])
 
 	// If there are more than one terminal arguments, replace the vars
@@ -77,9 +81,9 @@ func (l *ClusterLauncher) BuildLoginCommand(vars map[string]string) []string {
 		command = append(command, replaceVars(l.terminal[1:], vars)...)
 	}
 	command = append(command, replaceVars(l.clusterLoginCommand, vars)...)
-	log.Debug("launcher.ClusterLauncher():", "Built command", command)
+	log.Debug("launcher.ClusterLauncher(): built command", "command", command)
 	for x, i := range command {
-		log.Debug("launcher.ClusterLauncher():", fmt.Sprintf("Built command argument [%d]", x), i)
+		log.Debug("launcher.ClusterLauncher(): build command argument", fmt.Sprintf("[%d]", x), i)
 	}
 
 	return command
@@ -93,10 +97,12 @@ func replaceVars(args []string, vars map[string]string) []string {
 	str := strings.Join(args, " ")
 
 	for k, v := range vars {
-		log.Debug("ClusterLauncher():", "Replacing vars in string", str, k, v)
+		log.Debug("launcher.replaceVars(): Replacing vars in string", k, v)
 		str = strings.Replace(str, k, v, -1)
 	}
 
+	log.Debug("launcher.replaceVars(): Replaced vars in string", "string", str)
+
 	transformedArgs := strings.Split(str, " ")
 	return transformedArgs
 }

pkg/tui/commands.go

@@ -33,6 +33,27 @@ const (
 	loadingIncidentsStatus = "loading incidents..."
 )
 
+type execErr struct {
+	Err        error
+	ExitErr    *exec.ExitError
+	ExecStdErr string
+}
+
+func (ee *execErr) Error() string {
+	// Podman will return errors with the same formatting as this program
+	// so strip out `Error: ` prefix and `\n` suffixes, since ocm-container
+	// will just put them back
+	s := ee.ExecStdErr
+	s = strings.TrimPrefix(s, "Error: ")
+	s = strings.TrimPrefix(s, "error: ")
+	s = strings.TrimSuffix(s, "\n")
+	return s
+}
+
+func (ee *execErr) Code() int {
+	return ee.ExitErr.ExitCode()
+}
+
 // TODO: Deprecate incoming message
 // getIncidentMsg is a message that triggers the fetching of an incident
 type getIncidentMsg string
@@ -411,51 +432,100 @@ func login(vars map[string]string, launcher launcher.ClusterLauncher) tea.Cmd {
 	c := exec.Command(command[0], command[1:]...)
 
 	log.Debug(fmt.Sprintf("tui.login(): %v", c.String()))
-	stderr, pipeErr := c.StderrPipe()
-	if pipeErr != nil {
-		log.Debug("tui.login():", "pipeErr", pipeErr.Error())
+
+	var stdOut io.ReadCloser
+	var stdOutPipeErr error
+	stdOut, stdOutPipeErr = c.StdoutPipe()
+	if stdOutPipeErr != nil {
+		log.Warn("tui.login():", "stdOutPipeErr", stdOutPipeErr.Error())
 		return func() tea.Msg {
-			return loginFinishedMsg{err: pipeErr}
+			return loginFinishedMsg{stdOutPipeErr}
 		}
 	}
 
-	err := c.Start()
-	if err != nil {
-		log.Debug("tui.login():", "startErr", err.Error())
+	var stdErr io.ReadCloser
+	var stdErrPipeErr error
+	stdErr, stdErrPipeErr = c.StderrPipe()
+	if stdErrPipeErr != nil {
+		log.Warn("tui.login():", "stdErrErr", stdErrPipeErr.Error())
 		return func() tea.Msg {
-			return loginFinishedMsg{err}
+			return loginFinishedMsg{stdErrPipeErr}
 		}
 	}
 
-	out, err := io.ReadAll(stderr)
-	if err != nil {
-		log.Debug("tui.login():", "loginStdErr", err.Error())
+	startCmdErr := c.Start()
+	if startCmdErr != nil {
+		log.Warn("tui.login():", "startCmdErr", startCmdErr.Error())
+		return func() tea.Msg {
+			return loginFinishedMsg{startCmdErr}
+		}
+	}
+
+	var out []byte
+	var stdOutReadErr error
+	out, stdOutReadErr = io.ReadAll(stdOut)
+	if stdOutReadErr != nil {
+		log.Warn("tui.login():", "stdOutReadErr", stdOutReadErr.Error())
+		return func() tea.Msg {
+			return loginFinishedMsg{stdOutReadErr}
+		}
+	}
+
+	var errOut []byte
+	var stdErrReadErr error
+	errOut, stdErrReadErr = io.ReadAll(stdErr)
+	if stdErrReadErr != nil {
+		log.Warn("tui.login():", "stdErrReadErr", stdErrReadErr.Error())
+		return func() tea.Msg {
+			return loginFinishedMsg{stdErrReadErr}
+		}
+	}
+
+	var err error
+	if len(errOut) > 0 {
+		err = errors.New(string(errOut))
+	}
+
+	processErr := c.Wait()
+
+	if processErr != nil {
+		if exitError, ok := processErr.(*exec.ExitError); ok {
+			execExitErr := &execErr{
+				Err:        processErr,
+				ExitErr:    exitError,
+				ExecStdErr: string(errOut),
+			}
+			log.Warn("tui.login():", "processErr", execExitErr)
+			return func() tea.Msg {
+				return loginFinishedMsg{execExitErr}
+			}
+		}
+		log.Warn("tui.login():", "processErr", processErr.Error())
 		return func() tea.Msg {
-			return loginFinishedMsg{err}
+			return loginFinishedMsg{processErr}
 		}
 	}
 
-	// c.Wait() must be run after reading any output from stderrPipe
-	err = c.Wait()
 	if err != nil {
-		log.Debug("tui.login():", "waitErr", err.Error())
+		log.Warn("tui.login():", "execStdErr", err.Error())
 		return func() tea.Msg {
-			return loginFinishedMsg{err}
+			// Do not return the execStdErr as an error
+			return loginFinishedMsg{}
 		}
 	}
 
+	var stdOutAsErr error
 	if len(out) > 0 {
-		outErr := fmt.Errorf("%s", out)
-		log.Debug("tui.login():", "outErr", outErr.Error())
+		stdOutAsErr = errors.New(string(out))
+		log.Warn("tui.login():", "stdOutAsErr", stdOutAsErr.Error())
 		return func() tea.Msg {
-			return loginFinishedMsg{outErr}
+			return loginFinishedMsg{stdOutAsErr}
 		}
 	}
 
 	return func() tea.Msg {
-		return loginFinishedMsg{err}
+		return loginFinishedMsg{}
 	}
-
 }
 
 type clearSelectedIncidentsMsg string