Return error reason (#61)

* introduce custom Error
* use k6build.Error for wrapping errors
* return HTTP status ok In APIs when requests are processed but return an error

Signed-off-by: Pablo Chacin <[email protected]>

cmd/local/local.go

@@ -2,7 +2,6 @@
 package local
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"net/url"
@@ -16,8 +15,6 @@ import (
 	"github.com/spf13/cobra"
 )
 
-var ErrTargetPlatformUndefined = errors.New("target platform is required") //nolint:revive
-
 const (
 	long = `
 k6build local builder creates a custom k6 binary artifacts that satisfies certain

error.go

@@ -0,0 +1,101 @@
+package k6build
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+)
+
+// ErrReasonUnknown signals the reason for an APIError in unknown
+var ErrReasonUnknown = errors.New("reason unknown")
+
+// Error 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)
+// - Is method will not
+type Error struct {
+	Err    error `json:"error,omitempty"`
+	Reason error `json:"reason,omitempty"`
+}
+
+// Error returns the Error as a string
+func (e *Error) 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
+// It attempts several strategies:
+// - compare error and reason to target's Error()
+// - unwrap the error and reason and compare to target's Error
+// - unwrap target and compares to the error recursively
+func (e *Error) Is(target error) bool {
+	if target == nil {
+		return false
+	}
+
+	if e.Err.Error() == target.Error() {
+		return true
+	}
+
+	if e.Reason != nil && e.Reason.Error() == target.Error() {
+		return true
+	}
+
+	if u := errors.Unwrap(e.Err); u != nil && u.Error() == target.Error() {
+		return true
+	}
+
+	if u := errors.Unwrap(e.Reason); u != nil && u.Error() == target.Error() {
+		return true
+	}
+
+	return e.Is(errors.Unwrap(target))
+}
+
+// Unwrap returns the underlying reason for the Error
+func (e *Error) 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(),
+	})
+}
+
+// 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"`
+	}{}
+
+	if err := json.Unmarshal(data, &val); err != nil {
+		return err
+	}
+
+	e.Err = errors.New(val.Err)
+	e.Reason = errors.New(val.Reason)
+	return nil
+}
+
+// NewError creates an Error from an error and a reason
+// If the reason is nil, ErrReasonUnknown is used
+func NewError(err error, reason error) *Error {
+	if reason == nil {
+		reason = ErrReasonUnknown
+	}
+	return &Error{
+		Err:    err,
+		Reason: reason,
+	}
+}

error_test.go

@@ -0,0 +1,79 @@
+package k6build
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+)
+
+func Test_Error(t *testing.T) {
+	t.Parallel()
+
+	var (
+		err    = errors.New("error")
+		reason = errors.New("reason")
+	)
+
+	testCases := []struct {
+		title  string
+		err    error
+		reason error
+		expect []error
+	}{
+		{
+			title:  "error and reason",
+			err:    err,
+			reason: reason,
+			expect: []error{err, reason},
+		},
+		{
+			title:  "error not reason",
+			err:    err,
+			reason: nil,
+			expect: []error{err},
+		},
+		{
+			title:  "multiple and reasons",
+			err:    err,
+			reason: reason,
+			expect: []error{err, reason},
+		},
+		{
+			title:  "wrapped err",
+			err:    fmt.Errorf("wrapped %w", err),
+			reason: reason,
+			expect: []error{err, reason},
+		},
+		{
+			title:  "wrapped reason",
+			err:    err,
+			reason: fmt.Errorf("wrapped %w", reason),
+			expect: []error{err, reason},
+		},
+		{
+			title:  "wrapped err in target",
+			err:    err,
+			reason: reason,
+			expect: []error{fmt.Errorf("wrapped %w", err)},
+		},
+		{
+			title:  "wrapped reason in target",
+			err:    err,
+			reason: reason,
+			expect: []error{fmt.Errorf("wrapped %w", reason)},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.title, func(t *testing.T) {
+			t.Parallel()
+
+			err := NewError(tc.err, tc.reason)
+			for _, expected := range tc.expect {
+				if !errors.Is(err, expected) {
+					t.Fatalf("expected %v got %v", expected, err)
+				}
+			}
+		})
+	}
+}

pkg/api/api.go

@@ -3,11 +3,22 @@ package api
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 
 	"github.com/grafana/k6build"
 )
 
+var (
+	// ErrInvalidRequest signals the request could not be processed
+	// due to erroneous parameters
+	ErrInvalidRequest = errors.New("invalid request")
+	// ErrRequestFailed signals the request failed, probably due to a network error
+	ErrRequestFailed = errors.New("request failed")
+	// ErrBuildFailed signals the build process failed
+	ErrBuildFailed = errors.New("build failed")
+)
+
 // BuildRequest defines a request to the build service
 type BuildRequest struct {
 	K6Constrains string               `json:"k6,omitempty"`
@@ -28,6 +39,10 @@ func (r BuildRequest) String() string {
 
 // BuildResponse defines the response for a BuildRequest
 type BuildResponse struct {
-	Error    string           `json:"error,omitempty"`
+	// 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"`
+	// Artifact metadata. If an error occurred, content is undefined
 	Artifact k6build.Artifact `json:"artifact,omitempty"`
 }

pkg/cache/api/api.go

@@ -2,11 +2,24 @@
 package api
 
 import (
+	"errors"
+
+	"github.com/grafana/k6build"
 	"github.com/grafana/k6build/pkg/cache"
 )
 
+var (
+	// ErrInvalidRequest signals the request could not be processed
+	// due to erroneous parameters
+	ErrInvalidRequest = errors.New("invalid request")
+	// ErrRequestFailed signals the request failed, probably due to a network error
+	ErrRequestFailed = errors.New("request failed")
+	// ErrCacheAccess signals the access to the cache failed
+	ErrCacheAccess = errors.New("cache access failed")
+)
+
 // CacheResponse is the response to a cache server request
 type CacheResponse struct {
-	Error  string
+	Error  *k6build.Error
 	Object cache.Object
 }

pkg/cache/client/client.go

@@ -10,18 +10,13 @@ import (
 	"net/http"
 	"net/url"
 
+	"github.com/grafana/k6build"
 	"github.com/grafana/k6build/pkg/cache"
 	"github.com/grafana/k6build/pkg/cache/api"
 )
 
-var (
-	ErrAccessingServer = errors.New("making request")        //nolint:revive
-	ErrInvalidConfig   = errors.New("invalid configuration") //nolint:revive
-	ErrInvalidRequest  = errors.New("invalid request")       //nolint:revive
-	ErrInvalidResponse = errors.New("invalid response")      //nolint:revive
-	ErrRequestFailed   = errors.New("request failed")        //nolint:revive
-
-)
+// ErrInvalidConfig signals an error with the client configuration
+var ErrInvalidConfig = errors.New("invalid configuration")
 
 // CacheClientConfig defines the configuration for accessing a remote cache service
 type CacheClientConfig struct {
@@ -36,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, fmt.Errorf("%w: %w", ErrInvalidConfig, err)
+		return nil, k6build.NewError(ErrInvalidConfig, err)
 	}
 
 	return &CacheClient{
@@ -51,27 +46,27 @@ 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{}, fmt.Errorf("%w %w", ErrAccessingServer, err)
+		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, err)
 	}
 	defer func() {
 		_ = resp.Body.Close()
 	}()
 
 	if resp.StatusCode != http.StatusOK {
 		if resp.StatusCode == http.StatusNotFound {
-			return cache.Object{}, fmt.Errorf("%w with status", cache.ErrObjectNotFound)
+			return cache.Object{}, cache.ErrObjectNotFound
 		}
-		return cache.Object{}, fmt.Errorf("%w with status %s", ErrRequestFailed, resp.Status)
+		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
 	}
 
 	cacheResponse := api.CacheResponse{}
 	err = json.NewDecoder(resp.Body).Decode(&cacheResponse)
 	if err != nil {
-		return cache.Object{}, fmt.Errorf("%w: %s", ErrInvalidResponse, err.Error())
+		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, err)
 	}
 
-	if cacheResponse.Error != "" {
-		return cache.Object{}, fmt.Errorf("%w: %s", ErrRequestFailed, cacheResponse.Error)
+	if cacheResponse.Error != nil {
+		return cache.Object{}, cacheResponse.Error
 	}
 
 	return cacheResponse.Object, nil
@@ -86,23 +81,23 @@ func (c *CacheClient) Store(_ context.Context, id string, content io.Reader) (ca
 		content,
 	)
 	if err != nil {
-		return cache.Object{}, fmt.Errorf("%w %w", ErrAccessingServer, err)
+		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, err)
 	}
 	defer func() {
 		_ = resp.Body.Close()
 	}()
 
 	if resp.StatusCode != http.StatusOK {
-		return cache.Object{}, fmt.Errorf("%w with status %s", ErrRequestFailed, resp.Status)
+		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
 	}
 	cacheResponse := api.CacheResponse{}
 	err = json.NewDecoder(resp.Body).Decode(&cacheResponse)
 	if err != nil {
-		return cache.Object{}, fmt.Errorf("%w: %s", ErrInvalidResponse, err.Error())
+		return cache.Object{}, k6build.NewError(api.ErrRequestFailed, err)
 	}
 
-	if cacheResponse.Error != "" {
-		return cache.Object{}, fmt.Errorf("%w: %s", ErrRequestFailed, cacheResponse.Error)
+	if cacheResponse.Error != nil {
+		return cache.Object{}, cacheResponse.Error
 	}
 
 	return cacheResponse.Object, nil
@@ -112,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, fmt.Errorf("%w %w", ErrAccessingServer, err)
+		return nil, k6build.NewError(api.ErrRequestFailed, err)
 	}
 
 	if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("%w with status %s", ErrRequestFailed, resp.Status)
+		return nil, k6build.NewError(api.ErrRequestFailed, fmt.Errorf("status %s", resp.Status))
 	}
 
 	return resp.Request.Body, nil