chore: Improve console error experience (#124)

* chore: Improve console error experience * fix: Use standard error code and handle verbosity
2026-08-03 07:24:09 +02:00 · 2026-01-17 14:05:01 +05:30
parent 80a1747e3e
commit 6a3821d44a
6 changed files with 438 additions and 34 deletions
@@ -5,19 +5,23 @@ import "github.com/fatih/color"
 type ColorFn func(format string, a ...interface{}) string

 type TerminalColors struct {
-	Normal ColorFn
-	Red    ColorFn
-	Yellow ColorFn
-	Cyan   ColorFn
-	Green  ColorFn
-	Bold   ColorFn
+	Normal    ColorFn
+	Red       ColorFn
+	Yellow    ColorFn
+	Cyan      ColorFn
+	Green     ColorFn
+	Bold      ColorFn
+	Dim       ColorFn
+	ErrorCode ColorFn
 }

 var Colors = TerminalColors{
-	Normal: color.New().SprintfFunc(),
-	Red:    color.New(color.FgRed, color.Bold).SprintfFunc(),
-	Yellow: color.New(color.FgYellow).SprintfFunc(),
-	Cyan:   color.New(color.FgCyan).SprintfFunc(),
-	Green:  color.New(color.FgGreen).SprintfFunc(),
-	Bold:   color.New(color.Bold).SprintfFunc(),
+	Normal:    color.New().SprintfFunc(),
+	Red:       color.New(color.FgRed, color.Bold).SprintfFunc(),
+	Yellow:    color.New(color.FgYellow).SprintfFunc(),
+	Cyan:      color.New(color.FgCyan).SprintfFunc(),
+	Green:     color.New(color.FgGreen).SprintfFunc(),
+	Bold:      color.New(color.Bold).SprintfFunc(),
+	Dim:       color.New(color.Faint).SprintfFunc(),
+	ErrorCode: color.New(color.BgRed, color.FgBlack, color.Bold).SprintfFunc(),
 }
@@ -8,26 +8,57 @@ import (
 	"github.com/safedep/pmg/usefulerror"
 )

-// ErrorExit prints the error message and exits the program with a non-zero status code.
+// ErrorExit prints a minimal, clean error message and exits with a non-zero status code.
 func ErrorExit(err error) {
 	log.Errorf("Exiting due to error: %s", err)

-	usefulErr, ok := usefulerror.AsUsefulError(err)
-	if !ok {
-		Fatalf("Error: %s", err)
-	}
-
-	additionalHelp := usefulErr.AdditionalHelp()
-	if additionalHelp == "" {
-		additionalHelp = fmt.Sprintf("If you believe this is a bug, please report it at: %s",
-			"https://github.com/safedep/pmg/issues/new?assignees=&labels=bug")
-	}
+	usefulErr := convertToUsefulError(err)

 	ClearStatus()

-	fmt.Println(Colors.Red(fmt.Sprintf("Error occurred: %s", usefulErr.HumanError())))
-	fmt.Println(Colors.Yellow(usefulErr.Help()))
-	fmt.Println(Colors.Yellow(additionalHelp))
+	// Use help as hint, but for unknown errors show bug report link
+	hint := usefulErr.Help()
+	if usefulErr.Code() == usefulerror.ErrCodeUnknown {
+		hint = "Report this issue: https://github.com/safedep/pmg/issues/new?labels=bug"
+	}
+
+	if verbosityLevel == VerbosityLevelVerbose {
+		printVerboseError(usefulErr.Code(), usefulErr.HumanError(), hint,
+			usefulErr.AdditionalHelp(), usefulErr.Error())
+	} else {
+		printMinimalError(usefulErr.Code(), usefulErr.HumanError(), hint)
+	}

 	os.Exit(1)
 }
+
+// printMinimalError prints error in minimal two-line format:
+// Line 1: Error code (red background) + message (red)
+// Line 2: Actionable hint with arrow prefix (dimmed)
+func printMinimalError(code, message, hint string) {
+	// Line 1: Error code + message
+	fmt.Printf("%s  %s\n", Colors.ErrorCode(" %s ", code), Colors.Red(message))
+
+	// Line 2: Actionable hint with arrow (only if meaningful)
+	if hint != "" && hint != "No additional help is available for this error." {
+		fmt.Printf(" %s %s\n", Colors.Dim("→"), Colors.Dim(hint))
+	}
+}
+
+// printVerboseError prints detailed error for debugging (--verbose mode)
+// Includes additional help and original error chain for troubleshooting
+func printVerboseError(code, message, hint, additionalHelp, originalError string) {
+	fmt.Printf("%s  %s\n", Colors.ErrorCode(" %s ", code), Colors.Red(message))
+
+	if hint != "" && hint != "No additional help is available for this error." {
+		fmt.Printf(" %s %s\n", Colors.Dim("→"), Colors.Dim(hint))
+	}
+
+	if additionalHelp != "" && additionalHelp != "No additional help is available for this error." {
+		fmt.Printf(" %s %s\n", Colors.Dim("→"), Colors.Dim(additionalHelp))
+	}
+
+	if originalError != "" && originalError != message {
+		fmt.Printf(" %s %s\n", Colors.Dim("┄"), Colors.Dim(originalError))
+	}
+}
@@ -0,0 +1,207 @@
+package ui
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"io/fs"
+	"net"
+	"os"
+	"os/exec"
+	"strings"
+
+	"github.com/safedep/pmg/usefulerror"
+)
+
+// errorMatcher defines how to detect and convert a specific error type
+type errorMatcher struct {
+	match   func(err error) bool
+	convert func(err error) usefulerror.UsefulError
+}
+
+// errorMatchers is an ordered list of error matchers
+// Order matters - more specific matchers should come first
+var errorMatchers = []errorMatcher{
+	// File not found errors
+	{
+		match: func(err error) bool {
+			return errors.Is(err, os.ErrNotExist) || errors.Is(err, fs.ErrNotExist)
+		},
+		convert: func(err error) usefulerror.UsefulError {
+			path := extractPathFromError(err)
+			humanError := "File or directory not found"
+			if path != "" {
+				humanError = fmt.Sprintf("File or directory not found: %s", path)
+			}
+
+			return usefulerror.Useful().
+				WithCode(usefulerror.ErrCodeNotFound).
+				WithHumanError(humanError).
+				WithHelp("Check if the path exists").
+				WithAdditionalHelp("Use 'ls' to check directory contents").
+				Wrap(err)
+		},
+	},
+	// Permission denied errors
+	{
+		match: func(err error) bool {
+			return errors.Is(err, os.ErrPermission) || errors.Is(err, fs.ErrPermission)
+		},
+		convert: func(err error) usefulerror.UsefulError {
+			path := extractPathFromError(err)
+			humanError := "Permission denied"
+			if path != "" {
+				humanError = fmt.Sprintf("Permission denied: %s", path)
+			}
+			return usefulerror.Useful().
+				WithCode(usefulerror.ErrCodePermissionDenied).
+				WithHumanError(humanError).
+				WithHelp("Check permissions or use sudo").
+				WithAdditionalHelp("Use 'ls -la' to check permissions").
+				Wrap(err)
+		},
+	},
+	// Process exit errors
+	{
+		match: func(err error) bool {
+			var exitErr *exec.ExitError
+			return errors.As(err, &exitErr)
+		},
+		convert: func(err error) usefulerror.UsefulError {
+			var exitErr *exec.ExitError
+			errors.As(err, &exitErr)
+			exitCode := exitErr.ExitCode()
+			return usefulerror.Useful().
+				WithCode(usefulerror.ErrCodeLifecycle).
+				WithHumanError(fmt.Sprintf("Command failed with exit code %d", exitCode)).
+				WithHelp("Check command output above").
+				WithAdditionalHelp("Run with PMG_DEBUG=true for more details").
+				Wrap(err)
+		},
+	},
+	// Timeout errors (check before network errors since network timeouts also match)
+	{
+		match: func(err error) bool {
+			return errors.Is(err, context.DeadlineExceeded)
+		},
+		convert: func(err error) usefulerror.UsefulError {
+			return usefulerror.Useful().
+				WithCode(usefulerror.ErrCodeTimeout).
+				WithHumanError("Operation timed out").
+				WithHelp("Try again or check your network").
+				WithAdditionalHelp("Consider increasing timeout or retry later").
+				Wrap(err)
+		},
+	},
+	// Canceled errors
+	{
+		match: func(err error) bool {
+			return errors.Is(err, context.Canceled)
+		},
+		convert: func(err error) usefulerror.UsefulError {
+			return usefulerror.Useful().
+				WithCode(usefulerror.ErrCodeCanceled).
+				WithHumanError("Operation was canceled").
+				Wrap(err)
+		},
+	},
+	// Network errors
+	{
+		match: func(err error) bool {
+			var netErr net.Error
+			if errors.As(err, &netErr) {
+				return true
+			}
+			// Also check for common network-related error messages
+			errStr := err.Error()
+			return strings.Contains(errStr, "connection refused") ||
+				strings.Contains(errStr, "no such host") ||
+				strings.Contains(errStr, "network is unreachable")
+		},
+		convert: func(err error) usefulerror.UsefulError {
+			var netErr net.Error
+			if errors.As(err, &netErr) && netErr.Timeout() {
+				return usefulerror.Useful().
+					WithCode(usefulerror.ErrCodeTimeout).
+					WithHumanError("Network request timed out").
+					WithHelp("Check your internet connection").
+					WithAdditionalHelp("Consider increasing timeout or retry later").
+					Wrap(err)
+			}
+			return usefulerror.Useful().
+				WithCode(usefulerror.ErrCodeNetwork).
+				WithHumanError("Network error occurred").
+				WithHelp("Check your internet connection").
+				WithAdditionalHelp("The package registry may be temporarily unavailable").
+				Wrap(err)
+		},
+	},
+	// Unexpected EOF errors
+	{
+		match: func(err error) bool {
+			return errors.Is(err, io.EOF) || errors.Is(err, io.ErrUnexpectedEOF)
+		},
+		convert: func(err error) usefulerror.UsefulError {
+			return usefulerror.Useful().
+				WithCode(usefulerror.ErrCodeUnexpectedEOF).
+				WithHumanError("Unexpected end of data").
+				WithHelp("Retry the download").
+				WithAdditionalHelp("This may indicate network instability").
+				Wrap(err)
+		},
+	},
+}
+
+// convertToUsefulError attempts to convert a regular error to a UsefulError
+// by analyzing the error chain for known error types.
+// Returns the original error wrapped in a generic UsefulError if no specific match is found.
+func convertToUsefulError(err error) usefulerror.UsefulError {
+	if err == nil {
+		return nil
+	}
+
+	if ue, ok := usefulerror.AsUsefulError(err); ok {
+		return ue
+	}
+
+	for _, matcher := range errorMatchers {
+		if matcher.match(err) {
+			return matcher.convert(err)
+		}
+	}
+
+	return usefulerror.Useful().
+		WithCode(usefulerror.ErrCodeUnknown).
+		WithHumanError(extractRootCause(err)).
+		WithHelp("An unexpected error occurred.").
+		Wrap(err)
+}
+
+// extractRootCause traverses the error chain and returns the innermost error message.
+// This provides a cleaner, more human-friendly message instead of the full error chain.
+func extractRootCause(err error) string {
+	for {
+		unwrapped := errors.Unwrap(err)
+		if unwrapped == nil {
+			return err.Error()
+		}
+
+		err = unwrapped
+	}
+}
+
+// extractPathFromError attempts to extract a file path from path-related errors
+func extractPathFromError(err error) string {
+	var pathErr *fs.PathError
+	if errors.As(err, &pathErr) {
+		return pathErr.Path
+	}
+
+	var linkErr *os.LinkError
+	if errors.As(err, &linkErr) {
+		return linkErr.Old
+	}
+
+	return ""
+}
@@ -0,0 +1,146 @@
+package ui
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"io/fs"
+	"os"
+	"testing"
+
+	"github.com/safedep/pmg/usefulerror"
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_convertToUsefulError(t *testing.T) {
+	tests := []struct {
+		name           string
+		inputError     error
+		wantCode       string
+		wantHumanError string
+		wantContains   string
+		wantNil        bool
+	}{
+		{
+			name: "AlreadyUseful",
+			inputError: usefulerror.Useful().
+				WithCode("CUSTOM").
+				WithHumanError("Already useful").
+				Msg("test"),
+			wantCode:       "CUSTOM",
+			wantHumanError: "Already useful",
+		},
+		{
+			name:         "FileNotExist",
+			inputError:   &fs.PathError{Op: "open", Path: "/nonexistent/file.txt", Err: os.ErrNotExist},
+			wantCode:     usefulerror.ErrCodeNotFound,
+			wantContains: "/nonexistent/file.txt",
+		},
+		{
+			name:         "PermissionDenied",
+			inputError:   &fs.PathError{Op: "open", Path: "/root/secret", Err: os.ErrPermission},
+			wantCode:     usefulerror.ErrCodePermissionDenied,
+			wantContains: "/root/secret",
+		},
+		{
+			name:         "ContextTimeout",
+			inputError:   context.DeadlineExceeded,
+			wantCode:     usefulerror.ErrCodeTimeout,
+			wantContains: "timed out",
+		},
+		{
+			name:         "ContextCanceled",
+			inputError:   context.Canceled,
+			wantCode:     usefulerror.ErrCodeCanceled,
+			wantContains: "canceled",
+		},
+		{
+			name:       "UnexpectedEOF",
+			inputError: io.ErrUnexpectedEOF,
+			wantCode:   usefulerror.ErrCodeUnexpectedEOF,
+		},
+		{
+			name:       "WrappedError",
+			inputError: fmt.Errorf("failed to read config: %w", os.ErrNotExist),
+			wantCode:   usefulerror.ErrCodeNotFound,
+		},
+		{
+			name:           "UnknownError",
+			inputError:     errors.New("some unknown error"),
+			wantCode:       usefulerror.ErrCodeUnknown,
+			wantHumanError: "some unknown error",
+		},
+		{
+			name: "UnknownWrappedError",
+			inputError: fmt.Errorf("more context: %w",
+				fmt.Errorf("outer context: %w",
+					errors.New("root cause error"))),
+			wantCode:       usefulerror.ErrCodeUnknown,
+			wantHumanError: "root cause error",
+		},
+		{
+			name:       "Nil",
+			inputError: nil,
+			wantNil:    true,
+		},
+		{
+			name:       "NetworkErrorMessage",
+			inputError: errors.New("connection refused"),
+			wantCode:   usefulerror.ErrCodeNetwork,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := convertToUsefulError(tt.inputError)
+
+			if tt.wantNil {
+				assert.Nil(t, result)
+				return
+			}
+
+			assert.NotNil(t, result)
+			assert.Equal(t, tt.wantCode, result.Code())
+
+			if tt.wantHumanError != "" {
+				assert.Equal(t, tt.wantHumanError, result.HumanError())
+			}
+
+			if tt.wantContains != "" {
+				assert.Contains(t, result.HumanError(), tt.wantContains)
+			}
+		})
+	}
+}
+
+func TestExtractPathFromError(t *testing.T) {
+	tests := []struct {
+		name     string
+		inputErr error
+		wantPath string
+	}{
+		{
+			name:     "PathError",
+			inputErr: &fs.PathError{Op: "open", Path: "/some/path", Err: os.ErrNotExist},
+			wantPath: "/some/path",
+		},
+		{
+			name:     "LinkError",
+			inputErr: &os.LinkError{Op: "link", Old: "/old/path", New: "/new/path", Err: os.ErrPermission},
+			wantPath: "/old/path",
+		},
+		{
+			name:     "generic error",
+			inputErr: errors.New("some error"),
+			wantPath: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			path := extractPathFromError(tt.inputErr)
+			assert.Equal(t, tt.wantPath, path)
+		})
+	}
+}