From ea6f8ae20bdf8530be843ad6737f09d62f479e3f Mon Sep 17 00:00:00 2001 From: vg006 Date: Fri, 23 Jan 2026 18:29:04 +0530 Subject: [PATCH 01/11] feat(pkg): Initialize package errors Signed-off-by: vg006 --- pkg/errors/errors.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 pkg/errors/errors.go diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go new file mode 100644 index 000000000..81b597a5d --- /dev/null +++ b/pkg/errors/errors.go @@ -0,0 +1,14 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package errors From a8165cd17a19f5977361c4bf8f7f3a5971c9a43e Mon Sep 17 00:00:00 2001 From: vg006 Date: Tue, 27 Jan 2026 22:06:40 +0530 Subject: [PATCH 02/11] feat(pkg): Add error type and methods Signed-off-by: vg006 --- pkg/errors/errors.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 81b597a5d..0a5732588 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -12,3 +12,40 @@ // See the License for the specific language governing permissions and // limitations under the License. package errors + +type Error struct { + cause error + message string + hint string +} + +func New(message string) *Error { + return &Error{ + message: message, + } +} + +func (e *Error) WithCause(cause error) *Error { + e.cause = cause + return e +} + +func (e *Error) WithHint(hint string) *Error { + e.hint = hint + return e +} + +func (e *Error) Error() string { + if e.cause != nil { + return e.message + ": " + e.cause.Error() + } + return e.message +} + +func (e *Error) Hint() string { + return e.hint +} + +func (e *Error) Unwrap() error { + return e.cause +} From 53c1577526d19ac70f558e9f19ca4067c69a62cf Mon Sep 17 00:00:00 2001 From: vg006 Date: Fri, 30 Jan 2026 08:28:08 +0530 Subject: [PATCH 03/11] refac(pkg): Update error parse with utils Signed-off-by: vg006 --- pkg/errors/errors.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 0a5732588..ca69c8cdf 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -13,6 +13,10 @@ // limitations under the License. package errors +import ( + "github.com/goharbor/harbor-cli/pkg/utils" +) + type Error struct { cause error message string @@ -37,11 +41,16 @@ func (e *Error) WithHint(hint string) *Error { func (e *Error) Error() string { if e.cause != nil { - return e.message + ": " + e.cause.Error() + causeMsg := utils.ParseHarborErrorMsg(e.cause) + return e.message + ": " + causeMsg } return e.message } +func (e *Error) Status() string { + return utils.ParseHarborErrorCode(e.cause) +} + func (e *Error) Hint() string { return e.hint } From 4fe4418217718d8ff51343797cb1cf04602760be Mon Sep 17 00:00:00 2001 From: vg006 Date: Fri, 30 Jan 2026 21:15:55 +0530 Subject: [PATCH 04/11] fix(pkg): Add utilities to fix cycle import Signed-off-by: vg006 --- pkg/errors/utils.go | 60 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 pkg/errors/utils.go diff --git a/pkg/errors/utils.go b/pkg/errors/utils.go new file mode 100644 index 000000000..134811aff --- /dev/null +++ b/pkg/errors/utils.go @@ -0,0 +1,60 @@ +package errors + +import ( + "encoding/json" + "fmt" + "reflect" + "strings" +) + +type harborErrorPayload struct { + Errors []struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"errors"` +} + +func isHarborError(err error) *Error { + var e *Error + if as(err, &e) { + return e + } + return nil +} + +func parseHarborErrorMsg(err error) string { + if err == nil { + return "" + } + + val := reflect.ValueOf(err) + if val.Kind() == reflect.Pointer { + val = val.Elem() + } + field := val.FieldByName("Payload") + if field.IsValid() { + payload := field.Interface() + jsonBytes, jsonErr := json.Marshal(payload) + if jsonErr == nil { + var harborErr harborErrorPayload + if unmarshalErr := json.Unmarshal(jsonBytes, &harborErr); unmarshalErr == nil { + if len(harborErr.Errors) > 0 { + return harborErr.Errors[0].Message + } + } + } + } + return fmt.Sprintf("%v", err.Error()) +} + +func parseHarborErrorCode(err error) string { + parts := strings.Split(err.Error(), "]") + if len(parts) >= 2 { + codePart := strings.TrimSpace(parts[1]) + if strings.HasPrefix(codePart, "[") && len(codePart) == 4 { + code := codePart[1:4] + return code + } + } + return "" +} From 74fb55bd9e1d27e44487e8c05d3a01a2ff6459db Mon Sep 17 00:00:00 2001 From: vg006 Date: Fri, 30 Jan 2026 21:16:26 +0530 Subject: [PATCH 05/11] feat(pkg): Add functions and methods to errors Signed-off-by: vg006 --- pkg/errors/errors.go | 75 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index ca69c8cdf..95d2f137c 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -14,13 +14,18 @@ package errors import ( - "github.com/goharbor/harbor-cli/pkg/utils" + "errors" + "fmt" +) + +var ( + as = errors.As ) type Error struct { cause error message string - hint string + hints []string } func New(message string) *Error { @@ -29,32 +34,74 @@ func New(message string) *Error { } } +func Newf(format string, args ...any) *Error { + return &Error{ + message: fmt.Sprintf(format, args...), + } +} + +func AsError(err error) *Error { + var e *Error + if errors.As(err, &e) { + return e + } + return &Error{ + message: err.Error(), + } +} + +func IsError(err error) bool { + var e *Error + return as(err, &e) +} + +func Cause(err error) error { + if e := isHarborError(err); e != nil { + return e.Cause() + } + return nil +} + +func Hints(err error) []string { + if e := isHarborError(err); e != nil { + return e.Hints() + } + return nil +} + +func Status(err error) string { + if e := isHarborError(err); e != nil { + return e.Status() + } + return "" +} + func (e *Error) WithCause(cause error) *Error { - e.cause = cause + if e.cause == nil { + e.cause = cause + } return e } func (e *Error) WithHint(hint string) *Error { - e.hint = hint + e.hints = append(e.hints, hint) return e } func (e *Error) Error() string { if e.cause != nil { - causeMsg := utils.ParseHarborErrorMsg(e.cause) + causeMsg := parseHarborErrorMsg(e.cause) return e.message + ": " + causeMsg } return e.message } -func (e *Error) Status() string { - return utils.ParseHarborErrorCode(e.cause) -} +func (e *Error) Cause() error { return e.cause } -func (e *Error) Hint() string { - return e.hint -} +func (e *Error) Status() string { return parseHarborErrorCode(e.cause) } -func (e *Error) Unwrap() error { - return e.cause -} +func (e *Error) Hints() []string { return e.hints } + +func (e *Error) Message() string { return e.message } + +func (e *Error) Unwrap() error { return e.cause } From a6151e9e691a27ab50fa60d6da13d4736630f2a5 Mon Sep 17 00:00:00 2001 From: vg006 Date: Wed, 18 Feb 2026 11:43:50 +0530 Subject: [PATCH 06/11] fix(pkg): Update error package and Lint Signed-off-by: vg006 --- pkg/errors/errors.go | 1 + pkg/errors/utils.go | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 95d2f137c..fac02d8f5 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -47,6 +47,7 @@ func AsError(err error) *Error { } return &Error{ message: err.Error(), + cause: err, } } diff --git a/pkg/errors/utils.go b/pkg/errors/utils.go index 134811aff..8b68f667a 100644 --- a/pkg/errors/utils.go +++ b/pkg/errors/utils.go @@ -1,3 +1,16 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. package errors import ( From d8978b1fd429236b93d5f8af6f63fcc48da108db Mon Sep 17 00:00:00 2001 From: vg006 Date: Thu, 19 Feb 2026 14:26:44 +0530 Subject: [PATCH 07/11] refac(pkg): Update the Error structure Signed-off-by: vg006 --- pkg/errors/errors.go | 117 +++++++++++++++++++++++++++++-------------- 1 file changed, 80 insertions(+), 37 deletions(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index fac02d8f5..c4b9b4a5c 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -22,32 +22,105 @@ var ( as = errors.As ) +type Frame struct { + Message string + Hints []string +} + type Error struct { - cause error - message string - hints []string + frames []Frame + cause error } func New(message string) *Error { return &Error{ - message: message, + frames: []Frame{{Message: message}}, } } func Newf(format string, args ...any) *Error { return &Error{ - message: fmt.Sprintf(format, args...), + frames: []Frame{{Message: fmt.Sprintf(format, args...)}}, + } +} + +func Wrap(err error, message string) *Error { + e := AsError(err) + e.frames = append([]Frame{{Message: message}}, e.frames...) + return e +} + +func Wrapf(err error, format string, args ...any) *Error { + return Wrap(err, fmt.Sprintf(format, args...)) +} + +func (e *Error) WithHint(hint string) *Error { + if len(e.frames) > 0 { + e.frames[0].Hints = append(e.frames[0].Hints, hint) + } + return e +} + +func (e *Error) WithCause(cause error) *Error { + if e.cause == nil { + e.cause = cause + } + return e +} + +func (e *Error) Error() string { + if len(e.frames) == 0 { + return "" + } + return e.frames[len(e.frames)-1].Message +} + +func (e *Error) Message() string { + if len(e.frames) == 0 { + return "" + } + return e.frames[0].Message +} + +func (e *Error) Errors() []string { + msgs := make([]string, len(e.frames)) + for i, f := range e.frames { + msgs[i] = f.Message + } + return msgs +} + +func (e *Error) Hints() []string { + var all []string + for _, f := range e.frames { + all = append(all, f.Hints...) } + return all } +func (e *Error) Frames() []Frame { + return e.frames +} + +func (e *Error) Cause() error { return e.cause } + +func (e *Error) Status() string { + if e.cause == nil { + return "" + } + return parseHarborErrorCode(e.cause) +} + +func (e *Error) Unwrap() error { return e.cause } + func AsError(err error) *Error { var e *Error if errors.As(err, &e) { return e } return &Error{ - message: err.Error(), - cause: err, + frames: []Frame{{Message: parseHarborErrorMsg(err)}}, + cause: err, } } @@ -76,33 +149,3 @@ func Status(err error) string { } return "" } - -func (e *Error) WithCause(cause error) *Error { - if e.cause == nil { - e.cause = cause - } - return e -} - -func (e *Error) WithHint(hint string) *Error { - e.hints = append(e.hints, hint) - return e -} - -func (e *Error) Error() string { - if e.cause != nil { - causeMsg := parseHarborErrorMsg(e.cause) - return e.message + ": " + causeMsg - } - return e.message -} - -func (e *Error) Cause() error { return e.cause } - -func (e *Error) Status() string { return parseHarborErrorCode(e.cause) } - -func (e *Error) Hints() []string { return e.hints } - -func (e *Error) Message() string { return e.message } - -func (e *Error) Unwrap() error { return e.cause } From f03af0e3efa683d5c8dcacf6c7bb8d9b08b9865f Mon Sep 17 00:00:00 2001 From: vg006 Date: Thu, 19 Feb 2026 14:34:06 +0530 Subject: [PATCH 08/11] feat(pkg): Add tests to the errors package Signed-off-by: vg006 --- pkg/errors/errors_test.go | 268 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 268 insertions(+) create mode 100644 pkg/errors/errors_test.go diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go new file mode 100644 index 000000000..1844207ee --- /dev/null +++ b/pkg/errors/errors_test.go @@ -0,0 +1,268 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package errors_test + +import ( + "encoding/json" + "errors" + "fmt" + "testing" + + harborerr "github.com/goharbor/harbor-cli/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNew_SingleFrame(t *testing.T) { + err := harborerr.New("something went wrong") + require.NotNil(t, err) + assert.Equal(t, "something went wrong", err.Error()) + assert.Equal(t, "something went wrong", err.Message()) + assert.Equal(t, []string{"something went wrong"}, err.Errors()) + assert.Empty(t, err.Hints()) + assert.Nil(t, err.Cause()) +} + +func TestNewf_FormatsMessage(t *testing.T) { + err := harborerr.Newf("resource %q not found", "project-x") + require.NotNil(t, err) + assert.Equal(t, `resource "project-x" not found`, err.Error()) + assert.Equal(t, `resource "project-x" not found`, err.Message()) +} + +func TestWrap_PushesOutermostFrame(t *testing.T) { + root := harborerr.New("root problem") + wrapped := harborerr.Wrap(root, "operation failed") + + assert.Equal(t, "root problem", wrapped.Error()) + assert.Equal(t, "operation failed", wrapped.Message()) + assert.Equal(t, []string{"operation failed", "root problem"}, wrapped.Errors()) +} + +func TestWrapf_FormatsOutermostFrame(t *testing.T) { + root := harborerr.New("auth failed") + wrapped := harborerr.Wrapf(root, "login for user %q", "alice") + + assert.Equal(t, "auth failed", wrapped.Error()) + assert.Equal(t, `login for user "alice"`, wrapped.Message()) + assert.Equal(t, []string{`login for user "alice"`, "auth failed"}, wrapped.Errors()) +} + +func TestWrap_ThreeLevels(t *testing.T) { + root := harborerr.New("db timeout") + mid := harborerr.Wrap(root, "repository unavailable") + top := harborerr.Wrap(mid, "delete artifact failed") + + assert.Equal(t, "db timeout", top.Error()) + assert.Equal(t, "delete artifact failed", top.Message()) + assert.Equal(t, []string{"delete artifact failed", "repository unavailable", "db timeout"}, top.Errors()) +} + +func TestWrap_PlainStdlibError(t *testing.T) { + plain := errors.New("network error") + wrapped := harborerr.Wrap(plain, "could not reach registry") + + assert.Equal(t, "network error", wrapped.Error()) + assert.Equal(t, "could not reach registry", wrapped.Message()) + assert.Equal(t, []string{"could not reach registry", "network error"}, wrapped.Errors()) +} + +func TestErrors_SingleFrame(t *testing.T) { + err := harborerr.New("standalone") + assert.Equal(t, []string{"standalone"}, err.Errors()) +} + +func TestErrors_OutermostFirst(t *testing.T) { + err := harborerr.Wrap(harborerr.Wrap(harborerr.New("level-0"), "level-1"), "level-2") + assert.Equal(t, []string{"level-2", "level-1", "level-0"}, err.Errors()) +} + +func TestWithHint_AttachesToOutermostFrame(t *testing.T) { + err := harborerr.New("error").WithHint("try again later") + assert.Equal(t, []string{"try again later"}, err.Hints()) +} + +func TestWithHint_MultipleHintsOnSameFrame(t *testing.T) { + err := harborerr.New("error"). + WithHint("hint one"). + WithHint("hint two"). + WithHint("hint three") + assert.Equal(t, []string{"hint one", "hint two", "hint three"}, err.Hints()) +} + +func TestHints_AcrossFrames_OutermostFirst(t *testing.T) { + root := harborerr.New("root").WithHint("root-hint") + top := harborerr.Wrap(root, "top").WithHint("top-hint") + + assert.Equal(t, []string{"top-hint", "root-hint"}, top.Hints()) +} + +func TestHints_PackageLevel_HarborError(t *testing.T) { + err := harborerr.New("error").WithHint("check config") + assert.Equal(t, []string{"check config"}, harborerr.Hints(err)) +} + +func TestHints_PackageLevel_PlainError(t *testing.T) { + assert.Nil(t, harborerr.Hints(errors.New("plain"))) +} + +func TestIsError_True_DirectHarborError(t *testing.T) { + assert.True(t, harborerr.IsError(harborerr.New("err"))) +} + +func TestIsError_True_WrappedWithFmtErrorf(t *testing.T) { + wrapped := fmt.Errorf("outer: %w", harborerr.New("inner")) + assert.True(t, harborerr.IsError(wrapped)) +} + +func TestIsError_False_PlainError(t *testing.T) { + assert.False(t, harborerr.IsError(errors.New("plain"))) +} + +func TestAsError_FromHarborError_ReturnsSame(t *testing.T) { + original := harborerr.New("original") + wrapped := fmt.Errorf("wrapped: %w", original) + + result := harborerr.AsError(wrapped) + require.NotNil(t, result) + assert.Equal(t, "original", result.Error()) +} + +func TestAsError_FromPlainError_WrapsIntoSingleFrame(t *testing.T) { + plain := errors.New("plain error") + result := harborerr.AsError(plain) + require.NotNil(t, result) + assert.Equal(t, "plain error", result.Error()) + assert.Equal(t, []string{"plain error"}, result.Errors()) + assert.Equal(t, plain, result.Cause()) +} + +func TestWithCause_AttachesCauseForUnwrap(t *testing.T) { + sentinel := errors.New("sentinel") + err := harborerr.New("wrapper").WithCause(sentinel) + + assert.Equal(t, sentinel, err.Cause()) + assert.True(t, errors.Is(err, sentinel)) +} + +func TestWithCause_OnlyFirstCauseIsStored(t *testing.T) { + first := errors.New("first") + second := errors.New("second") + err := harborerr.New("e").WithCause(first).WithCause(second) + assert.Equal(t, first, err.Cause()) +} + +func TestCause_PackageLevel_HarborError(t *testing.T) { + sentinel := errors.New("root") + err := harborerr.New("top").WithCause(sentinel) + assert.Equal(t, sentinel, harborerr.Cause(err)) +} + +func TestCause_PackageLevel_PlainError(t *testing.T) { + assert.Nil(t, harborerr.Cause(errors.New("plain"))) +} + +func TestUnwrap_ErrorsAs_FindsOuterFrame(t *testing.T) { + inner := harborerr.New("inner") + outer := harborerr.New("outer").WithCause(inner) + + var target *harborerr.Error + assert.True(t, errors.As(outer, &target)) + assert.Equal(t, "outer", target.Error()) +} + +func TestStatus_PlainError_ReturnsEmpty(t *testing.T) { + assert.Equal(t, "", harborerr.Status(errors.New("plain"))) +} + +func TestStatus_NoCause_ReturnsEmpty(t *testing.T) { + assert.Equal(t, "", harborerr.Status(harborerr.New("no cause"))) +} + +type harborPayloadError struct { + Payload *harborPayloadBody +} + +type harborPayloadBody struct { + Errors []harborPayloadEntry `json:"errors"` +} + +type harborPayloadEntry struct { + Code string `json:"code"` + Message string `json:"message"` +} + +func (h *harborPayloadError) Error() string { + if h.Payload != nil && len(h.Payload.Errors) > 0 { + b, _ := json.Marshal(h.Payload) + return fmt.Sprintf("[%s] %s", h.Payload.Errors[0].Code, string(b)) + } + return "harbor error" +} + +func TestWrap_HarborPayloadCause_ExtractsMessage(t *testing.T) { + apiErr := &harborPayloadError{ + Payload: &harborPayloadBody{ + Errors: []harborPayloadEntry{ + {Code: "NOT_FOUND", Message: "repository does not exist"}, + }, + }, + } + err := harborerr.Wrap(apiErr, "delete artifact failed") + + assert.Equal(t, "repository does not exist", err.Error()) + assert.Equal(t, "delete artifact failed", err.Message()) + assert.Equal(t, []string{"delete artifact failed", "repository does not exist"}, err.Errors()) +} + +func TestFrames_SingleFrame(t *testing.T) { + err := harborerr.New("root").WithHint("hint-a") + frames := err.Frames() + require.Len(t, frames, 1) + assert.Equal(t, "root", frames[0].Message) + assert.Equal(t, []string{"hint-a"}, frames[0].Hints) +} + +func TestFrames_MultipleFrames_OutermostFirst(t *testing.T) { + root := harborerr.New("root").WithHint("root-hint") + mid := harborerr.Wrap(root, "mid").WithHint("mid-hint") + top := harborerr.Wrap(mid, "top").WithHint("top-hint") + + frames := top.Frames() + require.Len(t, frames, 3) + assert.Equal(t, "top", frames[0].Message) + assert.Equal(t, []string{"top-hint"}, frames[0].Hints) + assert.Equal(t, "mid", frames[1].Message) + assert.Equal(t, []string{"mid-hint"}, frames[1].Hints) + assert.Equal(t, "root", frames[2].Message) + assert.Equal(t, []string{"root-hint"}, frames[2].Hints) +} + +func TestFrames_NoHints(t *testing.T) { + err := harborerr.Wrap(harborerr.New("root"), "top") + frames := err.Frames() + require.Len(t, frames, 2) + assert.Empty(t, frames[0].Hints) + assert.Empty(t, frames[1].Hints) +} + +func TestChaining_WrapWithHints(t *testing.T) { + root := harborerr.New("connection refused").WithHint("check firewall rules") + top := harborerr.Wrap(root, "could not reach registry").WithHint("verify server URL") + + assert.Equal(t, "connection refused", top.Error()) + assert.Equal(t, "could not reach registry", top.Message()) + assert.Equal(t, []string{"could not reach registry", "connection refused"}, top.Errors()) + assert.Equal(t, []string{"verify server URL", "check firewall rules"}, top.Hints()) +} From 7c9788ecb23ac89b9b117e4c552dc7766492c661 Mon Sep 17 00:00:00 2001 From: vg006 Date: Tue, 17 Mar 2026 19:37:46 +0530 Subject: [PATCH 09/11] refac(pkg): update the methods, tests and styles Signed-off-by: vg006 --- pkg/errors/errors.go | 106 +++++++++--- pkg/errors/errors_test.go | 343 ++++++++++++++++++++------------------ pkg/views/styles.go | 30 +++- 3 files changed, 287 insertions(+), 192 deletions(-) diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index c4b9b4a5c..a35b671b9 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -16,6 +16,10 @@ package errors import ( "errors" "fmt" + "strings" + + "github.com/charmbracelet/lipgloss/tree" + "github.com/goharbor/harbor-cli/pkg/views" ) var ( @@ -32,9 +36,9 @@ type Error struct { cause error } -func New(message string) *Error { +func New(message string, hints ...string) *Error { return &Error{ - frames: []Frame{{Message: message}}, + frames: []Frame{{Message: message, Hints: hints}}, } } @@ -44,27 +48,15 @@ func Newf(format string, args ...any) *Error { } } -func Wrap(err error, message string) *Error { - e := AsError(err) - e.frames = append([]Frame{{Message: message}}, e.frames...) - return e -} - -func Wrapf(err error, format string, args ...any) *Error { - return Wrap(err, fmt.Sprintf(format, args...)) -} - -func (e *Error) WithHint(hint string) *Error { - if len(e.frames) > 0 { - e.frames[0].Hints = append(e.frames[0].Hints, hint) +func NewWithCause(cause error, message string, hints ...string) *Error { + return &Error{ + cause: cause, + frames: []Frame{{Message: message, Hints: hints}}, } - return e } -func (e *Error) WithCause(cause error) *Error { - if e.cause == nil { - e.cause = cause - } +func (e *Error) WithMessage(message string, hints ...string) *Error { + e.frames = append(e.frames, Frame{Message: message, Hints: hints}) return e } @@ -72,7 +64,78 @@ func (e *Error) Error() string { if len(e.frames) == 0 { return "" } - return e.frames[len(e.frames)-1].Message + + var parts []string + + rootFrame := e.frames[0] + parts = append(parts, views.ErrCauseStyle.Render(rootFrame.Message)) + + if e.cause != nil { + if code := parseHarborErrorCode(e.cause); code != "" { + parts = append(parts, + views.ErrTitleStyle.Render("Code: ")+views.ErrCauseStyle.Render(code), + ) + } + } + + if e.cause != nil { + causeText := e.cause.Error() + if he := isHarborError(e.cause); he != nil { + causeText = he.Message() + } + + cause := views.ErrTitleStyle.Render("Cause: ") + causeText = views.ErrCauseStyle.Render(causeText) + causeTree := tree.Root(cause + causeText). + Enumerator(tree.RoundedEnumerator). + EnumeratorStyle(views.ErrEnumeratorStyle). + ItemStyle(views.ErrHintStyle) + + if he := isHarborError(e.cause); he != nil { + for _, h := range he.Hints() { + causeTree.Child(h) + } + } + parts = append(parts, causeTree.String()) + } + + if len(rootFrame.Hints) > 0 { + hintsTree := tree.New(). + Root("Hints:"). + RootStyle(views.ErrTitleStyle). + Enumerator(tree.RoundedEnumerator). + EnumeratorStyle(views.ErrEnumeratorStyle). + ItemStyle(views.ErrHintStyle) + + for _, h := range rootFrame.Hints { + hintsTree.Child(h) + } + parts = append(parts, hintsTree.String()) + } + + if len(e.frames) > 1 { + msgTree := tree.New(). + Root("Messages:"). + RootStyle(views.ErrTitleStyle). + Enumerator(tree.RoundedEnumerator). + EnumeratorStyle(views.ErrEnumeratorStyle). + ItemStyle(views.ErrTitleStyle) + + for _, f := range e.frames[1:] { + msgWithHints := tree.Root(f.Message). + RootStyle(views.ErrTitleStyle). + Enumerator(tree.RoundedEnumerator). + EnumeratorStyle(views.ErrEnumeratorStyle). + ItemStyle(views.ErrHintStyle) + for _, h := range f.Hints { + msgWithHints.Child(h) + } + msgTree.Child(msgWithHints) + } + parts = append(parts, msgTree.String()) + } + + return strings.Join(parts, "\n") } func (e *Error) Message() string { @@ -112,7 +175,6 @@ func (e *Error) Status() string { } func (e *Error) Unwrap() error { return e.cause } - func AsError(err error) *Error { var e *Error if errors.As(err, &e) { diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go index 1844207ee..403558d4d 100644 --- a/pkg/errors/errors_test.go +++ b/pkg/errors/errors_test.go @@ -14,7 +14,6 @@ package errors_test import ( - "encoding/json" "errors" "fmt" "testing" @@ -24,245 +23,267 @@ import ( "github.com/stretchr/testify/require" ) -func TestNew_SingleFrame(t *testing.T) { +func TestNew_MessageOnly(t *testing.T) { err := harborerr.New("something went wrong") require.NotNil(t, err) - assert.Equal(t, "something went wrong", err.Error()) + + output := err.Error() + assert.Contains(t, output, "something went wrong") assert.Equal(t, "something went wrong", err.Message()) assert.Equal(t, []string{"something went wrong"}, err.Errors()) assert.Empty(t, err.Hints()) assert.Nil(t, err.Cause()) } +func TestNew_MessageWithHints(t *testing.T) { + err := harborerr.New("auth failed", "check your credentials", "ensure token is not expired") + require.NotNil(t, err) + + output := err.Error() + assert.Contains(t, output, "auth failed") + assert.Contains(t, output, "check your credentials") + assert.Contains(t, output, "ensure token is not expired") + + assert.Equal(t, "auth failed", err.Message()) + assert.Equal(t, []string{"check your credentials", "ensure token is not expired"}, err.Hints()) + assert.Nil(t, err.Cause()) +} + func TestNewf_FormatsMessage(t *testing.T) { err := harborerr.Newf("resource %q not found", "project-x") require.NotNil(t, err) - assert.Equal(t, `resource "project-x" not found`, err.Error()) + + output := err.Error() + assert.Contains(t, output, `resource "project-x" not found`) assert.Equal(t, `resource "project-x" not found`, err.Message()) + assert.Empty(t, err.Hints()) } -func TestWrap_PushesOutermostFrame(t *testing.T) { - root := harborerr.New("root problem") - wrapped := harborerr.Wrap(root, "operation failed") - - assert.Equal(t, "root problem", wrapped.Error()) - assert.Equal(t, "operation failed", wrapped.Message()) - assert.Equal(t, []string{"operation failed", "root problem"}, wrapped.Errors()) -} +func TestNewWithCause_Basic(t *testing.T) { + cause := errors.New("connection refused") + err := harborerr.NewWithCause(cause, "could not reach registry") -func TestWrapf_FormatsOutermostFrame(t *testing.T) { - root := harborerr.New("auth failed") - wrapped := harborerr.Wrapf(root, "login for user %q", "alice") + output := err.Error() + assert.Contains(t, output, "could not reach registry") + assert.Contains(t, output, "Cause:") + assert.Contains(t, output, "connection refused") + assert.NotContains(t, output, "Messages:") - assert.Equal(t, "auth failed", wrapped.Error()) - assert.Equal(t, `login for user "alice"`, wrapped.Message()) - assert.Equal(t, []string{`login for user "alice"`, "auth failed"}, wrapped.Errors()) + assert.Equal(t, cause, err.Cause()) + assert.Equal(t, "could not reach registry", err.Message()) } -func TestWrap_ThreeLevels(t *testing.T) { - root := harborerr.New("db timeout") - mid := harborerr.Wrap(root, "repository unavailable") - top := harborerr.Wrap(mid, "delete artifact failed") +func TestNewWithCause_WithHints(t *testing.T) { + cause := errors.New("401 Unauthorized") + err := harborerr.NewWithCause(cause, "authentication failed", + "ensure your credentials are correct", + "run `harbor login` to re-authenticate", + ) + + output := err.Error() + assert.Contains(t, output, "authentication failed") + assert.Contains(t, output, "ensure your credentials are correct") + assert.Contains(t, output, "run `harbor login` to re-authenticate") + assert.Contains(t, output, "Cause:") + assert.Contains(t, output, "401 Unauthorized") + assert.NotContains(t, output, "Messages:") - assert.Equal(t, "db timeout", top.Error()) - assert.Equal(t, "delete artifact failed", top.Message()) - assert.Equal(t, []string{"delete artifact failed", "repository unavailable", "db timeout"}, top.Errors()) + assert.Equal(t, cause, err.Cause()) + assert.Equal(t, []string{ + "ensure your credentials are correct", + "run `harbor login` to re-authenticate", + }, err.Hints()) } -func TestWrap_PlainStdlibError(t *testing.T) { - plain := errors.New("network error") - wrapped := harborerr.Wrap(plain, "could not reach registry") +func TestNewWithCause_CauseIsHarborError_ShowsCauseHintsInHeader(t *testing.T) { + inner := harborerr.New("db connection lost", "check that the database is running") + outer := harborerr.NewWithCause(inner, "repository unavailable") - assert.Equal(t, "network error", wrapped.Error()) - assert.Equal(t, "could not reach registry", wrapped.Message()) - assert.Equal(t, []string{"could not reach registry", "network error"}, wrapped.Errors()) + output := outer.Error() + assert.Contains(t, output, "repository unavailable") + assert.Contains(t, output, "check that the database is running") + assert.NotContains(t, output, "Messages:") } -func TestErrors_SingleFrame(t *testing.T) { - err := harborerr.New("standalone") - assert.Equal(t, []string{"standalone"}, err.Errors()) -} +func TestWithMessage_AppendsFrame(t *testing.T) { + err := harborerr.NewWithCause( + errors.New("dial tcp 127.0.0.1:5432: connect: connection refused"), + "repository unavailable", "check that the database is running", + ).WithMessage("failed to delete artifact", + "retry after resolving the underlying issue", + "use --force to skip confirmation prompts", + ) -func TestErrors_OutermostFirst(t *testing.T) { - err := harborerr.Wrap(harborerr.Wrap(harborerr.New("level-0"), "level-1"), "level-2") - assert.Equal(t, []string{"level-2", "level-1", "level-0"}, err.Errors()) -} + output := err.Error() + assert.Contains(t, output, "repository unavailable") + assert.Contains(t, output, "check that the database is running") + assert.Contains(t, output, "Cause:") + assert.Contains(t, output, "connection refused") + assert.Contains(t, output, "Messages:") + assert.Contains(t, output, "failed to delete artifact") + assert.Contains(t, output, "retry after resolving the underlying issue") + assert.Contains(t, output, "use --force to skip confirmation prompts") -func TestWithHint_AttachesToOutermostFrame(t *testing.T) { - err := harborerr.New("error").WithHint("try again later") - assert.Equal(t, []string{"try again later"}, err.Hints()) + assert.Equal(t, "repository unavailable", err.Message()) + assert.Len(t, err.Frames(), 2) } -func TestWithHint_MultipleHintsOnSameFrame(t *testing.T) { - err := harborerr.New("error"). - WithHint("hint one"). - WithHint("hint two"). - WithHint("hint three") - assert.Equal(t, []string{"hint one", "hint two", "hint three"}, err.Hints()) +func TestWithMessage_MultipleFrames(t *testing.T) { + err := harborerr.New("step 1"). + WithMessage("step 2", "hint A"). + WithMessage("step 3", "hint B", "hint C") + + assert.Len(t, err.Frames(), 3) + assert.Equal(t, "step 1", err.Frames()[0].Message) + assert.Equal(t, "step 2", err.Frames()[1].Message) + assert.Equal(t, []string{"hint A"}, err.Frames()[1].Hints) + assert.Equal(t, "step 3", err.Frames()[2].Message) + assert.Equal(t, []string{"hint B", "hint C"}, err.Frames()[2].Hints) } -func TestHints_AcrossFrames_OutermostFirst(t *testing.T) { - root := harborerr.New("root").WithHint("root-hint") - top := harborerr.Wrap(root, "top").WithHint("top-hint") +func TestWithMessage_NoCause_NoRootHeader(t *testing.T) { + err := harborerr.New("first").WithMessage("second", "a hint") - assert.Equal(t, []string{"top-hint", "root-hint"}, top.Hints()) + output := err.Error() + assert.NotContains(t, output, "Root:") + assert.NotContains(t, output, "Code:") + assert.Contains(t, output, "first") + assert.Contains(t, output, "second") + assert.Contains(t, output, "a hint") } -func TestHints_PackageLevel_HarborError(t *testing.T) { - err := harborerr.New("error").WithHint("check config") - assert.Equal(t, []string{"check config"}, harborerr.Hints(err)) +func TestError_EmptyFrames(t *testing.T) { + err := &harborerr.Error{} + assert.Equal(t, "", err.Error()) } -func TestHints_PackageLevel_PlainError(t *testing.T) { - assert.Nil(t, harborerr.Hints(errors.New("plain"))) -} +func TestError_NoCause_SingleFrameNoHints(t *testing.T) { + err := harborerr.New("operation not supported") + output := err.Error() -func TestIsError_True_DirectHarborError(t *testing.T) { - assert.True(t, harborerr.IsError(harborerr.New("err"))) + assert.NotContains(t, output, "Root:") + assert.NotContains(t, output, "Code:") + assert.Contains(t, output, "operation not supported") } -func TestIsError_True_WrappedWithFmtErrorf(t *testing.T) { - wrapped := fmt.Errorf("outer: %w", harborerr.New("inner")) - assert.True(t, harborerr.IsError(wrapped)) +func TestError_NoCause_SingleFrameWithHints(t *testing.T) { + err := harborerr.New("plugin system not implemented", + "this command is a placeholder for future plugin management", + ) + + output := err.Error() + assert.NotContains(t, output, "Root:") + assert.Contains(t, output, "plugin system not implemented") + assert.Contains(t, output, "this command is a placeholder for future plugin management") } -func TestIsError_False_PlainError(t *testing.T) { - assert.False(t, harborerr.IsError(errors.New("plain"))) +func TestMessage_ReturnsFirstFrame(t *testing.T) { + err := harborerr.New("first").WithMessage("second") + assert.Equal(t, "first", err.Message()) } -func TestAsError_FromHarborError_ReturnsSame(t *testing.T) { - original := harborerr.New("original") - wrapped := fmt.Errorf("wrapped: %w", original) +func TestErrors_AllMessages(t *testing.T) { + err := harborerr.New("a").WithMessage("b").WithMessage("c") + assert.Equal(t, []string{"a", "b", "c"}, err.Errors()) +} - result := harborerr.AsError(wrapped) - require.NotNil(t, result) - assert.Equal(t, "original", result.Error()) +func TestHints_AcrossAllFrames(t *testing.T) { + err := harborerr.New("m1", "h1", "h2").WithMessage("m2", "h3") + assert.Equal(t, []string{"h1", "h2", "h3"}, err.Hints()) } -func TestAsError_FromPlainError_WrapsIntoSingleFrame(t *testing.T) { - plain := errors.New("plain error") - result := harborerr.AsError(plain) - require.NotNil(t, result) - assert.Equal(t, "plain error", result.Error()) - assert.Equal(t, []string{"plain error"}, result.Errors()) - assert.Equal(t, plain, result.Cause()) +func TestFrames_ReturnsAll(t *testing.T) { + err := harborerr.New("root", "hint-a") + frames := err.Frames() + require.Len(t, frames, 1) + assert.Equal(t, "root", frames[0].Message) + assert.Equal(t, []string{"hint-a"}, frames[0].Hints) } -func TestWithCause_AttachesCauseForUnwrap(t *testing.T) { +func TestCause_ReturnsUnderlyingError(t *testing.T) { sentinel := errors.New("sentinel") - err := harborerr.New("wrapper").WithCause(sentinel) - + err := harborerr.NewWithCause(sentinel, "wrapper") assert.Equal(t, sentinel, err.Cause()) - assert.True(t, errors.Is(err, sentinel)) } -func TestWithCause_OnlyFirstCauseIsStored(t *testing.T) { - first := errors.New("first") - second := errors.New("second") - err := harborerr.New("e").WithCause(first).WithCause(second) - assert.Equal(t, first, err.Cause()) +func TestCause_NilWhenNoCause(t *testing.T) { + err := harborerr.New("standalone") + assert.Nil(t, err.Cause()) } -func TestCause_PackageLevel_HarborError(t *testing.T) { - sentinel := errors.New("root") - err := harborerr.New("top").WithCause(sentinel) - assert.Equal(t, sentinel, harborerr.Cause(err)) +func TestStatus_NoCause_ReturnsEmpty(t *testing.T) { + assert.Equal(t, "", harborerr.Status(harborerr.New("no cause"))) } -func TestCause_PackageLevel_PlainError(t *testing.T) { - assert.Nil(t, harborerr.Cause(errors.New("plain"))) +func TestStatus_PlainError_ReturnsEmpty(t *testing.T) { + assert.Equal(t, "", harborerr.Status(errors.New("plain"))) } -func TestUnwrap_ErrorsAs_FindsOuterFrame(t *testing.T) { +func TestUnwrap_ReturnsTheCause(t *testing.T) { + sentinel := errors.New("sentinel") + err := harborerr.NewWithCause(sentinel, "wrapper") + + assert.Equal(t, sentinel, err.Unwrap()) + assert.True(t, errors.Is(err, sentinel)) +} + +func TestErrorsAs_FindsHarborError(t *testing.T) { inner := harborerr.New("inner") - outer := harborerr.New("outer").WithCause(inner) + outer := harborerr.NewWithCause(inner, "outer") var target *harborerr.Error assert.True(t, errors.As(outer, &target)) - assert.Equal(t, "outer", target.Error()) + assert.Contains(t, target.Error(), "inner") + assert.Contains(t, target.Error(), "outer") } -func TestStatus_PlainError_ReturnsEmpty(t *testing.T) { - assert.Equal(t, "", harborerr.Status(errors.New("plain"))) +func TestIsError_True_DirectHarborError(t *testing.T) { + assert.True(t, harborerr.IsError(harborerr.New("err"))) } -func TestStatus_NoCause_ReturnsEmpty(t *testing.T) { - assert.Equal(t, "", harborerr.Status(harborerr.New("no cause"))) +func TestIsError_True_WrappedWithFmtErrorf(t *testing.T) { + wrapped := fmt.Errorf("outer: %w", harborerr.New("inner")) + assert.True(t, harborerr.IsError(wrapped)) } -type harborPayloadError struct { - Payload *harborPayloadBody +func TestIsError_False_PlainError(t *testing.T) { + assert.False(t, harborerr.IsError(errors.New("plain"))) } -type harborPayloadBody struct { - Errors []harborPayloadEntry `json:"errors"` -} +func TestAsError_FromHarborError_ReturnsSame(t *testing.T) { + original := harborerr.New("original") + wrapped := fmt.Errorf("wrapped: %w", original) -type harborPayloadEntry struct { - Code string `json:"code"` - Message string `json:"message"` + result := harborerr.AsError(wrapped) + require.NotNil(t, result) + assert.Contains(t, result.Error(), "original") } -func (h *harborPayloadError) Error() string { - if h.Payload != nil && len(h.Payload.Errors) > 0 { - b, _ := json.Marshal(h.Payload) - return fmt.Sprintf("[%s] %s", h.Payload.Errors[0].Code, string(b)) - } - return "harbor error" -} +func TestAsError_FromPlainError_WrapsIntoSingleFrame(t *testing.T) { + plain := errors.New("plain error") + result := harborerr.AsError(plain) + require.NotNil(t, result) -func TestWrap_HarborPayloadCause_ExtractsMessage(t *testing.T) { - apiErr := &harborPayloadError{ - Payload: &harborPayloadBody{ - Errors: []harborPayloadEntry{ - {Code: "NOT_FOUND", Message: "repository does not exist"}, - }, - }, - } - err := harborerr.Wrap(apiErr, "delete artifact failed") - - assert.Equal(t, "repository does not exist", err.Error()) - assert.Equal(t, "delete artifact failed", err.Message()) - assert.Equal(t, []string{"delete artifact failed", "repository does not exist"}, err.Errors()) + assert.Contains(t, result.Error(), "plain error") + assert.Equal(t, []string{"plain error"}, result.Errors()) + assert.Equal(t, plain, result.Cause()) } -func TestFrames_SingleFrame(t *testing.T) { - err := harborerr.New("root").WithHint("hint-a") - frames := err.Frames() - require.Len(t, frames, 1) - assert.Equal(t, "root", frames[0].Message) - assert.Equal(t, []string{"hint-a"}, frames[0].Hints) +func TestCause_PackageLevel_HarborError(t *testing.T) { + sentinel := errors.New("root") + err := harborerr.NewWithCause(sentinel, "top") + assert.Equal(t, sentinel, harborerr.Cause(err)) } -func TestFrames_MultipleFrames_OutermostFirst(t *testing.T) { - root := harborerr.New("root").WithHint("root-hint") - mid := harborerr.Wrap(root, "mid").WithHint("mid-hint") - top := harborerr.Wrap(mid, "top").WithHint("top-hint") - - frames := top.Frames() - require.Len(t, frames, 3) - assert.Equal(t, "top", frames[0].Message) - assert.Equal(t, []string{"top-hint"}, frames[0].Hints) - assert.Equal(t, "mid", frames[1].Message) - assert.Equal(t, []string{"mid-hint"}, frames[1].Hints) - assert.Equal(t, "root", frames[2].Message) - assert.Equal(t, []string{"root-hint"}, frames[2].Hints) +func TestCause_PackageLevel_PlainError(t *testing.T) { + assert.Nil(t, harborerr.Cause(errors.New("plain"))) } -func TestFrames_NoHints(t *testing.T) { - err := harborerr.Wrap(harborerr.New("root"), "top") - frames := err.Frames() - require.Len(t, frames, 2) - assert.Empty(t, frames[0].Hints) - assert.Empty(t, frames[1].Hints) +func TestHints_PackageLevel_HarborError(t *testing.T) { + err := harborerr.New("error", "check config") + assert.Equal(t, []string{"check config"}, harborerr.Hints(err)) } -func TestChaining_WrapWithHints(t *testing.T) { - root := harborerr.New("connection refused").WithHint("check firewall rules") - top := harborerr.Wrap(root, "could not reach registry").WithHint("verify server URL") - - assert.Equal(t, "connection refused", top.Error()) - assert.Equal(t, "could not reach registry", top.Message()) - assert.Equal(t, []string{"could not reach registry", "connection refused"}, top.Errors()) - assert.Equal(t, []string{"verify server URL", "check firewall rules"}, top.Hints()) +func TestHints_PackageLevel_PlainError(t *testing.T) { + assert.Nil(t, harborerr.Hints(errors.New("plain"))) } diff --git a/pkg/views/styles.go b/pkg/views/styles.go index 240ef23bc..a216dc049 100644 --- a/pkg/views/styles.go +++ b/pkg/views/styles.go @@ -14,8 +14,6 @@ package views import ( - "strings" - "github.com/charmbracelet/bubbles/list" "github.com/charmbracelet/lipgloss" ) @@ -37,10 +35,24 @@ var ( var BaseStyle = lipgloss.NewStyle(). BorderStyle(lipgloss.NormalBorder()).Padding(0, 1) -func RedText(strs ...string) string { - var msg strings.Builder - for _, str := range strs { - msg.WriteString(str) - } - return RedStyle.Render(msg.String()) -} +var ( + ErrCauseStyle = lipgloss.NewStyle(). + Foreground(lipgloss.Color("203")). + Bold(true) + + ErrTitleStyle = lipgloss.NewStyle(). + Foreground(lipgloss.Color("214")) + + ErrHintStyle = lipgloss.NewStyle(). + Foreground(lipgloss.Color("44")) + + ErrEnumeratorStyle = lipgloss.NewStyle(). + Foreground(lipgloss.Color("242")).MarginRight(1) +) + +const ( + GreenANSI = "\033[32m" + RedANSI = "\033[31m" + BoldANSI = "\033[1m" + ResetANSI = "\033[0m" +) From 56380a934fef87f7ff811e67d8cbc1e9c08a3e7a Mon Sep 17 00:00:00 2001 From: vg006 Date: Tue, 31 Mar 2026 18:37:33 +0530 Subject: [PATCH 10/11] refac: update return statements with pkg/errors Signed-off-by: vg006 --- cmd/harbor/root/artifact/list.go | 16 ++++++------- pkg/prompt/prompt.go | 6 ++--- pkg/utils/client.go | 12 +++++----- pkg/utils/config.go | 40 ++++++++++++++++---------------- pkg/utils/encryption.go | 36 ++++++++++++++-------------- pkg/utils/helper.go | 5 ++-- pkg/utils/utils.go | 4 ++-- 7 files changed, 59 insertions(+), 60 deletions(-) diff --git a/cmd/harbor/root/artifact/list.go b/cmd/harbor/root/artifact/list.go index bf8ee4980..4d9b3ed30 100644 --- a/cmd/harbor/root/artifact/list.go +++ b/cmd/harbor/root/artifact/list.go @@ -14,10 +14,9 @@ package artifact import ( - "fmt" - "github.com/goharbor/go-client/pkg/sdk/v2.0/client/artifact" "github.com/goharbor/harbor-cli/pkg/api" + "github.com/goharbor/harbor-cli/pkg/errors" "github.com/goharbor/harbor-cli/pkg/prompt" "github.com/goharbor/harbor-cli/pkg/utils" artifactViews "github.com/goharbor/harbor-cli/pkg/views/artifact/list" @@ -44,11 +43,11 @@ Supports pagination, search queries, and sorting using flags.`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if opts.PageSize < 0 { - return fmt.Errorf("page size must be greater than or equal to 0") + return errors.New("Invalid page size", "page size must be greater than or equal to 0") } if opts.PageSize > 100 { - return fmt.Errorf("page size should be less than or equal to 100") + return errors.New("Invalid page size", "page size must be less than or equal to 100") } var err error var artifacts artifact.ListArtifactsOK @@ -57,12 +56,12 @@ Supports pagination, search queries, and sorting using flags.`, if len(args) > 0 { projectName, repoName, err = utils.ParseProjectRepo(args[0]) if err != nil { - return fmt.Errorf("failed to parse project/repo: %v", err) + return errors.New("Invalid project/repository format", "expected format: /") } } else { projectName, err = prompt.GetProjectNameFromUser() if err != nil { - return fmt.Errorf("failed to get project name: %v", utils.ParseHarborErrorMsg(err)) + return errors.AsError(err).WithMessage("failed to get project name") } repoName = prompt.GetRepoNameFromUser(projectName) } @@ -70,13 +69,12 @@ Supports pagination, search queries, and sorting using flags.`, artifacts, err = api.ListArtifact(projectName, repoName, opts) if err != nil { - return fmt.Errorf("failed to list artifacts: %v", err) + return errors.AsError(err).WithMessage("failed to list artifacts") } FormatFlag := viper.GetString("output-format") if FormatFlag != "" { - err = utils.PrintFormat(artifacts, FormatFlag) - if err != nil { + if err = utils.PrintFormat(artifacts, FormatFlag); err != nil { return err } } else { diff --git a/pkg/prompt/prompt.go b/pkg/prompt/prompt.go index 2ed2e2890..0425907cd 100644 --- a/pkg/prompt/prompt.go +++ b/pkg/prompt/prompt.go @@ -14,7 +14,6 @@ package prompt import ( - "errors" "fmt" "strconv" @@ -24,6 +23,7 @@ import ( "github.com/goharbor/go-client/pkg/sdk/v2.0/models" "github.com/goharbor/harbor-cli/pkg/api" "github.com/goharbor/harbor-cli/pkg/constants" + "github.com/goharbor/harbor-cli/pkg/errors" aview "github.com/goharbor/harbor-cli/pkg/views/artifact/select" tview "github.com/goharbor/harbor-cli/pkg/views/artifact/tags/select" immview "github.com/goharbor/harbor-cli/pkg/views/immutable/select" @@ -102,7 +102,7 @@ func GetProjectNameFromUser() (string, error) { go func() { response, err := api.ListAllProjects() if err != nil { - resultChan <- result{"", err} + resultChan <- result{"", errors.NewWithCause(err, "failed to list projects")} return } @@ -116,7 +116,7 @@ func GetProjectNameFromUser() (string, error) { if err == pview.ErrUserAborted { resultChan <- result{"", errors.New("user aborted project selection")} } else { - resultChan <- result{"", fmt.Errorf("error during project selection: %w", err)} + resultChan <- result{"", errors.NewWithCause(err, "error during project selection")} } return } diff --git a/pkg/utils/client.go b/pkg/utils/client.go index 8929661ca..aa495418f 100644 --- a/pkg/utils/client.go +++ b/pkg/utils/client.go @@ -15,11 +15,11 @@ package utils import ( "context" - "fmt" "sync" "github.com/goharbor/go-client/pkg/harbor" v2client "github.com/goharbor/go-client/pkg/sdk/v2.0/client" + "github.com/goharbor/harbor-cli/pkg/errors" log "github.com/sirupsen/logrus" ) @@ -33,12 +33,12 @@ func GetClient() (*v2client.HarborAPI, error) { ClientOnce.Do(func() { config, err := GetCurrentHarborConfig() if err != nil { - ClientErr = fmt.Errorf("failed to get current credential name: %v", err) + ClientErr = errors.AsError(err).WithMessage("failed to get current credential name") return } credentialName := config.CurrentCredentialName if credentialName == "" { - ClientErr = fmt.Errorf("no Harbor credentials found. Please run `harbor login` to configure access") + ClientErr = errors.New("no Harbor credentials found.", "Please run `harbor login` to configure access") return } @@ -73,19 +73,19 @@ func GetClientByConfig(clientConfig *harbor.ClientSetConfig) *v2client.HarborAPI func GetClientByCredentialName(credentialName string) (*v2client.HarborAPI, error) { credential, err := GetCredentials(credentialName) if err != nil { - return nil, fmt.Errorf("failed to get credential %s: %w", credentialName, err) + return nil, errors.AsError(err).WithMessage("failed to get credential", "credential: "+credentialName) } // Get encryption key key, err := GetEncryptionKey() if err != nil { - return nil, fmt.Errorf("failed to get encryption key: %w", err) + return nil, errors.NewWithCause(err, "failed to get encryption key") } // Decrypt password decryptedPassword, err := Decrypt(key, string(credential.Password)) if err != nil { - return nil, fmt.Errorf("failed to decrypt password: %w", err) + return nil, errors.NewWithCause(err, "failed to decrypt password") } clientConfig := &harbor.ClientSetConfig{ diff --git a/pkg/utils/config.go b/pkg/utils/config.go index 0a28e0ee0..bbd40fad7 100644 --- a/pkg/utils/config.go +++ b/pkg/utils/config.go @@ -14,13 +14,13 @@ package utils import ( - "errors" "fmt" "os" "path/filepath" "sync" "github.com/goharbor/go-client/pkg/sdk/v2.0/models" + "github.com/goharbor/harbor-cli/pkg/errors" log "github.com/sirupsen/logrus" "github.com/spf13/viper" @@ -75,7 +75,7 @@ func InitConfig(cfgFile string, userSpecifiedConfig bool) { // Ensure data directory exists if err := os.MkdirAll(harborDataDir, os.ModePerm); err != nil { - configInitError = fmt.Errorf("failed to create data directory: %w", err) + configInitError = errors.NewWithCause(err, "failed to create data directory") log.Fatalf("%v", configInitError) } @@ -100,7 +100,7 @@ func InitConfig(cfgFile string, userSpecifiedConfig bool) { var harborConfig HarborConfig if err := viper.Unmarshal(&harborConfig); err != nil { - configInitError = fmt.Errorf("failed to unmarshal config file: %w", err) + configInitError = errors.NewWithCause(err, "failed to unmarshal config file") log.Fatalf("%v", configInitError) } @@ -135,7 +135,7 @@ func DetermineConfigPath(cfgFile string, userSpecifiedConfig bool) (string, erro if userSpecifiedConfig && cfgFile != "" { harborConfigPath, err = filepath.Abs(cfgFile) if err != nil { - return "", fmt.Errorf("failed to resolve absolute path for config file: %w", err) + return "", errors.NewWithCause(err, "failed to resolve absolute path for config file") } return harborConfigPath, nil } @@ -144,7 +144,7 @@ func DetermineConfigPath(cfgFile string, userSpecifiedConfig bool) (string, erro if harborEnvVar != "" { harborConfigPath, err = filepath.Abs(harborEnvVar) if err != nil { - return "", fmt.Errorf("failed to resolve absolute path for config file from HARBOR_CLI_CONFIG: %w", err) + return "", errors.NewWithCause(err, "failed to resolve absolute path for config file from HARBOR_CLI_CONFIG") } return harborConfigPath, nil } @@ -154,13 +154,13 @@ func DetermineConfigPath(cfgFile string, userSpecifiedConfig bool) (string, erro if xdgConfigHome == "" { home, err := os.UserHomeDir() if err != nil { - return "", fmt.Errorf("unable to determine user home directory: %w", err) + return "", errors.NewWithCause(err, "unable to determine user home directory") } xdgConfigHome = filepath.Join(home, ".config") } harborConfigPath, err = filepath.Abs(filepath.Join(xdgConfigHome, "harbor-cli", "config.yaml")) if err != nil { - return "", fmt.Errorf("failed to resolve absolute path for default config file: %w", err) + return "", errors.NewWithCause(err, "failed to resolve absolute path for default config file") } return harborConfigPath, nil } @@ -170,22 +170,22 @@ func EnsureConfigFileExists(harborConfigPath string) error { // Ensure parent directory exists configDir := filepath.Dir(harborConfigPath) if err := os.MkdirAll(configDir, os.ModePerm); err != nil { - return fmt.Errorf("failed to create config directory: %w", err) + return errors.NewWithCause(err, "failed to create config directory") } if fileInfo, err := os.Stat(harborConfigPath); err != nil { // No need to throw error if config file does not exist if !os.IsNotExist(err) { - return fmt.Errorf("error while checking config file: %w", err) + return errors.NewWithCause(err, "error while checking config file") } } else if fileInfo.IsDir() { - return fmt.Errorf("expected a file but found a directory at path: %s", harborConfigPath) + return errors.Newf("expected a file but found a directory at path: %s", harborConfigPath) } // Create config file if it doesn't exist if _, err := os.Stat(harborConfigPath); os.IsNotExist(err) { if err := CreateConfigFile(harborConfigPath); err != nil { - return fmt.Errorf("failed to create config file: %w", err) + return errors.NewWithCause(err, "failed to create config file") } } return nil @@ -196,7 +196,7 @@ func ReadConfig(harborConfigPath string) error { viper.SetConfigFile(harborConfigPath) viper.SetConfigType("yaml") if err := viper.ReadInConfig(); err != nil { - return fmt.Errorf("error reading config file: %w. Please ensure the config file exists", err) + return errors.NewWithCause(err, "error reading config file", "Please ensure the config file exists") } return nil } @@ -207,7 +207,7 @@ func GetCurrentHarborConfig() (*HarborConfig, error) { }) if configInitError != nil { - return nil, fmt.Errorf("initialization error: %w", configInitError) + return nil, errors.NewWithCause(configInitError, "initialization error") } configMutex.RLock() @@ -267,7 +267,7 @@ func GetCurrentHarborData() (*HarborData, error) { }) if configInitError != nil { - return nil, fmt.Errorf("initialization error: %w", configInitError) + return nil, errors.NewWithCause(configInitError, "initialization error") } configMutex.RLock() @@ -320,7 +320,7 @@ func ReadDataFile(dataFilePath string) (HarborData, error) { v.SetConfigFile(dataFilePath) if err := v.ReadInConfig(); err != nil { - return dataFile, fmt.Errorf("failed to read data file: %v", err) + return dataFile, errors.NewWithCause(err, "failed to read data file") } dataFile.ConfigPath = v.GetString("configPath") @@ -334,11 +334,11 @@ func ApplyDataFile(harborDataPath, harborConfigPath string) error { if err == nil { if dataFileContent.ConfigPath != harborConfigPath { if err := UpdateDataFile(harborDataPath, harborConfigPath); err != nil { - return fmt.Errorf("failed to update data file: %w", err) + return errors.NewWithCause(err, "failed to update data file") } } else if dataFileContent.ConfigPath == "" { if err := CreateDataFile(harborDataPath, harborConfigPath); err != nil { - return fmt.Errorf("failed to create data file: %w", err) + return errors.NewWithCause(err, "failed to create data file") } } else { log.Debugf("Data file already exists with the same config path: %s", harborConfigPath) @@ -346,7 +346,7 @@ func ApplyDataFile(harborDataPath, harborConfigPath string) error { } else { // Data file does not exist, create it if err := CreateDataFile(harborDataPath, harborConfigPath); err != nil { - return fmt.Errorf("failed to create data file: %w", err) + return errors.NewWithCause(err, "failed to create data file") } } return nil @@ -459,7 +459,7 @@ func UpdateConfigFile(config *HarborConfig) error { func GetCredentials(credentialName string) (Credential, error) { currentConfig, err := GetCurrentHarborConfig() if err != nil { - return Credential{}, fmt.Errorf("failed to get current Harbor configuration: %w", err) + return Credential{}, errors.AsError(err).WithMessage("failed to get current Harbor configuration") } if currentConfig == nil || currentConfig.Credentials == nil { @@ -472,7 +472,7 @@ func GetCredentials(credentialName string) (Credential, error) { } } - return Credential{}, fmt.Errorf("credential with name '%s' not found", credentialName) + return Credential{}, errors.Newf("credential with name '%s' not found", credentialName) } func AddCredentialsToConfigFile(credential Credential, configPath string) error { diff --git a/pkg/utils/encryption.go b/pkg/utils/encryption.go index 0f61840c8..e8d870512 100644 --- a/pkg/utils/encryption.go +++ b/pkg/utils/encryption.go @@ -18,13 +18,13 @@ import ( "crypto/cipher" "crypto/rand" "encoding/base64" - "fmt" "io" "os" "path/filepath" "strings" "sync" + "github.com/goharbor/harbor-cli/pkg/errors" "github.com/sirupsen/logrus" "github.com/zalando/go-keyring" ) @@ -65,7 +65,7 @@ type FileKeyring struct { func (f *FileKeyring) Set(service, user, password string) error { if err := os.MkdirAll(f.BaseDir, 0700); err != nil { - return fmt.Errorf("failed to create directory: %w", err) + return errors.NewWithCause(err, "failed to create directory") } filename := filepath.Join(f.BaseDir, sanitizeFilename(service+"_"+user)) @@ -104,7 +104,7 @@ type EnvironmentKeyring struct { func (e *EnvironmentKeyring) Set(service, user, password string) error { // Set environment variable for the current process if err := os.Setenv(e.EnvVarName, password); err != nil { - return fmt.Errorf("failed to set environment variable: %w", err) + return errors.NewWithCause(err, "failed to set environment variable") } return nil } @@ -112,14 +112,14 @@ func (e *EnvironmentKeyring) Set(service, user, password string) error { func (e *EnvironmentKeyring) Get(service, user string) (string, error) { value := os.Getenv(e.EnvVarName) if value == "" { - return "", fmt.Errorf("environment variable %s not found or empty", e.EnvVarName) + return "", errors.Newf("environment variable %s not found or empty", e.EnvVarName) } return value, nil } func (e *EnvironmentKeyring) Delete(service, user string) error { // Can't delete environment variables at runtime - return fmt.Errorf("deleting environment variables at runtime is not supported") + return errors.New("deleting environment variables at runtime is not supported") } // GetKeyringProvider selects the appropriate keyring provider @@ -173,7 +173,7 @@ func GenerateEncryptionKey() error { key := make([]byte, 32) // AES-256 key if _, err := rand.Read(key); err != nil { - return fmt.Errorf("failed to generate encryption key: %w", err) + return errors.NewWithCause(err, "failed to generate encryption key") } return keyringProvider.Set(KeyringService, KeyringUser, base64.StdEncoding.EncodeToString(key)) } @@ -184,23 +184,23 @@ func GetEncryptionKey() ([]byte, error) { if err != nil || keyBase64 == "" { // Attempt to generate a new key if not found if genErr := GenerateEncryptionKey(); genErr != nil { - return nil, fmt.Errorf("failed to retrieve or generate encryption key: %w", err) + return nil, errors.NewWithCause(genErr, "failed to retrieve or generate encryption key") } keyBase64, err = keyringProvider.Get(KeyringService, KeyringUser) if err != nil { - return nil, fmt.Errorf("failed to retrieve encryption key after generation: %w", err) + return nil, errors.NewWithCause(err, "failed to retrieve encryption key after generation") } } key, err := base64.StdEncoding.DecodeString(keyBase64) if err != nil { - return nil, fmt.Errorf("failed to decode encryption key: %w", err) + return nil, errors.NewWithCause(err, "failed to decode encryption key") } // Validate the key size for AES keySize := len(key) if keySize != 16 && keySize != 24 && keySize != 32 { - return nil, fmt.Errorf("invalid encryption key size: %d bytes. Must be 16, 24, or 32 bytes (after base64 decoding)", keySize) + return nil, errors.Newf("invalid encryption key size: %d bytes. Must be 16, 24, or 32 bytes (after base64 decoding)", keySize) } return key, nil @@ -209,17 +209,17 @@ func GetEncryptionKey() ([]byte, error) { func Encrypt(key, plaintext []byte) (string, error) { block, err := aes.NewCipher(key) if err != nil { - return "", fmt.Errorf("failed to create cipher: %w", err) + return "", errors.NewWithCause(err, "failed to create cipher") } aesGCM, err := cipher.NewGCM(block) if err != nil { - return "", fmt.Errorf("failed to create GCM: %w", err) + return "", errors.NewWithCause(err, "failed to create GCM") } nonce := make([]byte, aesGCM.NonceSize()) if _, err := io.ReadFull(rand.Reader, nonce); err != nil { - return "", fmt.Errorf("failed to generate nonce: %w", err) + return "", errors.NewWithCause(err, "failed to generate nonce") } ciphertext := aesGCM.Seal(nonce, nonce, plaintext, nil) @@ -229,29 +229,29 @@ func Encrypt(key, plaintext []byte) (string, error) { func Decrypt(key []byte, ciphertext string) (string, error) { block, err := aes.NewCipher(key) if err != nil { - return "", fmt.Errorf("failed to create cipher: %w", err) + return "", errors.NewWithCause(err, "failed to create cipher") } aesGCM, err := cipher.NewGCM(block) if err != nil { - return "", fmt.Errorf("failed to create GCM: %w", err) + return "", errors.NewWithCause(err, "failed to create GCM") } data, err := base64.StdEncoding.DecodeString(ciphertext) if err != nil { - return "", fmt.Errorf("failed to decode ciphertext: %w", err) + return "", errors.NewWithCause(err, "failed to decode ciphertext") } nonceSize := aesGCM.NonceSize() if len(data) < nonceSize { - return "", fmt.Errorf("ciphertext too short") + return "", errors.New("ciphertext too short") } nonce := data[:nonceSize] ciphertextBytes := data[nonceSize:] plaintext, err := aesGCM.Open(nil, nonce, ciphertextBytes, nil) if err != nil { - return "", fmt.Errorf("failed to decrypt ciphertext: %w", err) + return "", errors.NewWithCause(err, "failed to decrypt ciphertext") } return string(plaintext), nil diff --git a/pkg/utils/helper.go b/pkg/utils/helper.go index cd50ba3b0..45b74812c 100644 --- a/pkg/utils/helper.go +++ b/pkg/utils/helper.go @@ -14,7 +14,6 @@ package utils import ( - "errors" "fmt" "net" "net/url" @@ -23,6 +22,8 @@ import ( "strings" "time" "unicode" + + "github.com/goharbor/harbor-cli/pkg/errors" ) func pluralise(value int, unit string) string { @@ -212,7 +213,7 @@ func PrintFormat[T any](resp T, format string) error { PrintPayloadInYAMLFormat(resp) return nil } - return fmt.Errorf("unable to output in the specified '%s' format", format) + return errors.Newf("unable to output in the specified '%s' format", format) } func EmptyStringValidator(variable string) func(string) error { diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index de1b83bc5..e677728d5 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -16,7 +16,6 @@ package utils import ( "context" "encoding/json" - "errors" "fmt" "os" "regexp" @@ -26,6 +25,7 @@ import ( "github.com/charmbracelet/bubbles/table" "github.com/goharbor/go-client/pkg/sdk/v2.0/client/user" + "github.com/goharbor/harbor-cli/pkg/errors" uview "github.com/goharbor/harbor-cli/pkg/views/user/select" log "github.com/sirupsen/logrus" "github.com/spf13/viper" @@ -66,7 +66,7 @@ func PrintPayloadInYAMLFormat(payload any) { func ParseProjectRepo(projectRepo string) (project, repo string, err error) { split := strings.SplitN(projectRepo, "/", 2) // splits only at first slash if len(split) != 2 { - return "", "", fmt.Errorf("invalid project/repository format: %s", projectRepo) + return "", "", errors.Newf("invalid project/repository format: %s", projectRepo) } return split[0], split[1], nil } From 27bfaa706bb4afee59e183b5aa991e87c532f830 Mon Sep 17 00:00:00 2001 From: vg006 Date: Tue, 7 Apr 2026 17:15:21 +0530 Subject: [PATCH 11/11] fix: avoid new error creation Signed-off-by: vg006 --- cmd/harbor/root/artifact/list.go | 6 +++--- pkg/utils/utils.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/harbor/root/artifact/list.go b/cmd/harbor/root/artifact/list.go index 4d9b3ed30..1b1d793fc 100644 --- a/cmd/harbor/root/artifact/list.go +++ b/cmd/harbor/root/artifact/list.go @@ -56,12 +56,12 @@ Supports pagination, search queries, and sorting using flags.`, if len(args) > 0 { projectName, repoName, err = utils.ParseProjectRepo(args[0]) if err != nil { - return errors.New("Invalid project/repository format", "expected format: /") + return err } } else { projectName, err = prompt.GetProjectNameFromUser() if err != nil { - return errors.AsError(err).WithMessage("failed to get project name") + return err } repoName = prompt.GetRepoNameFromUser(projectName) } @@ -69,7 +69,7 @@ Supports pagination, search queries, and sorting using flags.`, artifacts, err = api.ListArtifact(projectName, repoName, opts) if err != nil { - return errors.AsError(err).WithMessage("failed to list artifacts") + return err } FormatFlag := viper.GetString("output-format") diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go index e677728d5..bc101ba66 100644 --- a/pkg/utils/utils.go +++ b/pkg/utils/utils.go @@ -66,7 +66,7 @@ func PrintPayloadInYAMLFormat(payload any) { func ParseProjectRepo(projectRepo string) (project, repo string, err error) { split := strings.SplitN(projectRepo, "/", 2) // splits only at first slash if len(split) != 2 { - return "", "", errors.Newf("invalid project/repository format: %s", projectRepo) + return "", "", errors.New(fmt.Sprintf("Invalid project/repository format: %s", projectRepo), "expected format: /") } return split[0], split[1], nil }