From 000d18c2edabc237bda89e0f837597018082c616 Mon Sep 17 00:00:00 2001 From: Shetty Gaurav Jagadeesha Shetty Date: Wed, 25 Mar 2026 16:58:18 +0530 Subject: [PATCH] feat: add structured error handling for JSON/YAML output --- cmd/lint/lint.go | 30 ++- cmd/main.go | 9 +- pkg/util/client/config.go | 4 +- pkg/util/errors/errors.go | 192 +++++++++++++++++ pkg/util/errors/errors_test.go | 379 +++++++++++++++++++++++++++++++++ pkg/util/errors/output.go | 94 ++++++++ pkg/util/errors/output_test.go | 127 +++++++++++ pkg/util/errors/types.go | 75 +++++++ 8 files changed, 904 insertions(+), 6 deletions(-) create mode 100644 pkg/util/errors/errors.go create mode 100644 pkg/util/errors/errors_test.go create mode 100644 pkg/util/errors/output.go create mode 100644 pkg/util/errors/output_test.go create mode 100644 pkg/util/errors/types.go diff --git a/cmd/lint/lint.go b/cmd/lint/lint.go index 6a830b90..7a3048b2 100644 --- a/cmd/lint/lint.go +++ b/cmd/lint/lint.go @@ -9,6 +9,7 @@ import ( "k8s.io/cli-runtime/pkg/genericiooptions" lintpkg "github.com/opendatahub-io/odh-cli/pkg/lint" + clierrors "github.com/opendatahub-io/odh-cli/pkg/util/errors" ) const ( @@ -85,32 +86,55 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) { SilenceUsage: true, SilenceErrors: true, // We'll handle error output manually based on --quiet flag RunE: func(cmd *cobra.Command, _ []string) error { + outputFormat := string(command.OutputFormat) + // Complete phase if err := command.Complete(); err != nil { + if clierrors.WriteStructuredError(cmd.ErrOrStderr(), err, outputFormat) { + return clierrors.NewAlreadyHandledError(err) + } + if command.Verbose { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", err) + clierrors.WriteSuggestion(cmd.ErrOrStderr(), err) + } else { + clierrors.WriteTextError(cmd.ErrOrStderr(), err) } - return fmt.Errorf("completing command: %w", err) + return clierrors.NewAlreadyHandledError(err) } // Validate phase if err := command.Validate(); err != nil { + if clierrors.WriteStructuredError(cmd.ErrOrStderr(), err, outputFormat) { + return clierrors.NewAlreadyHandledError(err) + } + if command.Verbose { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", err) + clierrors.WriteSuggestion(cmd.ErrOrStderr(), err) + } else { + clierrors.WriteTextError(cmd.ErrOrStderr(), err) } - return fmt.Errorf("validating command: %w", err) + return clierrors.NewAlreadyHandledError(err) } // Run phase err := command.Run(cmd.Context()) if err != nil { + if clierrors.WriteStructuredError(cmd.ErrOrStderr(), err, outputFormat) { + return clierrors.NewAlreadyHandledError(err) + } + if command.Verbose { _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", err) + clierrors.WriteSuggestion(cmd.ErrOrStderr(), err) + } else { + clierrors.WriteTextError(cmd.ErrOrStderr(), err) } - return fmt.Errorf("running command: %w", err) + return clierrors.NewAlreadyHandledError(err) } return nil diff --git a/cmd/main.go b/cmd/main.go index 543a9f93..d35aebe4 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -1,6 +1,7 @@ package main import ( + "errors" "os" "github.com/spf13/cobra" @@ -9,6 +10,7 @@ import ( "github.com/opendatahub-io/odh-cli/cmd/lint" "github.com/opendatahub-io/odh-cli/cmd/version" + clierrors "github.com/opendatahub-io/odh-cli/pkg/util/errors" ) func main() { @@ -29,9 +31,12 @@ func main() { lint.AddCommand(cmd, flags) if err := cmd.Execute(); err != nil { - if _, writeErr := os.Stderr.WriteString(err.Error() + "\n"); writeErr != nil { - os.Exit(1) + if !errors.Is(err, clierrors.ErrAlreadyHandled) { + if _, writeErr := os.Stderr.WriteString(err.Error() + "\n"); writeErr != nil { + os.Exit(1) + } } + os.Exit(1) } } diff --git a/pkg/util/client/config.go b/pkg/util/client/config.go index d0b58d93..f6723eb0 100644 --- a/pkg/util/client/config.go +++ b/pkg/util/client/config.go @@ -5,6 +5,8 @@ import ( "k8s.io/cli-runtime/pkg/genericclioptions" "k8s.io/client-go/rest" + + clierrors "github.com/opendatahub-io/odh-cli/pkg/util/errors" ) const ( @@ -42,7 +44,7 @@ func NewRESTConfig( ) (*rest.Config, error) { restConfig, err := configFlags.ToRESTConfig() if err != nil { - return nil, fmt.Errorf("failed to create REST config: %w", err) + return nil, fmt.Errorf("failed to create REST config: %w", clierrors.NewConfigError(err)) } ConfigureThrottling(restConfig, qps, burst) diff --git a/pkg/util/errors/errors.go b/pkg/util/errors/errors.go new file mode 100644 index 00000000..9a95b230 --- /dev/null +++ b/pkg/util/errors/errors.go @@ -0,0 +1,192 @@ +package errors + +import ( + "context" + "errors" + "io/fs" + "net" + + apierrors "k8s.io/apimachinery/pkg/api/errors" +) + +const ( + suggestionAuthentication = "Refresh your kubeconfig credentials with 'oc login' or 'kubectl config'" + suggestionAuthorization = "Verify your RBAC permissions for the required resources" + suggestionConnection = "Check if the API server is reachable" + suggestionNotFound = "Verify the resource exists in the cluster" + suggestionAlreadyExists = "Resource already exists, use update or delete first" + suggestionConflict = "Retry the operation (resource was modified concurrently)" + suggestionValidation = "Check the request parameters and resource spec" + suggestionGone = "Resource version expired, retry with a fresh list/watch" + suggestionServer = "API server error, retry later" + suggestionTimeout = "Increase --timeout value or retry the operation" + suggestionRateLimited = "Too many requests, retry after a brief wait" + suggestionRequestTooLarge = "Reduce the size of the request payload" + suggestionInternal = "Unexpected error, please report a bug" + suggestionCanceled = "Operation was canceled" + suggestionFilePath = "Verify the file path exists and is readable (e.g. --kubeconfig)" + suggestionConfig = "Check your kubeconfig: verify the --context, --cluster, and --kubeconfig flags are correct" +) + +// apiErrorEntry maps an apierrors check function to its structured error fields. +type apiErrorEntry struct { + check func(error) bool + code string + category ErrorCategory + retriable bool + suggestion string +} + +// apiErrorTable defines the classification for every Kubernetes API error type. +// Order matters: more specific checks (e.g. IsUnexpectedServerError) must +// appear before broader ones (e.g. IsInternalError) that match the same status code. +// +//nolint:gochecknoglobals // package-level lookup table is intentional +var apiErrorTable = []apiErrorEntry{ + {apierrors.IsUnauthorized, "AUTH_FAILED", CategoryAuthentication, false, suggestionAuthentication}, + {apierrors.IsForbidden, "AUTHZ_DENIED", CategoryAuthorization, false, suggestionAuthorization}, + {apierrors.IsNotFound, "NOT_FOUND", CategoryNotFound, false, suggestionNotFound}, + {apierrors.IsAlreadyExists, "ALREADY_EXISTS", CategoryConflict, false, suggestionAlreadyExists}, + {apierrors.IsConflict, "CONFLICT", CategoryConflict, true, suggestionConflict}, + {apierrors.IsInvalid, "INVALID", CategoryValidation, false, suggestionValidation}, + {apierrors.IsBadRequest, "BAD_REQUEST", CategoryValidation, false, suggestionValidation}, + {apierrors.IsMethodNotSupported, "METHOD_NOT_SUPPORTED", CategoryValidation, false, suggestionValidation}, + {apierrors.IsNotAcceptable, "NOT_ACCEPTABLE", CategoryValidation, false, suggestionValidation}, + {apierrors.IsUnsupportedMediaType, "UNSUPPORTED_MEDIA_TYPE", CategoryValidation, false, suggestionValidation}, + {apierrors.IsRequestEntityTooLargeError, "REQUEST_TOO_LARGE", CategoryValidation, false, suggestionRequestTooLarge}, + {apierrors.IsGone, "GONE", CategoryServer, true, suggestionGone}, + {apierrors.IsResourceExpired, "RESOURCE_EXPIRED", CategoryServer, true, suggestionGone}, + {apierrors.IsServerTimeout, "SERVER_TIMEOUT", CategoryTimeout, true, suggestionTimeout}, + {apierrors.IsServiceUnavailable, "SERVER_UNAVAILABLE", CategoryServer, true, suggestionServer}, + {apierrors.IsUnexpectedServerError, "UNEXPECTED_SERVER_ERROR", CategoryServer, true, suggestionServer}, + {apierrors.IsInternalError, "SERVER_ERROR", CategoryServer, true, suggestionServer}, + {apierrors.IsTimeout, "GATEWAY_TIMEOUT", CategoryTimeout, true, suggestionTimeout}, + {apierrors.IsTooManyRequests, "RATE_LIMITED", CategoryServer, true, suggestionRateLimited}, + {apierrors.IsUnexpectedObjectError, "UNEXPECTED_OBJECT", CategoryServer, false, suggestionServer}, + {apierrors.IsStoreReadError, "STORE_READ_ERROR", CategoryServer, true, suggestionServer}, +} + +// Classify inspects an error and returns a StructuredError with the +// appropriate category, error code, retriable flag, and suggestion. +func Classify(err error) *StructuredError { + if err == nil { + return nil + } + + var structuredErr *StructuredError + if errors.As(err, &structuredErr) && structuredErr != nil { + return structuredErr + } + + for _, entry := range apiErrorTable { + if entry.check(err) { + return &StructuredError{ + Code: entry.code, + Message: err.Error(), + Category: entry.category, + Retriable: entry.retriable, + Suggestion: entry.suggestion, + cause: err, + } + } + } + + switch { + case errors.Is(err, context.DeadlineExceeded): + return &StructuredError{ + Code: "TIMEOUT", + Message: err.Error(), + Category: CategoryTimeout, + Retriable: true, + Suggestion: suggestionTimeout, + cause: err, + } + + case errors.Is(err, context.Canceled): + return &StructuredError{ + Code: "CANCELED", + Message: err.Error(), + Category: CategoryInternal, + Retriable: false, + Suggestion: suggestionCanceled, + cause: err, + } + + case isFilesystemError(err): + return &StructuredError{ + Code: "CONFIG_INVALID", + Message: err.Error(), + Category: CategoryValidation, + Retriable: false, + Suggestion: suggestionFilePath, + cause: err, + } + + case isConfigError(err): + return &StructuredError{ + Code: "CONFIG_INVALID", + Message: err.Error(), + Category: CategoryValidation, + Retriable: false, + Suggestion: suggestionConfig, + cause: err, + } + + case isNetworkTimeout(err): + return &StructuredError{ + Code: "NET_TIMEOUT", + Message: err.Error(), + Category: CategoryTimeout, + Retriable: true, + Suggestion: suggestionTimeout, + cause: err, + } + + case isNetworkError(err): + return &StructuredError{ + Code: "CONN_FAILED", + Message: err.Error(), + Category: CategoryConnection, + Retriable: true, + Suggestion: suggestionConnection, + cause: err, + } + + default: + return &StructuredError{ + Code: "INTERNAL", + Message: err.Error(), + Category: CategoryInternal, + Retriable: false, + Suggestion: suggestionInternal, + cause: err, + } + } +} + +func isFilesystemError(err error) bool { + var pathErr *fs.PathError + + return errors.As(err, &pathErr) +} + +func isConfigError(err error) bool { + var cfgErr *ConfigError + + return errors.As(err, &cfgErr) +} + +func isNetworkError(err error) bool { + var netErr net.Error + + return errors.As(err, &netErr) +} + +func isNetworkTimeout(err error) bool { + var netErr net.Error + if errors.As(err, &netErr) { + return netErr.Timeout() + } + + return false +} diff --git a/pkg/util/errors/errors_test.go b/pkg/util/errors/errors_test.go new file mode 100644 index 00000000..e17d87a9 --- /dev/null +++ b/pkg/util/errors/errors_test.go @@ -0,0 +1,379 @@ +package errors_test + +import ( + "context" + "errors" + "fmt" + "io/fs" + "net/http" + "testing" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/validation/field" + + clierrors "github.com/opendatahub-io/odh-cli/pkg/util/errors" + + . "github.com/onsi/gomega" +) + +// testNetError implements net.Error for testing network error classification. +type testNetError struct { + timeout bool +} + +func (e *testNetError) Error() string { return "network error" } +func (e *testNetError) Timeout() bool { return e.timeout } +func (e *testNetError) Temporary() bool { return false } + +func TestClassify(t *testing.T) { + t.Run("should classify unauthorized as authentication", func(t *testing.T) { + g := NewWithT(t) + err := apierrors.NewUnauthorized("token expired") + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryAuthentication))) + g.Expect(result).To(HaveField("Code", Equal("AUTH_FAILED"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + g.Expect(result).To(HaveField("Suggestion", Not(BeEmpty()))) + g.Expect(result.Unwrap()).To(Equal(err)) + }) + + t.Run("should classify forbidden as authorization", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewForbidden(gr, "test", errors.New("access denied")) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryAuthorization))) + g.Expect(result).To(HaveField("Code", Equal("AUTHZ_DENIED"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify not found", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewNotFound(gr, "my-deploy") + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryNotFound))) + g.Expect(result).To(HaveField("Code", Equal("NOT_FOUND"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify conflict as retriable", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewConflict(gr, "my-deploy", errors.New("version changed")) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryConflict))) + g.Expect(result).To(HaveField("Code", Equal("CONFLICT"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify server timeout as timeout", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewServerTimeout(gr, "list", 5) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryTimeout))) + g.Expect(result).To(HaveField("Code", Equal("SERVER_TIMEOUT"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify service unavailable as server", func(t *testing.T) { + g := NewWithT(t) + err := apierrors.NewServiceUnavailable("overloaded") + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryServer))) + g.Expect(result).To(HaveField("Code", Equal("SERVER_UNAVAILABLE"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify internal server error as server", func(t *testing.T) { + g := NewWithT(t) + err := apierrors.NewInternalError(errors.New("server crashed")) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryServer))) + g.Expect(result).To(HaveField("Code", Equal("SERVER_ERROR"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify already exists as conflict", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewAlreadyExists(gr, "my-deploy") + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryConflict))) + g.Expect(result).To(HaveField("Code", Equal("ALREADY_EXISTS"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify invalid as validation", func(t *testing.T) { + g := NewWithT(t) + gk := schema.GroupKind{Group: "apps", Kind: "Deployment"} + err := apierrors.NewInvalid(gk, "my-deploy", field.ErrorList{}) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("INVALID"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify bad request as validation", func(t *testing.T) { + g := NewWithT(t) + err := apierrors.NewBadRequest("malformed input") + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("BAD_REQUEST"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify method not supported as validation", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewMethodNotSupported(gr, "PATCH") + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("METHOD_NOT_SUPPORTED"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify not acceptable as validation", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewGenericServerResponse(http.StatusNotAcceptable, "GET", gr, "", "not acceptable", 0, false) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("NOT_ACCEPTABLE"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify unsupported media type as validation", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewGenericServerResponse(http.StatusUnsupportedMediaType, "POST", gr, "", "unsupported", 0, false) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("UNSUPPORTED_MEDIA_TYPE"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify request entity too large as validation", func(t *testing.T) { + g := NewWithT(t) + err := apierrors.NewRequestEntityTooLargeError("payload too big") + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("REQUEST_TOO_LARGE"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify gone as server", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewGenericServerResponse(http.StatusGone, "GET", gr, "", "resource expired", 0, false) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryServer))) + g.Expect(result).To(HaveField("Code", Equal("GONE"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify resource expired as server", func(t *testing.T) { + g := NewWithT(t) + err := apierrors.NewResourceExpired("watch expired") + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryServer))) + g.Expect(result).To(HaveField("Code", Equal("RESOURCE_EXPIRED"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify timeout error as timeout", func(t *testing.T) { + g := NewWithT(t) + err := apierrors.NewTimeoutError("gateway timeout", 5) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryTimeout))) + g.Expect(result).To(HaveField("Code", Equal("GATEWAY_TIMEOUT"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify too many requests as server", func(t *testing.T) { + g := NewWithT(t) + err := apierrors.NewTooManyRequests("rate limited", 30) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryServer))) + g.Expect(result).To(HaveField("Code", Equal("RATE_LIMITED"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify unexpected server error as server", func(t *testing.T) { + g := NewWithT(t) + gr := schema.GroupResource{Group: "apps", Resource: "deployments"} + err := apierrors.NewGenericServerResponse(http.StatusInternalServerError, "GET", gr, "", "unexpected", 0, true) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryServer))) + g.Expect(result).To(HaveField("Code", Equal("UNEXPECTED_SERVER_ERROR"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify unexpected object error as server", func(t *testing.T) { + g := NewWithT(t) + err := &apierrors.UnexpectedObjectError{Object: &metav1.Status{Status: metav1.StatusFailure}} + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryServer))) + g.Expect(result).To(HaveField("Code", Equal("UNEXPECTED_OBJECT"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify store read error as server", func(t *testing.T) { + g := NewWithT(t) + err := &apierrors.StatusError{ErrStatus: metav1.Status{ + Status: metav1.StatusFailure, + Reason: metav1.StatusReasonStoreReadError, + }} + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryServer))) + g.Expect(result).To(HaveField("Code", Equal("STORE_READ_ERROR"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify wrapped kubernetes errors", func(t *testing.T) { + g := NewWithT(t) + original := apierrors.NewUnauthorized("expired") + wrapped := fmt.Errorf("failed to create REST config: %w", original) + result := clierrors.Classify(wrapped) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryAuthentication))) + g.Expect(result).To(HaveField("Code", Equal("AUTH_FAILED"))) + }) + + t.Run("should classify network timeout", func(t *testing.T) { + g := NewWithT(t) + err := &testNetError{timeout: true} + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryTimeout))) + g.Expect(result).To(HaveField("Code", Equal("NET_TIMEOUT"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify network error as connection", func(t *testing.T) { + g := NewWithT(t) + err := &testNetError{timeout: false} + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryConnection))) + g.Expect(result).To(HaveField("Code", Equal("CONN_FAILED"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify deadline exceeded as timeout", func(t *testing.T) { + g := NewWithT(t) + result := clierrors.Classify(context.DeadlineExceeded) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryTimeout))) + g.Expect(result).To(HaveField("Code", Equal("TIMEOUT"))) + g.Expect(result).To(HaveField("Retriable", BeTrue())) + }) + + t.Run("should classify canceled as internal", func(t *testing.T) { + g := NewWithT(t) + result := clierrors.Classify(context.Canceled) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryInternal))) + g.Expect(result).To(HaveField("Code", Equal("CANCELED"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should classify filesystem path error as validation", func(t *testing.T) { + g := NewWithT(t) + err := &fs.PathError{Op: "stat", Path: "not/a/real/file", Err: fs.ErrNotExist} + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("CONFIG_INVALID"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + g.Expect(result).To(HaveField("Suggestion", Not(BeEmpty()))) + }) + + t.Run("should classify wrapped filesystem error as validation", func(t *testing.T) { + g := NewWithT(t) + pathErr := &fs.PathError{Op: "stat", Path: "/bad/kubeconfig", Err: fs.ErrNotExist} + wrapped := fmt.Errorf("failed to create REST config: %w", pathErr) + result := clierrors.Classify(wrapped) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("CONFIG_INVALID"))) + }) + + t.Run("should classify config error as validation", func(t *testing.T) { + g := NewWithT(t) + err := clierrors.NewConfigError(errors.New(`context "fake" does not exist`)) + result := clierrors.Classify(err) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("CONFIG_INVALID"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + g.Expect(result).To(HaveField("Suggestion", ContainSubstring("kubeconfig"))) + }) + + t.Run("should classify wrapped config error as validation", func(t *testing.T) { + g := NewWithT(t) + cfgErr := clierrors.NewConfigError(errors.New(`cluster "fake" does not exist`)) + wrapped := fmt.Errorf("failed to create REST config: %w", cfgErr) + result := clierrors.Classify(wrapped) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryValidation))) + g.Expect(result).To(HaveField("Code", Equal("CONFIG_INVALID"))) + }) + + t.Run("should classify unknown error as internal", func(t *testing.T) { + g := NewWithT(t) + result := clierrors.Classify(errors.New("something unexpected")) + + g.Expect(result).To(HaveField("Category", Equal(clierrors.CategoryInternal))) + g.Expect(result).To(HaveField("Code", Equal("INTERNAL"))) + g.Expect(result).To(HaveField("Retriable", BeFalse())) + }) + + t.Run("should return nil for nil error", func(t *testing.T) { + g := NewWithT(t) + result := clierrors.Classify(nil) + + g.Expect(result).ToNot(HaveOccurred()) + }) + + t.Run("should implement error interface", func(t *testing.T) { + g := NewWithT(t) + se := &clierrors.StructuredError{Category: clierrors.CategoryAuthentication, Message: "test"} + + g.Expect(se.Error()).To(Equal("[authentication] test")) + }) + + t.Run("should preserve error chain in NewAlreadyHandledError", func(t *testing.T) { + g := NewWithT(t) + original := errors.New("token expired") + wrapped := clierrors.NewAlreadyHandledError(original) + + g.Expect(wrapped).To(MatchError(ContainSubstring("token expired"))) + g.Expect(errors.Is(wrapped, clierrors.ErrAlreadyHandled)).To(BeTrue()) + g.Expect(errors.Is(wrapped, original)).To(BeTrue()) + }) +} diff --git a/pkg/util/errors/output.go b/pkg/util/errors/output.go new file mode 100644 index 00000000..088ff74d --- /dev/null +++ b/pkg/util/errors/output.go @@ -0,0 +1,94 @@ +package errors + +import ( + "encoding/json" + "errors" + "fmt" + "io" + + "sigs.k8s.io/yaml" +) + +// WriteSuggestion classifies an error and renders only the suggestion line. +func WriteSuggestion(w io.Writer, err error) { + if err == nil { + return + } + + var structErr *StructuredError + if !errors.As(err, &structErr) { + structErr = Classify(err) + } + + _, _ = fmt.Fprintf(w, "Suggestion: %s\n", structErr.Suggestion) +} + +// WriteTextError classifies an error and renders it as human-readable plain +// text with the suggestion included. Returns true if the error was written. +func WriteTextError(w io.Writer, err error) bool { + if err == nil { + return false + } + + var structErr *StructuredError + if !errors.As(err, &structErr) { + structErr = Classify(err) + } + + if _, writeErr := fmt.Fprintf(w, "%s\nSuggestion: %s\n", structErr.Message, structErr.Suggestion); writeErr != nil { + return false + } + + return true +} + +// WriteStructuredError renders a structured error as JSON or YAML to the +// provided writer. Returns true if the error was rendered, false if the +// format is not json/yaml (caller should fall back to plain text). +func WriteStructuredError(w io.Writer, err error, format string) bool { + if err == nil { + return false + } + + var structErr *StructuredError + if !errors.As(err, &structErr) { + structErr = Classify(err) + } + + envelope := errorEnvelope{Error: structErr} + + switch format { + case "json": + data, marshalErr := json.MarshalIndent(envelope, "", " ") + if marshalErr != nil { + if _, writeErr := fmt.Fprintf(w, "Error: %v\n", err); writeErr != nil { + return false + } + + return true + } + + if _, writeErr := fmt.Fprintln(w, string(data)); writeErr != nil { + return false + } + + return true + case "yaml": + data, marshalErr := yaml.Marshal(envelope) + if marshalErr != nil { + if _, writeErr := fmt.Fprintf(w, "Error: %v\n", err); writeErr != nil { + return false + } + + return true + } + + if _, writeErr := fmt.Fprint(w, string(data)); writeErr != nil { + return false + } + + return true + default: + return false + } +} diff --git a/pkg/util/errors/output_test.go b/pkg/util/errors/output_test.go new file mode 100644 index 00000000..4736c057 --- /dev/null +++ b/pkg/util/errors/output_test.go @@ -0,0 +1,127 @@ +package errors_test + +import ( + "bytes" + "errors" + "testing" + + clierrors "github.com/opendatahub-io/odh-cli/pkg/util/errors" + + . "github.com/onsi/gomega" +) + +func TestWriteTextError(t *testing.T) { + t.Run("should render message and suggestion", func(t *testing.T) { + g := NewWithT(t) + buf := &bytes.Buffer{} + err := &clierrors.StructuredError{ + Code: "AUTH_FAILED", + Message: "token expired", + Category: clierrors.CategoryAuthentication, + Suggestion: "Refresh credentials", + } + + handled := clierrors.WriteTextError(buf, err) + + g.Expect(handled).To(BeTrue()) + g.Expect(buf.String()).To(ContainSubstring("token expired")) + g.Expect(buf.String()).To(ContainSubstring("Suggestion: Refresh credentials")) + }) + + t.Run("should auto-classify raw errors", func(t *testing.T) { + g := NewWithT(t) + buf := &bytes.Buffer{} + rawErr := errors.New("something broke") + + handled := clierrors.WriteTextError(buf, rawErr) + + g.Expect(handled).To(BeTrue()) + g.Expect(buf.String()).To(ContainSubstring("something broke")) + g.Expect(buf.String()).To(ContainSubstring("Suggestion:")) + }) + + t.Run("should return false for nil error", func(t *testing.T) { + g := NewWithT(t) + buf := &bytes.Buffer{} + + handled := clierrors.WriteTextError(buf, nil) + + g.Expect(handled).To(BeFalse()) + }) +} + +func TestWriteStructuredError(t *testing.T) { + t.Run("should render JSON output", func(t *testing.T) { + g := NewWithT(t) + buf := &bytes.Buffer{} + err := &clierrors.StructuredError{ + Code: "AUTH_FAILED", + Message: "token expired", + Category: clierrors.CategoryAuthentication, + Retriable: false, + Suggestion: "Refresh credentials", + } + + handled := clierrors.WriteStructuredError(buf, err, "json") + + g.Expect(handled).To(BeTrue()) + g.Expect(buf.String()).To(ContainSubstring(`"code": "AUTH_FAILED"`)) + g.Expect(buf.String()).To(ContainSubstring(`"category": "authentication"`)) + g.Expect(buf.String()).To(ContainSubstring(`"retriable": false`)) + g.Expect(buf.String()).To(ContainSubstring(`"suggestion": "Refresh credentials"`)) + }) + + t.Run("should render YAML output", func(t *testing.T) { + g := NewWithT(t) + buf := &bytes.Buffer{} + err := &clierrors.StructuredError{ + Code: "AUTH_FAILED", + Message: "token expired", + Category: clierrors.CategoryAuthentication, + Retriable: false, + Suggestion: "Refresh credentials", + } + + handled := clierrors.WriteStructuredError(buf, err, "yaml") + + g.Expect(handled).To(BeTrue()) + g.Expect(buf.String()).To(ContainSubstring("code: AUTH_FAILED")) + g.Expect(buf.String()).To(ContainSubstring("category: authentication")) + }) + + t.Run("should not render table output", func(t *testing.T) { + g := NewWithT(t) + buf := &bytes.Buffer{} + err := &clierrors.StructuredError{ + Code: "AUTH_FAILED", + Message: "token expired", + Category: clierrors.CategoryAuthentication, + } + + handled := clierrors.WriteStructuredError(buf, err, "table") + + g.Expect(handled).To(BeFalse()) + g.Expect(buf.String()).To(BeEmpty()) + }) + + t.Run("should return false for nil error", func(t *testing.T) { + g := NewWithT(t) + buf := &bytes.Buffer{} + + handled := clierrors.WriteStructuredError(buf, nil, "json") + + g.Expect(handled).To(BeFalse()) + }) + + t.Run("should auto-classify raw errors", func(t *testing.T) { + g := NewWithT(t) + buf := &bytes.Buffer{} + rawErr := errors.New("something broke") + + handled := clierrors.WriteStructuredError(buf, rawErr, "json") + + g.Expect(handled).To(BeTrue()) + g.Expect(buf.String()).To(ContainSubstring(`"category": "internal"`)) + g.Expect(buf.String()).To(ContainSubstring(`"code": "INTERNAL"`)) + }) +} diff --git a/pkg/util/errors/types.go b/pkg/util/errors/types.go new file mode 100644 index 00000000..10a08c50 --- /dev/null +++ b/pkg/util/errors/types.go @@ -0,0 +1,75 @@ +package errors + +import ( + "errors" + "fmt" +) + +// ErrAlreadyHandled is a sentinel error indicating that the error has already +// been rendered to the output (e.g. as structured JSON/YAML) and should not +// be printed again by the caller. +var ErrAlreadyHandled = errors.New("error already rendered to output") + +// ErrorCategory represents the classification of a structured error. +type ErrorCategory string + +const ( + CategoryAuthentication ErrorCategory = "authentication" + CategoryAuthorization ErrorCategory = "authorization" + CategoryConnection ErrorCategory = "connection" + CategoryNotFound ErrorCategory = "not_found" + CategoryValidation ErrorCategory = "validation" + CategoryConflict ErrorCategory = "conflict" + CategoryServer ErrorCategory = "server" + CategoryTimeout ErrorCategory = "timeout" + CategoryInternal ErrorCategory = "internal" +) + +// StructuredError provides machine-readable error information for programmatic +// consumption by agents and automation tools. +type StructuredError struct { + Code string `json:"code" yaml:"code"` + Message string `json:"message" yaml:"message"` + Category ErrorCategory `json:"category" yaml:"category"` + Retriable bool `json:"retriable" yaml:"retriable"` + Suggestion string `json:"suggestion" yaml:"suggestion"` + + cause error +} + +// Error implements the error interface. +func (e *StructuredError) Error() string { + return fmt.Sprintf("[%s] %s", e.Category, e.Message) +} + +// Unwrap returns the underlying error, preserving the error chain +// for use with errors.Is and errors.As. +func (e *StructuredError) Unwrap() error { + return e.cause +} + +// NewAlreadyHandledError wraps the original error with ErrAlreadyHandled, +// preserving the full error chain for callers that inspect the cause. +func NewAlreadyHandledError(err error) error { + return fmt.Errorf("%w: %w", ErrAlreadyHandled, err) +} + +// ConfigError indicates a configuration problem such as an invalid kubeconfig, +// missing context, or unreachable cluster entry. Wrapping errors with this type +// allows Classify to distinguish user configuration mistakes from internal bugs. +type ConfigError struct { + cause error +} + +func (e *ConfigError) Error() string { return e.cause.Error() } +func (e *ConfigError) Unwrap() error { return e.cause } + +// NewConfigError wraps err as a ConfigError. +func NewConfigError(err error) *ConfigError { + return &ConfigError{cause: err} +} + +// errorEnvelope wraps a StructuredError for JSON/YAML output rendering. +type errorEnvelope struct { + Error *StructuredError `json:"error" yaml:"error"` +}