diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index f0b47992b0b..7cb9a3eee51 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -842,7 +842,7 @@ func main() { if isAlreadyFormatted { fmt.Fprintln(os.Stderr, errMsg) } else { - fmt.Fprintln(os.Stderr, console.FormatErrorMessage(errMsg)) + fmt.Fprintln(os.Stderr, console.FormatErrorChain(err)) } os.Exit(1) } diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index c53451b97cb..1c7305c933f 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -533,11 +533,11 @@ func addWorkflowWithTracking(resolved *ResolvedWorkflow, tracker *FileTracker, o // Compile the workflow if tracker != nil { if err := compileWorkflowWithTracking(destFile, opts.Verbose, opts.Quiet, opts.EngineOverride, tracker); err != nil { - fmt.Fprintln(os.Stderr, console.FormatErrorMessage(err.Error())) + fmt.Fprintln(os.Stderr, console.FormatErrorChain(err)) } } else { if err := compileWorkflow(destFile, opts.Verbose, opts.Quiet, opts.EngineOverride); err != nil { - fmt.Fprintln(os.Stderr, console.FormatErrorMessage(err.Error())) + fmt.Fprintln(os.Stderr, console.FormatErrorChain(err)) } } diff --git a/pkg/console/console.go b/pkg/console/console.go index 606dfbc4be4..7834373bd54 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -3,6 +3,7 @@ package console import ( + "errors" "fmt" "os" "strconv" @@ -245,6 +246,86 @@ func FormatErrorMessage(message string) string { return applyStyle(styles.Error, "✗ ") + message } +// FormatErrorChain formats an error and its full unwrapped chain in a reading-friendly way. +// For wrapped errors (fmt.Errorf with %w), each level of the chain is shown on a new +// indented line. For errors whose message contains newlines (e.g. errors.Join), each +// line is indented after the first. +func FormatErrorChain(err error) string { + if err == nil { + return "" + } + + chain := unwrapErrorChain(err) + if len(chain) <= 1 { + return formatMultilineError(err.Error()) + } + + var sb strings.Builder + sb.WriteString(applyStyle(styles.Error, "✗ ")) + sb.WriteString(chain[0]) + for _, msg := range chain[1:] { + // Each message in the chain may itself contain newlines (e.g. from errors.Join + // nested inside a wrapping error); expand them all with consistent indentation. + for line := range strings.SplitSeq(msg, "\n") { + if line != "" { + sb.WriteString("\n ") + sb.WriteString(line) + } + } + } + return sb.String() +} + +// unwrapErrorChain walks the error chain via errors.Unwrap and returns a slice of +// individual message contributions, from outermost to innermost. Each entry contains +// only the message added at that level (i.e. the inner error's message is stripped). +func unwrapErrorChain(err error) []string { + var chain []string + current := err + for current != nil { + next := errors.Unwrap(current) + if next == nil { + chain = append(chain, current.Error()) + break + } + outerMsg := current.Error() + innerMsg := next.Error() + // Strip the inner error's message from the current error's message + // to isolate this level's own contribution. This assumes the standard + // fmt.Errorf("prefix: %w", inner) pattern (colon-space separator). + // If the pattern does not match, the full message is used as a fallback + // so no information is lost. + suffix := ": " + innerMsg + if strings.HasSuffix(outerMsg, suffix) { + chain = append(chain, outerMsg[:len(outerMsg)-len(suffix)]) + } else { + // Format does not follow the standard ": %w" pattern; keep the full message. + chain = append(chain, outerMsg) + } + current = next + } + return chain +} + +// formatMultilineError formats a plain error message, indenting any newlines so that +// continuation lines are visually subordinate to the leading "✗" prefix. +func formatMultilineError(msg string) string { + if !strings.Contains(msg, "\n") { + return FormatErrorMessage(msg) + } + lines := strings.Split(msg, "\n") + var sb strings.Builder + sb.WriteString(applyStyle(styles.Error, "✗ ")) + sb.WriteString(lines[0]) + for _, line := range lines[1:] { + if line != "" { + sb.WriteString("\n ") + sb.WriteString(line) + } + } + return sb.String() +} + // FormatSectionHeader formats a section header with proper styling func FormatSectionHeader(header string) string { if isTTY() { diff --git a/pkg/console/console_formatting_test.go b/pkg/console/console_formatting_test.go index f96cd19e067..86fa02f6ef8 100644 --- a/pkg/console/console_formatting_test.go +++ b/pkg/console/console_formatting_test.go @@ -3,6 +3,8 @@ package console import ( + "errors" + "fmt" "strings" "testing" ) @@ -375,3 +377,104 @@ func TestFormattingFunctionsWithUnicodeCharacters(t *testing.T) { } }) } + +func TestFormatErrorChain(t *testing.T) { + tests := []struct { + name string + err error + expectedContains []string + expectMultiLine bool + }{ + { + name: "nil error", + err: nil, + expectedContains: []string{}, + expectMultiLine: false, + }, + { + name: "simple single error", + err: errors.New("file not found"), + expectedContains: []string{"✗", "file not found"}, + expectMultiLine: false, + }, + { + name: "two-level wrapped error", + err: fmt.Errorf("outer: %w", errors.New("inner cause")), + expectedContains: []string{"✗", "outer", "inner cause"}, + expectMultiLine: true, + }, + { + name: "three-level wrapped error chain", + err: fmt.Errorf("workflow not found: %w", + fmt.Errorf("failed to download: %w", + errors.New("HTTP 404: Not Found"))), + expectedContains: []string{"✗", "workflow not found", "failed to download", "HTTP 404: Not Found"}, + expectMultiLine: true, + }, + { + name: "multiline error from errors.Join", + err: errors.Join(errors.New("error one"), errors.New("error two")), + expectedContains: []string{"✗", "error one", "error two"}, + expectMultiLine: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := FormatErrorChain(tt.err) + + for _, expected := range tt.expectedContains { + if !strings.Contains(result, expected) { + t.Errorf("FormatErrorChain() = %q, should contain %q", result, expected) + } + } + + if tt.expectMultiLine && !strings.Contains(result, "\n") { + t.Errorf("FormatErrorChain() should produce multi-line output for wrapped/joined errors, got: %q", result) + } + + // Verify indentation: continuation lines should start with spaces, first line should not + if tt.expectMultiLine && tt.err != nil { + lines := strings.Split(result, "\n") + if len(lines) > 1 { + // First line must start with the ✗ symbol (not spaces) + if strings.HasPrefix(lines[0], " ") { + t.Errorf("FormatErrorChain() first line should not be indented, got: %q", lines[0]) + } + if !strings.Contains(lines[0], "✗") { + t.Errorf("FormatErrorChain() first line should contain ✗ symbol, got: %q", lines[0]) + } + // Continuation lines must be indented + for _, line := range lines[1:] { + if line != "" && !strings.HasPrefix(line, " ") { + t.Errorf("FormatErrorChain() continuation line should be indented with 2 spaces, got: %q", line) + } + } + } + } + }) + } +} + +// TestFormatErrorChainDoesNotRepeatContext verifies that the chain format does not +// duplicate inner messages on the first line when errors are properly wrapped. +func TestFormatErrorChainDoesNotRepeatContext(t *testing.T) { + inner := errors.New("HTTP 404: Not Found") + middle := fmt.Errorf("failed to fetch file: %w", inner) + outer := fmt.Errorf("workflow not found: %w", middle) + + result := FormatErrorChain(outer) + lines := strings.Split(result, "\n") + + if len(lines) < 3 { + t.Fatalf("expected at least 3 lines, got %d: %q", len(lines), result) + } + + // First line should only contain the outermost message prefix, not inner messages + if strings.Contains(lines[0], "failed to fetch file") { + t.Errorf("first line should not contain middle message, got: %q", lines[0]) + } + if strings.Contains(lines[0], "HTTP 404") { + t.Errorf("first line should not contain innermost message, got: %q", lines[0]) + } +}