Skip to content

Commit

Permalink
feat: refactor error message format
Browse files Browse the repository at this point in the history
  • Loading branch information
@binbin-li
binbin-li committed Aug 1, 2024
1 parent 2f53832 commit 34081c4
Show file tree
Hide file tree
Showing 4 changed files with 144 additions and 65 deletions.
4 changes: 2 additions & 2 deletions cmd/ratify/cmd/cmd_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ func TestDiscover(t *testing.T) {

// TODO: make ratify cli more unit testable
// unit test should not need to resolve real image
if !strings.Contains(err.Error(), "referrer store failure") {
t.Errorf("error expected")
if !strings.Contains(err.Error(), "REFERRER_STORE_FAILURE") {
t.Errorf("expected containing: %s, but got: %s", "REFERRER_STORE_FAILURE", err.Error())
}
}

Expand Down
86 changes: 56 additions & 30 deletions errors/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,10 @@ type Error struct {
Detail interface{} `json:"detail,omitempty"`
ComponentType ComponentType `json:"componentType,omitempty"`
PluginName string `json:"pluginName,omitempty"`
LinkToDoc string `json:"linkToDoc,omitempty"`
Remediation string `json:"remediation,omitempty"`
Stack string `json:"stack,omitempty"`
Description string `json:"description,omitempty"`
isRootError bool // isRootError is true if the originalError is either nil or not an Error type
}

// ErrorDescriptor provides relevant information about a given error code.
Expand Down Expand Up @@ -152,7 +153,7 @@ func (ec ErrorCode) WithComponentType(componentType ComponentType) Error {

// WithLinkToDoc returns a new Error object with attached link to the documentation.
func (ec ErrorCode) WithLinkToDoc(link string) Error {
return newError(ec, ec.Message()).WithLinkToDoc(link)
return newError(ec, ec.Message()).WithRemediation(link)
}

func (ec ErrorCode) WithDescription() Error {
Expand All @@ -165,7 +166,7 @@ func (ec ErrorCode) WithPluginName(pluginName string) Error {
}

// NewError returns a new Error object.
func (ec ErrorCode) NewError(componentType ComponentType, pluginName, link string, err error, detail interface{}, printStackTrace bool) Error {
func (ec ErrorCode) NewError(componentType ComponentType, pluginName, remediation string, err error, detail interface{}, printStackTrace bool) Error {
stack := ""
if printStackTrace {
stack = getStackTrace()
Expand All @@ -177,15 +178,17 @@ func (ec ErrorCode) NewError(componentType ComponentType, pluginName, link strin
Detail: detail,
ComponentType: componentType,
PluginName: pluginName,
LinkToDoc: link,
Remediation: remediation,
Stack: stack,
isRootError: err == nil || !errors.As(err, &Error{}),
}
}

func newError(code ErrorCode, message string) Error {
return Error{
Code: code,
Message: message,
Code: code,
Message: message,
isRootError: true,
}
}

Expand All @@ -209,39 +212,61 @@ func (e Error) Unwrap() error {
}

// Error returns a human readable representation of the error.
// An Error message includes the error code, detail from nested errors, root cause and remediation, all separated by ": ".
func (e Error) Error() string {
var errStr string
if e.OriginalError != nil {
errStr += fmt.Sprintf("Original Error: (%s), ", e.OriginalError.Error())
err, details := e.getRootError()
if err.Detail != nil {
details = append(details, fmt.Sprintf("%s", err.Detail))
}

errStr += fmt.Sprintf("Error: %s, Code: %s", e.Message, e.Code.String())

if e.PluginName != "" {
errStr += fmt.Sprintf(", Plugin Name: %s", e.PluginName)
if err.OriginalError != nil {
details = append(details, err.OriginalError.Error())
}

if e.ComponentType != "" {
errStr += fmt.Sprintf(", Component Type: %s", e.ComponentType)
if err.Remediation != "" {
details = append(details, err.Remediation)
}
return fmt.Sprintf("%s: %s", err.ErrorCode().Descriptor().Value, strings.Join(details, ": "))
}

if e.LinkToDoc != "" {
errStr += fmt.Sprintf(", Documentation: %s", e.LinkToDoc)
// GetFullDetails returns details from all nested errors.
func (e Error) GetFullDetails() string {
err, details := e.getRootError()
if err.OriginalError != nil && err.Detail != nil {
details = append(details, fmt.Sprintf("%s", err.Detail))
}

if e.Detail != nil {
errStr += fmt.Sprintf(", Detail: %v", e.Detail)
}
return strings.Join(details, ": ")
}

if e.Description != "" {
errStr += fmt.Sprintf(", Description: %v", e.Description)
// GetRootCause returns the root cause of the error.
func (e Error) GetRootCause() string {
err, _ := e.getRootError()
if err.OriginalError != nil {
return err.OriginalError.Error()
}
return fmt.Sprintf("%s", err.Detail)
}

if e.Stack != "" {
errStr += fmt.Sprintf(", Stack trace: %s", e.Stack)
}
func (e Error) GetRootRemediation() string {
err, _ := e.getRootError()
return err.Remediation
}

return errStr
func (e Error) getRootError() (err Error, details []string) {
err = e
for !err.isRootError {
if err.Detail != nil {
details = append(details, fmt.Sprintf("%s", err.Detail))
}
var ratifyError Error
if errors.As(err.OriginalError, &ratifyError) {
err = ratifyError
} else {
// break is unnecessary, but added for safety
break
}
}
return err, details
}

// WithDetail will return a new Error, based on the current one, but with
Expand All @@ -266,12 +291,13 @@ func (e Error) WithComponentType(componentType ComponentType) Error {
// WithError returns a new Error object with original error.
func (e Error) WithError(err error) Error {
e.OriginalError = err
e.isRootError = err == nil || !errors.As(err, &Error{})
return e
}

// WithLinkToDoc returns a new Error object attached with link to documentation.
func (e Error) WithLinkToDoc(link string) Error {
e.LinkToDoc = link
// WithRemediation returns a new Error object attached with remediation.
func (e Error) WithRemediation(remediation string) Error {
e.Remediation = remediation
return e
}

Expand Down
117 changes: 85 additions & 32 deletions errors/types_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,31 @@ import (
)

const (
testGroup = "test-group"
testValue = "test-value"
testMessage = "test-message"
testDescription = "test-description"
testDetail = "test-detail"
testComponentType = "test-component-type"
testLink = "test-link"
testPluginName = "test-plugin-name"
testErrorString = "Original Error: (Error: , Code: UNKNOWN), Error: test-message, Code: test-value, Plugin Name: test-plugin-name, Component Type: test-component-type, Documentation: test-link, Detail: test-detail"
nonexistentEC = 2000
testGroup = "test-group"
testErrCode1 = "TEST_ERROR_CODE_1"
testErrCode2 = "TEST_ERROR_CODE_2"
testMessage = "test-message"
testDescription = "test-description"
testDetail1 = "test-detail-1"
testDetail2 = "test-detail-2"
testComponentType1 = "test-component-type-1"
testComponentType2 = "test-component-type-2"
testLink1 = "test-link-1"
testLink2 = "test-link-2"
testPluginName = "test-plugin-name"
nonexistentEC = 2000
)

var (
testEC = Register(testGroup, ErrorDescriptor{
Value: testValue,
Value: testErrCode1,
Message: testMessage,
Description: testDescription,
})

testEC2 = Register(testGroup, ErrorDescriptor{})
testEC2 = Register(testGroup, ErrorDescriptor{
Value: testErrCode2,
})
)

func TestErrorCode(t *testing.T) {
Expand All @@ -52,8 +57,9 @@ func TestErrorCode(t *testing.T) {
}

func TestError(t *testing.T) {
if testEC.Error() != testValue {
t.Fatalf("expected: %s, got: %s", testValue, testEC.Error())
expectedStr := "test error code 1"
if testEC.Error() != expectedStr {
t.Fatalf("expected: %s, got: %s", expectedStr, testEC.Error())
}
}

Expand All @@ -66,7 +72,7 @@ func TestDescriptor(t *testing.T) {
{
name: "existing error code",
ec: testEC,
expectedValue: testValue,
expectedValue: testErrCode1,
},
{
name: "nonexistent error code",
Expand All @@ -92,9 +98,9 @@ func TestMessage(t *testing.T) {
}

func TestWithDetail(t *testing.T) {
err := testEC.WithDetail(testDetail)
if err.Detail != testDetail {
t.Fatalf("expected detail: %s, got: %s", testDetail, err.Detail)
err := testEC.WithDetail(testDetail1)
if err.Detail != testDetail1 {
t.Fatalf("expected detail: %s, got: %s", testDetail1, err.Detail)
}
}

Expand All @@ -106,16 +112,16 @@ func TestWithError(t *testing.T) {
}

func TestWithComponentType(t *testing.T) {
err := testEC.WithComponentType(testComponentType)
if err.ComponentType != testComponentType {
t.Fatalf("expected component type: %s, got: %s", testComponentType, err.ComponentType)
err := testEC.WithComponentType(testComponentType1)
if err.ComponentType != testComponentType1 {
t.Fatalf("expected component type: %s, got: %s", testComponentType1, err.ComponentType)
}
}

func TestWithLinkToDoc(t *testing.T) {
err := testEC.WithLinkToDoc(testLink)
if err.LinkToDoc != testLink {
t.Fatalf("expected link to doc: %s, got: %s", testLink, err.LinkToDoc)
err := testEC.WithLinkToDoc(testLink1)
if err.Remediation != testLink1 {
t.Fatalf("expected link to doc: %s, got: %s", testLink1, err.Remediation)
}
}

Expand All @@ -134,7 +140,7 @@ func TestWithDescription(t *testing.T) {
}

func TestIs(t *testing.T) {
err := testEC.WithDetail(testDetail)
err := testEC.WithDetail(testDetail1)
result := err.Is(err)
if !result {
t.Fatalf("expected true, got: %v", result)
Expand Down Expand Up @@ -168,17 +174,64 @@ func TestIsEmpty(t *testing.T) {
}

func TestError_Error(t *testing.T) {
err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType).WithLinkToDoc(testLink).WithDetail(testDetail).WithError(Error{}).WithDescription()
result := err.Error()
if !strings.HasPrefix(result, testErrorString) {
t.Fatalf("expected string starts with: %s, but got: %s", testErrorString, result)
// Nested errors.
rootErr := testEC.NewError(testComponentType1, "", testLink1, errors.New(testMessage), testDetail1, false)
err := testEC2.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr)

expectedErrStr := strings.Join([]string{testErrCode1, testDetail2, testDetail1, testMessage, testLink1}, ": ")
if err.Error() != expectedErrStr {
t.Fatalf("expected string: %s, but got: %s", expectedErrStr, err.Error())
}

// Single error.
err = testEC.WithDetail(testDetail1)
expectedErrStr = "TEST_ERROR_CODE_1: test-detail-1"
if err.Error() != expectedErrStr {
t.Fatalf("expected string: %s, but got: %s", expectedErrStr, err.Error())
}
}

func TestError_GetRootCause(t *testing.T) {
// rootErr contains original error.
rootErr := testEC.NewError(testComponentType1, "", testLink1, errors.New(testMessage), testDetail1, false)
err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr)

if err.GetRootCause() != testMessage {
t.Fatalf("expected root cause: %v, but got: %v", err.GetRootCause(), testMessage)
}

// rootErr does not contain original error.
rootErr = testEC.NewError(testComponentType1, "", testLink1, nil, testDetail1, false)
err = testEC.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr)

if err.GetRootCause() != testDetail1 {
t.Fatalf("expected root cause: %v, but got: %v", err.GetRootCause(), testDetail1)
}
}

func TestError_GetFullDetails(t *testing.T) {
rootErr := testEC.NewError(testComponentType1, "", testLink1, errors.New(testMessage), testDetail1, false)
err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr)

expectedDetails := strings.Join([]string{testDetail2, testDetail1}, ": ")
if err.GetFullDetails() != expectedDetails {
t.Fatalf("expected full details: %v, but got: %v", expectedDetails, err.GetFullDetails())
}
}

func TestError_GetRootRemediation(t *testing.T) {
rootErr := testEC.NewError(testComponentType1, "", testLink1, errors.New(testMessage), testDetail1, false)
err := testEC.WithPluginName(testPluginName).WithComponentType(testComponentType2).WithRemediation(testLink2).WithDetail(testDetail2).WithError(rootErr)

if err.GetRootRemediation() != testLink1 {
t.Fatalf("expected root remediation: %v, but got: %v", err.GetRootRemediation(), testLink1)
}
}

func TestNewError(t *testing.T) {
err := testEC.NewError(testComponentType, testPluginName, testLink, Error{}, testDetail, false)
err := testEC.NewError(testComponentType1, testPluginName, testLink1, Error{}, testDetail1, false)

if err.ComponentType != testComponentType || err.PluginName != testPluginName || err.LinkToDoc != testLink || err.Detail != testDetail {
t.Fatalf("expected component type: %s, plugin name: %s, link to doc: %s, detail: %s, but got: %s, %s, %s, %s", testComponentType, testPluginName, testLink, testDetail, err.ComponentType, err.PluginName, err.LinkToDoc, err.Detail)
if err.ComponentType != testComponentType1 || err.PluginName != testPluginName || err.Remediation != testLink1 || err.Detail != testDetail1 {
t.Fatalf("expected component type: %s, plugin name: %s, link to doc: %s, detail: %s, but got: %s, %s, %s, %s", testComponentType1, testPluginName, testLink1, testDetail1, err.ComponentType, err.PluginName, err.Remediation, err.Detail)
}
}
2 changes: 1 addition & 1 deletion test/bats/plugin-test.bats
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ RATIFY_NAMESPACE=gatekeeper-system
sed 's/licensechecker/invalidlicensechecker/' ./config/samples/clustered/verifier/config_v1beta1_verifier_complete_licensechecker.yaml >invalidVerifier.yaml
run kubectl apply -f invalidVerifier.yaml
assert_success
run bash -c "kubectl describe verifiers.config.ratify.deislabs.io/verifier-license-checker -n ${RATIFY_NAMESPACE} | grep 'Brieferror: Original Error:'"
run bash -c "kubectl describe verifiers.config.ratify.deislabs.io/verifier-license-checker -n ${RATIFY_NAMESPACE} | grep 'Brieferror: PLUGIN_NOT_FOUND:'"
assert_success

# apply a valid verifier, validate status property shows success
Expand Down

0 comments on commit 34081c4

Please sign in to comment.