Serialize wrapped errors (#62)

* serialize wrapped errors * rename Error as WrappedError --------- Signed-off-by: Pablo Chacin <[email protected]>
grafana · Nov 27, 2024 · 25b08c2 · 25b08c2
1 parent 604fca1
commit 25b08c2
Show file tree

Hide file tree

Showing 12 changed files with 176 additions and 80 deletions.
diff --git a/error.go b/error.go
@@ -6,33 +6,33 @@ import (
 	"fmt"
 )
 
-// ErrReasonUnknown signals the reason for an APIError in unknown
+// ErrReasonUnknown signals the reason for an WrappedError is unknown
 var ErrReasonUnknown = errors.New("reason unknown")
 
-// Error represents an error returned by the build service
+// WrappedError represents an error returned by the build service
 // This custom error type facilitates extracting the reason of an error
 // by using errors.Unwrap method.
 // It also facilitates checking an error (or its reason) using errors.Is by
 // comparing the error and its reason.
 // This custom type has the following known limitations:
-// - A nil Error 'e' will not satisfy errors.Is(e, nil)
+// - A nil WrappedError 'e' will not satisfy errors.Is(e, nil)
 // - Is method will not
-type Error struct {
+type WrappedError struct {
 	Err    error `json:"error,omitempty"`
 	Reason error `json:"reason,omitempty"`
 }
 
 // Error returns the Error as a string
-func (e *Error) Error() string {
+func (e *WrappedError) Error() string {
 	return fmt.Sprintf("%s: %s", e.Err, e.Reason)
 }
 
-// Is returns true if the target error is the same as the Error or its reason
+// Is returns true if the target error is the same as the WrappedError or its reason
 // It attempts several strategies:
 // - compare error and reason to target's Error()
-// - unwrap the error and reason and compare to target's Error
+// - unwrap the error and reason and compare to target's WrappedError
 // - unwrap target and compares to the error recursively
-func (e *Error) Is(target error) bool {
+func (e *WrappedError) Is(target error) bool {
 	if target == nil {
 		return false
 	}
@@ -56,46 +56,76 @@ func (e *Error) Is(target error) bool {
 	return e.Is(errors.Unwrap(target))
 }
 
-// Unwrap returns the underlying reason for the Error
-func (e *Error) Unwrap() error {
+// Unwrap returns the underlying reason for the WrappedError
+func (e *WrappedError) Unwrap() error {
 	return e.Reason
 }
 
-// MarshalJSON implements the json.Marshaler interface for the Error type
-func (e *Error) MarshalJSON() ([]byte, error) {
-	return json.Marshal(&struct {
-		Err    string `json:"error,omitempty"`
-		Reason string `json:"reason,omitempty"`
-	}{
-		Err:    e.Err.Error(),
-		Reason: e.Reason.Error(),
-	})
+type jsonError struct {
+	Err    string     `json:"error,omitempty"`
+	Reason *jsonError `json:"reason,omitempty"`
 }
 
-// UnmarshalJSON implements the json.Unmarshaler interface for the Error type
-func (e *Error) UnmarshalJSON(data []byte) error {
-	val := struct {
-		Err    string `json:"error,omitempty"`
-		Reason string `json:"reason,omitempty"`
-	}{}
+func wrap(e *jsonError) error {
+	if e == nil {
+		return nil
+	}
+	err := errors.New(e.Err)
+	if e.Reason == nil {
+		return err
+	}
+
+	return NewWrappedError(err, wrap(e.Reason))
+}
+
+func unwrap(e error) *jsonError {
+	if e == nil {
+		return nil
+	}
+
+	err, ok := AsError(e)
+	if !ok {
+		return &jsonError{Err: e.Error()}
+	}
+
+	return &jsonError{Err: err.Err.Error(), Reason: unwrap(errors.Unwrap(err))}
+}
+
+// MarshalJSON implements the json.Marshaler interface for the WrappedError type
+func (e *WrappedError) MarshalJSON() ([]byte, error) {
+	return json.Marshal(unwrap(e))
+}
+
+// UnmarshalJSON implements the json.Unmarshaler interface for the WrappedError type
+func (e *WrappedError) UnmarshalJSON(data []byte) error {
+	val := jsonError{}
 
 	if err := json.Unmarshal(data, &val); err != nil {
 		return err
 	}
 
 	e.Err = errors.New(val.Err)
-	e.Reason = errors.New(val.Reason)
+	e.Reason = wrap(val.Reason)
 	return nil
 }
 
-// NewError creates an Error from an error and a reason
+// NewWrappedError creates an Error from an error and a reason
 // If the reason is nil, ErrReasonUnknown is used
-func NewError(err error, reason error) *Error {
+func NewWrappedError(err error, reason error) *WrappedError {
 	if reason == nil {
 		reason = ErrReasonUnknown
 	}
-	return &Error{
+	return &WrappedError{
 		Err:    err,
 		Reason: reason,
 	}
 }
+
+// AsError returns an error as an Error, if possible
+func AsError(e error) (*WrappedError, bool) {
+	err := &WrappedError{}
+	if !errors.As(e, &err) {
+		return nil, false
+	}
+	return err, true
+}
diff --git a/error_test.go b/error_test.go
@@ -1,12 +1,15 @@
 package k6build
 
 import (
+	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
+	"reflect"
 	"testing"
 )
 
-func Test_Error(t *testing.T) {
+func Test_WrappedError(t *testing.T) {
 	t.Parallel()
 
 	var (
@@ -68,7 +71,7 @@ func Test_Error(t *testing.T) {
 		t.Run(tc.title, func(t *testing.T) {
 			t.Parallel()
 
-			err := NewError(tc.err, tc.reason)
+			err := NewWrappedError(tc.err, tc.reason)
 			for _, expected := range tc.expect {
 				if !errors.Is(err, expected) {
 					t.Fatalf("expected %v got %v", expected, err)
@@ -77,3 +80,60 @@ func Test_Error(t *testing.T) {
 		})
 	}
 }
+
+func Test_JsonSerialization(t *testing.T) {
+	t.Parallel()
+
+	var (
+		err    = errors.New("error")
+		reason = errors.New("reason")
+		root   = errors.New("root")
+	)
+
+	testCases := []struct {
+		title  string
+		err    *WrappedError
+		expect []byte
+	}{
+		{
+			title:  "error with cause",
+			err:    NewWrappedError(err, reason),
+			expect: []byte(`{"error":"error","reason":{"error":"reason"}}`),
+		},
+		{
+			title:  "error with nested causes",
+			err:    NewWrappedError(err, NewWrappedError(reason, root)),
+			expect: []byte(`{"error":"error","reason":{"error":"reason","reason":{"error":"root"}}}`),
+		},
+		{
+			title:  "error with nil cause",
+			err:    NewWrappedError(err, nil),
+			expect: []byte(`{"error":"error","reason":{"error":"reason unknown"}}`),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.title, func(t *testing.T) {
+			t.Parallel()
+
+			marshalled, err := json.Marshal(tc.err)
+			if err != nil {
+				t.Fatalf("error marshaling: %v", err)
+			}
+
+			if !bytes.Equal(marshalled, tc.expect) {
+				t.Fatalf("failed unmarshaling expected %v got %v", string(tc.expect), string(marshalled))
+			}
+
+			unmashalled := &WrappedError{}
+			err = json.Unmarshal(marshalled, unmashalled)
+			if err != nil {
+				t.Fatalf("error unmashaling: %v", err)
+			}
+
+			if !reflect.DeepEqual(tc.err, unmashalled) {
+				t.Fatalf("failed marshaling expected %v got %v", tc.err, unmashalled)
+			}
+		})
+	}
+}
diff --git a/pkg/api/api.go b/pkg/api/api.go
@@ -42,7 +42,7 @@ type BuildResponse struct {
 	// If not empty an error occurred processing the request
 	// This Error can be compared to the errors defined in this package using errors.Is
 	// to know the type of error, and use Unwrap to obtain its cause if available.
-	Error *k6build.Error `json:"error,omitempty"`
+	Error *k6build.WrappedError `json:"error,omitempty"`
 	// Artifact metadata. If an error occurred, content is undefined
 	Artifact k6build.Artifact `json:"artifact,omitempty"`
 }
diff --git a/pkg/cache/api/api.go b/pkg/cache/api/api.go
@@ -20,6 +20,6 @@ var (
 
 // CacheResponse is the response to a cache server request
 type CacheResponse struct {
-	Error  *k6build.Error
+	Error  *k6build.WrappedError
 	Object cache.Object
 }
diff --git a/pkg/cache/client/client.go b/pkg/cache/client/client.go
@@ -31,7 +31,7 @@ type CacheClient struct {
 // NewCacheClient returns a client for a cache server
 func NewCacheClient(config CacheClientConfig) (*CacheClient, error) {
 	if _, err := url.Parse(config.Server); err != nil {
-		return nil, k6build.NewError(ErrInvalidConfig, err)
+		return nil, k6build.NewWrappedError(ErrInvalidConfig, err)
 	}
 
 	return &CacheClient{
@@ -46,7 +46,7 @@ func (c *CacheClient) Get(_ context.Context, id string) (cache.Object, error) {
 	// TODO: use http.Request
 	resp, err := http.Get(url) //nolint:gosec,noctx
 	if err != nil {
-		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, err)
+		return cache.Object{}, k6build.NewWrappedError(api.ErrRequestFailed, err)
 	}
 	defer func() {
 		_ = resp.Body.Close()
@@ -56,13 +56,13 @@ func (c *CacheClient) Get(_ context.Context, id string) (cache.Object, error) {
 		if resp.StatusCode == http.StatusNotFound {
 			return cache.Object{}, cache.ErrObjectNotFound
 		}
-		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
+		return cache.Object{}, k6build.NewWrappedError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
 	}
 
 	cacheResponse := api.CacheResponse{}
 	err = json.NewDecoder(resp.Body).Decode(&cacheResponse)
 	if err != nil {
-		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, err)
+		return cache.Object{}, k6build.NewWrappedError(api.ErrRequestFailed, err)
 	}
 
 	if cacheResponse.Error != nil {
@@ -81,19 +81,19 @@ func (c *CacheClient) Store(_ context.Context, id string, content io.Reader) (ca
 		content,
 	)
 	if err != nil {
-		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, err)
+		return cache.Object{}, k6build.NewWrappedError(api.ErrRequestFailed, err)
 	}
 	defer func() {
 		_ = resp.Body.Close()
 	}()
 
 	if resp.StatusCode != http.StatusOK {
-		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
+		return cache.Object{}, k6build.NewWrappedError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
 	}
 	cacheResponse := api.CacheResponse{}
 	err = json.NewDecoder(resp.Body).Decode(&cacheResponse)
 	if err != nil {
-		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, err)
+		return cache.Object{}, k6build.NewWrappedError(api.ErrRequestFailed, err)
 	}
 
 	if cacheResponse.Error != nil {
@@ -107,11 +107,11 @@ func (c *CacheClient) Store(_ context.Context, id string, content io.Reader) (ca
 func (c *CacheClient) Download(_ context.Context, object cache.Object) (io.ReadCloser, error) {
 	resp, err := http.Get(object.URL) //nolint:noctx,bodyclose
 	if err != nil {
-		return nil, k6build.NewError(api.ErrRequestFailed, err)
+		return nil, k6build.NewWrappedError(api.ErrRequestFailed, err)
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, k6build.NewError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
+		return nil, k6build.NewWrappedError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
 	}
 
 	return resp.Request.Body, nil

diff --git a/pkg/cache/client/client_test.go b/pkg/cache/client/client_test.go
@@ -73,7 +73,7 @@ func TestCacheClientGet(t *testing.T) {
 			title:  "error accessing object",
 			status: http.StatusInternalServerError,
 			resp: &api.CacheResponse{
-				Error:  k6build.NewError(cache.ErrAccessingObject, k6build.ErrReasonUnknown),
+				Error:  k6build.NewWrappedError(cache.ErrAccessingObject, k6build.ErrReasonUnknown),
 				Object: cache.Object{},
 			},
 			expectErr: api.ErrRequestFailed,
@@ -121,7 +121,7 @@ func TestCacheClientStore(t *testing.T) {
 			title:  "error creating object",
 			status: http.StatusInternalServerError,
 			resp: &api.CacheResponse{
-				Error:  k6build.NewError(cache.ErrCreatingObject, k6build.ErrReasonUnknown),
+				Error:  k6build.NewWrappedError(cache.ErrCreatingObject, k6build.ErrReasonUnknown),
 				Object: cache.Object{},
 			},
 			expectErr: api.ErrRequestFailed,