Improve error output when executing step fails

CHANGELOG.md

@@ -15,6 +15,8 @@ All notable changes to `src-cli` are documented in this file.
 
 ### Changed
 
+- Error reporting by `src campaign [preview|apply]` has been improved and now includes more information about which step failed in which repository. [#325](https://github.com/sourcegraph/src-cli/pull/325)
+
 ### Fixed
 
 - Log files created by `src campaigns [preview|apply]` are deleted again after successful execution. This was a regression and is not new behaviour. If steps failed to execute or the `-keep-logs` flag is set the log files are not cleaned up.

cmd/src/campaigns_apply.go

@@ -63,11 +63,8 @@ Examples:
 		})
 
 		if err := doApply(ctx, out, svc, flags); err != nil {
-			out.Write("")
-			block := out.Block(output.Line("❌", output.StyleWarning, "Error"))
-			block.Write(err.Error())
-			block.Close()
-			out.Write("")
+			printExecutionError(out, err)
+			return &exitCodeError{nil, 1}
 		}
 
 		return nil

cmd/src/campaigns_common.go

@@ -3,13 +3,15 @@ package main
 import (
 	"context"
 	"flag"
+	"fmt"
 	"io"
 	"os"
 	"path"
 	"runtime"
 	"time"
 
 	"github.com/hashicorp/go-multierror"
+	"github.com/neelance/parallel"
 	"github.com/pkg/errors"
 	"github.com/sourcegraph/src-cli/internal/api"
 	"github.com/sourcegraph/src-cli/internal/campaigns"
@@ -277,3 +279,55 @@ func campaignsExecute(ctx context.Context, out *output.Output, svc *campaigns.Se
 
 	return id, url, nil
 }
+
+// printExecutionError is used to print the possible error returned by
+// campaignsExecute.
+func printExecutionError(out *output.Output, err error) {
+	out.Write("")
+
+	writeErr := func(block *output.Block, err error) {
+		if block == nil {
+			return
+		}
+
+		if taskErr, ok := err.(campaigns.TaskExecutionErr); ok {
+			block.Write(formatTaskExecutionErr(taskErr))
+		} else {
+			block.Write(err.Error())
+		}
+	}
+
+	var block *output.Block
+	singleErrHeader := output.Line(output.EmojiFailure, output.StyleWarning, "Error:")
+
+	if parErr, ok := err.(parallel.Errors); ok {
+		if len(parErr) > 1 {
+			block = out.Block(output.Linef(output.EmojiFailure, output.StyleWarning, "%d errors:", len(parErr)))
+		} else {
+			block = out.Block(singleErrHeader)
+		}
+
+		for _, e := range parErr {
+			writeErr(block, e)
+		}
+	} else {
+		block = out.Block(singleErrHeader)
+		writeErr(block, err)
+	}
+
+	if block != nil {
+		block.Close()
+	}
+	out.Write("")
+}
+
+func formatTaskExecutionErr(err campaigns.TaskExecutionErr) string {
+	return fmt.Sprintf(
+		"%s%s%s:\n%s\nLog: %s\n",
+		output.StyleBold,
+		err.Repository,
+		output.StyleReset,
+		err.Err,
+		err.Logfile,
+	)
+}

cmd/src/campaigns_preview.go

@@ -41,11 +41,7 @@ Examples:
 
 		_, url, err := campaignsExecute(ctx, out, svc, flags)
 		if err != nil {
-			out.Write("")
-			block := out.Block(output.Line("❌", output.StyleWarning, "Error"))
-			block.Write(err.Error())
-			block.Close()
-			out.Write("")
+			printExecutionError(out, err)
 			return &exitCodeError{nil, 1}
 		}
 

internal/campaigns/executor.go

@@ -13,6 +13,21 @@ import (
 	"github.com/sourcegraph/src-cli/internal/campaigns/graphql"
 )
 
+type TaskExecutionErr struct {
+	Err        error
+	Logfile    string
+	Repository string
+}
+
+func (e TaskExecutionErr) Error() string {
+	return fmt.Sprintf(
+		"execution in %s failed: %s (see %s for details)",
+		e.Repository,
+		e.Err,
+		e.Logfile,
+	)
+}
+
 type Executor interface {
 	AddTask(repo *graphql.Repository, steps []Step, template *ChangesetTemplate) *TaskStatus
 	LogFiles() []string
@@ -186,7 +201,11 @@ func (x *executor) do(ctx context.Context, task *Task) (err error) {
 	}
 	defer func() {
 		if err != nil {
-			err = errors.Wrapf(err, "Error executing task. The log file can be found at:\n%s\n", log.Path())
+			err = TaskExecutionErr{
+				Err:        err,
+				Logfile:    log.Path(),
+				Repository: task.Repository.Name,
+			}
 			log.MarkErrored()
 		}
 		log.Close()

internal/campaigns/run_steps.go

@@ -70,7 +70,7 @@ func runSteps(ctx context.Context, client api.Client, repo *graphql.Repository,
 	}
 
 	for i, step := range steps {
-		logger.Logf("[Step %d] docker run %s", i+1, step.image)
+		logger.Logf("[Step %d] docker run %s %q", i+1, step.Container, step.Run)
 
 		cidFile, err := ioutil.TempFile(tempDir, prefix+"-container-id")
 		if err != nil {
@@ -120,8 +120,10 @@ func runSteps(ctx context.Context, client api.Client, repo *graphql.Repository,
 		}
 		cmd.Args = append(cmd.Args, "--", step.image, containerTemp)
 		cmd.Dir = volumeDir
-		cmd.Stdout = logger.PrefixWriter("stdout")
-		cmd.Stderr = logger.PrefixWriter("stderr")
+
+		var stdoutBuffer, stderrBuffer bytes.Buffer
+		cmd.Stdout = io.MultiWriter(&stdoutBuffer, logger.PrefixWriter("stdout"))
+		cmd.Stderr = io.MultiWriter(&stderrBuffer, logger.PrefixWriter("stderr"))
 
 		a, err := json.Marshal(cmd.Args)
 		if err != nil {
@@ -134,7 +136,16 @@ func runSteps(ctx context.Context, client api.Client, repo *graphql.Repository,
 		elapsed := time.Since(t0).Round(time.Millisecond)
 		if err != nil {
 			logger.Logf("[Step %d] took %s; error running Docker container: %+v", i+1, elapsed, err)
-			return nil, errors.Wrapf(err, "Running Docker container for image %q failed", step.image)
+
+			return nil, stepFailedErr{
+				Err:         err,
+				Args:        cmd.Args,
+				Run:         step.Run,
+				Container:   step.Container,
+				TmpFilename: containerTemp,
+				Stdout:      strings.TrimSpace(stdoutBuffer.String()),
+				Stderr:      strings.TrimSpace(stderrBuffer.String()),
+			}
 		}
 		logger.Logf("[Step %d] complete in %s", i+1, elapsed)
 
@@ -297,3 +308,57 @@ func probeImageForShell(ctx context.Context, image string) (shell, tempfile stri
 	// admit defeat and return. At least err is already in place.
 	return
 }
+
+type stepFailedErr struct {
+	Run       string
+	Container string
+
+	TmpFilename string
+
+	Args   []string
+	Stdout string
+	Stderr string
+
+	Err error
+}
+
+func (e stepFailedErr) Error() string {
+	var out strings.Builder
+
+	fmtRun := func(run string) string {
+		lines := strings.Split(run, "\n")
+		if len(lines) == 1 {
+			return lines[0]
+		}
+		return lines[0] + fmt.Sprintf("\n\t(... and %d more lines)", len(lines)-1)
+	}
+
+	out.WriteString(fmt.Sprintf("run: %s\ncontainer: %s\n", fmtRun(e.Run), e.Container))
+
+	printOutput := func(output string) {
+		for _, line := range strings.Split(output, "\n") {
+			if e.TmpFilename != "" {
+				line = strings.ReplaceAll(line, e.TmpFilename+": ", "")
+			}
+			out.WriteString("\t" + line + "\n")
+		}
+	}
+
+	if len(e.Stdout) > 0 {
+		out.WriteString("\nstandard out:\n")
+		printOutput(e.Stdout)
+	}
+
+	if len(e.Stderr) > 0 {
+		out.WriteString("\nstandard error:\n")
+		printOutput(e.Stderr)
+	}
+
+	if exitErr, ok := e.Err.(*exec.ExitError); ok {
+		out.WriteString(fmt.Sprintf("\nCommand failed with exit code %d.", exitErr.ExitCode()))
+	} else {
+		out.WriteString(fmt.Sprintf("\nCommand failed: %s", e.Err))
+	}
+
+	return out.String()
+}

internal/campaigns/service.go

@@ -196,7 +196,7 @@ func (svc *Service) ExecuteCampaignSpec(ctx context.Context, repos []*graphql.Re
 		done <- struct{}{}
 	}
 	if err != nil {
-		return nil, errors.Wrap(err, "executing campaign spec")
+		return nil, err
 	}
 
 	// Add external changeset specs.

internal/output/style.go

@@ -33,6 +33,10 @@ var (
 	StyleWarning = Fg256Color(124)
 	StyleSuccess = Fg256Color(2)
 
+	StyleBold      = &style{"\033[1m"}
+	StyleItalic    = &style{"\033[3m"}
+	StyleUnderline = &style{"\033[4m"}
+
 	// Search-specific colors.
 	StyleSearchQuery         = Fg256Color(68)
 	StyleSearchBorder        = Fg256Color(239)