1890 - Update error encoding (absmach#1891)

* update error encoding Signed-off-by: ianmuchyri <[email protected]> * fix semaphore fail Signed-off-by: ianmuchyri <[email protected]> * update encode error Signed-off-by: ianmuchyri <[email protected]> * update bootstraptests Signed-off-by: ianmuchyri <[email protected]> * Update notifiers endpoint_test Signed-off-by: ianmuchyri <[email protected]> * Update tokens_test Signed-off-by: ianmuchyri <[email protected]> * Update json.unmarshal into expected struct Signed-off-by: ianmuchyri <[email protected]> * update .env Signed-off-by: ianmuchyri <[email protected]> * update sdk_error to check if err is empty Signed-off-by: ianmuchyri <[email protected]> * update message_test Signed-off-by: ianmuchyri <[email protected]> * Update error marshaling Signed-off-by: ianmuchyri <[email protected]> * update tests Signed-off-by: ianmuchyri <[email protected]> * Redo makefile Signed-off-by: ianmuchyri <[email protected]> * Add fullstop to comments Signed-off-by: ianmuchyri <[email protected]> * Update rebase error Signed-off-by: ianmuchyri <[email protected]> --------- Signed-off-by: ianmuchyri <[email protected]>
AryanGodara · Aug 24, 2023 · 5fa2bf4 · 5fa2bf4
1 parent 822a607
commit 5fa2bf4
Show file tree

Hide file tree

Showing 17 changed files with 82 additions and 103 deletions.
diff --git a/bootstrap/api/endpoint_test.go b/bootstrap/api/endpoint_test.go
@@ -88,11 +88,11 @@ var (
 		CACert:     "newca",
 	}
 
-	missingIDRes  = toJSON(apiutil.ErrorRes{Err: errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingID).Error()})
-	missingKeyRes = toJSON(apiutil.ErrorRes{Err: errors.Wrap(apiutil.ErrValidation, apiutil.ErrBearerKey).Error()})
-	bsErrorRes    = toJSON(apiutil.ErrorRes{Err: errors.Wrap(bootstrap.ErrBootstrap, errors.ErrNotFound).Error()})
-	extKeyRes     = toJSON(apiutil.ErrorRes{Err: bootstrap.ErrExternalKey.Error()})
-	extSecKeyRes  = toJSON(apiutil.ErrorRes{Err: errors.Wrap(bootstrap.ErrExternalKeySecure, errors.New("encoding/hex: invalid byte: U+0078 'x'")).Error()})
+	missingIDRes  = toJSON(apiutil.ErrorRes{Err: apiutil.ErrMissingID.Error(), Msg: apiutil.ErrValidation.Error()})
+	missingKeyRes = toJSON(apiutil.ErrorRes{Err: apiutil.ErrBearerKey.Error(), Msg: apiutil.ErrValidation.Error()})
+	bsErrorRes    = toJSON(apiutil.ErrorRes{Err: errors.ErrNotFound.Error(), Msg: bootstrap.ErrBootstrap.Error()})
+	extKeyRes     = toJSON(apiutil.ErrorRes{Msg: bootstrap.ErrExternalKey.Error()})
+	extSecKeyRes  = toJSON(apiutil.ErrorRes{Err: "encoding/hex: invalid byte: U+0078 'x'", Msg: bootstrap.ErrExternalKeySecure.Error()})
 )
 
 type testRequest struct {

diff --git a/bootstrap/api/transport.go b/bootstrap/api/transport.go
@@ -6,7 +6,6 @@ package api
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net/http"
 	"net/url"
 	"strings"
@@ -303,13 +302,7 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) {
 
 	if errorVal, ok := err.(errors.Error); ok {
 		w.Header().Set("Content-Type", contentType)
-
-		errMsg := errorVal.Msg()
-		if errorVal.Err() != nil {
-			errMsg = fmt.Sprintf("%s : %s", errMsg, errorVal.Err().Msg())
-		}
-
-		if err := json.NewEncoder(w).Encode(apiutil.ErrorRes{Err: errMsg}); err != nil {
+		if err := json.NewEncoder(w).Encode(errorVal); err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}

diff --git a/certs/api/transport.go b/certs/api/transport.go
@@ -6,7 +6,6 @@ package api
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net/http"
 
 	kithttp "github.com/go-kit/kit/transport/http"
@@ -173,13 +172,7 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) {
 
 	if errorVal, ok := err.(errors.Error); ok {
 		w.Header().Set("Content-Type", contentType)
-
-		errMsg := errorVal.Msg()
-		if errorVal.Err() != nil {
-			errMsg = fmt.Sprintf("%s : %s", errMsg, errorVal.Err().Msg())
-		}
-
-		if err := json.NewEncoder(w).Encode(apiutil.ErrorRes{Err: errMsg}); err != nil {
+		if err := json.NewEncoder(w).Encode(errorVal); err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}

diff --git a/consumers/notifiers/api/endpoint_test.go b/consumers/notifiers/api/endpoint_test.go
@@ -35,10 +35,10 @@ const (
 )
 
 var (
-	notFoundRes   = toJSON(apiutil.ErrorRes{Err: errors.ErrNotFound.Error()})
-	unauthRes     = toJSON(apiutil.ErrorRes{Err: errors.ErrAuthentication.Error()})
-	invalidRes    = toJSON(apiutil.ErrorRes{Err: errors.Wrap(apiutil.ErrValidation, apiutil.ErrInvalidQueryParams).Error()})
-	missingTokRes = toJSON(apiutil.ErrorRes{Err: errors.Wrap(apiutil.ErrValidation, apiutil.ErrBearerToken).Error()})
+	notFoundRes   = toJSON(apiutil.ErrorRes{Msg: errors.ErrNotFound.Error()})
+	unauthRes     = toJSON(apiutil.ErrorRes{Msg: errors.ErrAuthentication.Error()})
+	invalidRes    = toJSON(apiutil.ErrorRes{Err: apiutil.ErrInvalidQueryParams.Error(), Msg: apiutil.ErrValidation.Error()})
+	missingTokRes = toJSON(apiutil.ErrorRes{Err: apiutil.ErrBearerToken.Error(), Msg: apiutil.ErrValidation.Error()})
 )
 
 type testRequest struct {

diff --git a/consumers/notifiers/api/transport.go b/consumers/notifiers/api/transport.go
@@ -6,7 +6,6 @@ package api
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net/http"
 	"strings"
 
@@ -176,13 +175,7 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) {
 
 	if errorVal, ok := err.(errors.Error); ok {
 		w.Header().Set("Content-Type", contentType)
-
-		errMsg := errorVal.Msg()
-		if errorVal.Err() != nil {
-			errMsg = fmt.Sprintf("%s : %s", errMsg, errorVal.Err().Msg())
-		}
-
-		if err := json.NewEncoder(w).Encode(apiutil.ErrorRes{Err: errMsg}); err != nil {
+		if err := json.NewEncoder(w).Encode(errorVal); err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}

diff --git a/http/api/transport.go b/http/api/transport.go
@@ -6,7 +6,6 @@ package api
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -190,13 +189,7 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) {
 
 	if errorVal, ok := err.(errors.Error); ok {
 		w.Header().Set("Content-Type", contentType)
-
-		errMsg := errorVal.Msg()
-		if errorVal.Err() != nil {
-			errMsg = fmt.Sprintf("%s : %s", errMsg, errorVal.Err().Msg())
-		}
-
-		if err := json.NewEncoder(w).Encode(apiutil.ErrorRes{Err: errMsg}); err != nil {
+		if err := json.NewEncoder(w).Encode(errorVal); err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}

diff --git a/internal/api/common.go b/internal/api/common.go
@@ -6,7 +6,6 @@ package api
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net/http"
 
 	"github.com/gofrs/uuid"
@@ -130,11 +129,7 @@ func EncodeError(_ context.Context, err error, w http.ResponseWriter) {
 	}
 
 	if errorVal, ok := err.(errors.Error); ok {
-		errMsg := errorVal.Msg()
-		if errorVal.Err() != nil {
-			errMsg = fmt.Sprintf("%s : %s", errMsg, errorVal.Err().Msg())
-		}
-		if err := json.NewEncoder(w).Encode(apiutil.ErrorRes{Err: errMsg}); err != nil {
+		if err := json.NewEncoder(w).Encode(errorVal); err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}

diff --git a/internal/apiutil/responses.go b/internal/apiutil/responses.go
@@ -6,4 +6,5 @@ package apiutil
 // ErrorRes represents the HTTP error response body.
 type ErrorRes struct {
 	Err string `json:"error"`
+	Msg string `json:"message"`
 }
diff --git a/opcua/api/transport.go b/opcua/api/transport.go
@@ -6,7 +6,6 @@ package api
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net/http"
 
 	kithttp "github.com/go-kit/kit/transport/http"
@@ -119,13 +118,7 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) {
 
 	if errorVal, ok := err.(errors.Error); ok {
 		w.Header().Set("Content-Type", contentType)
-
-		errMsg := errorVal.Msg()
-		if errorVal.Err() != nil {
-			errMsg = fmt.Sprintf("%s : %s", errMsg, errorVal.Err().Msg())
-		}
-
-		if err := json.NewEncoder(w).Encode(apiutil.ErrorRes{Err: errMsg}); err != nil {
+		if err := json.NewEncoder(w).Encode(errorVal); err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}

diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go
@@ -5,6 +5,7 @@ package errors
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"os"
 	"os/signal"
@@ -16,11 +17,14 @@ type Error interface {
 	// Error implements the error interface.
 	Error() string
 
-	// Msg returns error message
+	// Msg returns error message.
 	Msg() string
 
-	// Err returns wrapped error
+	// Err returns wrapped error.
 	Err() Error
+
+	// MarshalJSON returns a marshaled error.
+	MarshalJSON() ([]byte, error)
 }
 
 var _ Error = (*customError)(nil)
@@ -49,6 +53,20 @@ func (ce *customError) Err() Error {
 	return ce.err
 }
 
+func (ce *customError) MarshalJSON() ([]byte, error) {
+	var val string
+	if e := ce.Err(); e != nil {
+		val = e.Msg()
+	}
+	return json.Marshal(&struct {
+		Err string `json:"error"`
+		Msg string `json:"message"`
+	}{
+		Err: val,
+		Msg: ce.Msg(),
+	})
+}
+
 // Contains inspects if e2 error is contained in any layer of e1 error.
 func Contains(e1 error, e2 error) bool {
 	if e1 == nil || e2 == nil {

diff --git a/pkg/errors/sdk_errors.go b/pkg/errors/sdk_errors.go
@@ -10,7 +10,10 @@ import (
 	"net/http"
 )
 
-const errorKey = "error"
+type errorRes struct {
+	Err string `json:"error"`
+	Msg string `json:"message"`
+}
 
 // Failed to read response body.
 var errRespBody = New("failed to read response body")
@@ -44,6 +47,15 @@ func (ce *sdkError) StatusCode() int {
 
 // NewSDKError returns an SDK Error that formats as the given text.
 func NewSDKError(err error) SDKError {
+	if e, ok := err.(Error); ok {
+		return &sdkError{
+			statusCode: 0,
+			customError: &customError{
+				msg: e.Msg(),
+				err: cast(e.Err()),
+			},
+		}
+	}
 	return &sdkError{
 		customError: &customError{
 			msg: err.Error(),
@@ -55,6 +67,15 @@ func NewSDKError(err error) SDKError {
 
 // NewSDKErrorWithStatus returns an SDK Error setting the status code.
 func NewSDKErrorWithStatus(err error, statusCode int) SDKError {
+	if e, ok := err.(Error); ok {
+		return &sdkError{
+			statusCode: statusCode,
+			customError: &customError{
+				msg: e.Msg(),
+				err: cast(e.Err()),
+			},
+		}
+	}
 	return &sdkError{
 		statusCode: statusCode,
 		customError: &customError{
@@ -78,15 +99,13 @@ func CheckError(resp *http.Response, expectedStatusCodes ...int) SDKError {
 	if err != nil {
 		return NewSDKErrorWithStatus(Wrap(errRespBody, err), resp.StatusCode)
 	}
-	var content map[string]interface{}
-	_ = json.Unmarshal(body, &content)
-
-	if msg, ok := content[errorKey]; ok {
-		if v, ok := msg.(string); ok {
-			return NewSDKErrorWithStatus(New(v), resp.StatusCode)
-		}
-		return NewSDKErrorWithStatus(fmt.Errorf("%v", msg), resp.StatusCode)
+	var content errorRes
+	if err := json.Unmarshal(body, &content); err != nil {
+		return NewSDKErrorWithStatus(err, resp.StatusCode)
+	}
+	if content.Err == "" {
+		return NewSDKErrorWithStatus(New(content.Msg), resp.StatusCode)
 	}
 
-	return NewSDKErrorWithStatus(New(string(body)), resp.StatusCode)
+	return NewSDKErrorWithStatus(Wrap(New(content.Msg), New(content.Err)), resp.StatusCode)
 }
diff --git a/pkg/sdk/go/message_test.go b/pkg/sdk/go/message_test.go
@@ -19,6 +19,8 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+var unexpectedJSONEnd = errors.New("unexpected end of JSON input")
+
 func newMessageService(cc policies.AuthServiceClient) adapter.Service {
 	pub := mocks.NewPublisher()
 
@@ -70,7 +72,7 @@ func TestSendMessage(t *testing.T) {
 			chanID: chanID,
 			msg:    msg,
 			auth:   invalidToken,
-			err:    errors.NewSDKErrorWithStatus(errors.New(""), http.StatusUnauthorized),
+			err:    errors.NewSDKErrorWithStatus(unexpectedJSONEnd, http.StatusUnauthorized),
 		},
 		"publish message with wrong content type": {
 			chanID: chanID,
@@ -88,17 +90,19 @@ func TestSendMessage(t *testing.T) {
 			chanID: chanID,
 			msg:    msg,
 			auth:   "invalid-token",
-			err:    errors.NewSDKErrorWithStatus(errors.New(""), http.StatusUnauthorized),
+			err:    errors.NewSDKErrorWithStatus(unexpectedJSONEnd, http.StatusUnauthorized),
 		},
 	}
 	for desc, tc := range cases {
 		err := mfsdk.SendMessage(tc.chanID, tc.msg, tc.auth)
-		if tc.err == nil {
+		switch tc.err {
+		case nil:
 			assert.Nil(t, err, fmt.Sprintf("%s: got unexpected error: %s", desc, err))
-		} else {
-			assert.Equal(t, tc.err.Error(), err.Error(), fmt.Sprintf("%s: expected error %s, got %s", desc, err, tc.err))
+		default:
+			assert.Equal(t, tc.err.Error(), err.Error(), fmt.Sprintf("%s: expected error %s, got %s", desc, tc.err, err))
 		}
 	}
+
 }
 
 func TestSetContentType(t *testing.T) {

diff --git a/pkg/sdk/go/tokens_test.go b/pkg/sdk/go/tokens_test.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"net/http"
-	"strings"
 	"testing"
 
 	"github.com/mainflux/mainflux/internal/apiutil"
@@ -61,18 +60,18 @@ func TestIssueToken(t *testing.T) {
 		{
 			desc:   "issue token for an empty user",
 			client: sdk.User{},
-			err:    errors.NewSDKErrorWithStatus(apiutil.ErrMissingIdentity, http.StatusBadRequest),
+			err:    errors.NewSDKErrorWithStatus(errors.Wrap(apiutil.ErrValidation, apiutil.ErrMissingIdentity), http.StatusInternalServerError),
 		},
 		{
-			desc: "issue token for invalid secret",
+			desc: "issue token for invalid identity",
 			client: sdk.User{
 				Credentials: sdk.Credentials{
 					Identity: "invalid",
 					Secret:   "secret",
 				},
 			},
 			dbClient: wrongClient,
-			err:      errors.NewSDKErrorWithStatus(errors.Wrap(errors.ErrAuthentication, apiutil.ErrValidation), http.StatusUnauthorized),
+			err:      errors.NewSDKErrorWithStatus(errors.ErrAuthentication, http.StatusUnauthorized),
 		},
 	}
 	for _, tc := range cases {
@@ -84,7 +83,7 @@ func TestIssueToken(t *testing.T) {
 			ok := repoCall.Parent.AssertCalled(t, "RetrieveByIdentity", mock.Anything, mock.Anything)
 			assert.True(t, ok, fmt.Sprintf("RetrieveByIdentity was not called on %s", tc.desc))
 		default:
-			assert.True(t, strings.Contains(err.Msg(), tc.err.Msg()), fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err))
+			assert.Equal(t, tc.err, err, fmt.Sprintf("%s: expected error %s, got %s", tc.desc, tc.err, err))
 		}
 		repoCall.Unset()
 	}

diff --git a/provision/api/transport.go b/provision/api/transport.go
@@ -3,7 +3,6 @@ package api
 import (
 	"context"
 	"encoding/json"
-	"fmt"
 	"net/http"
 
 	kithttp "github.com/go-kit/kit/transport/http"
@@ -117,13 +116,7 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) {
 
 	if errorVal, ok := err.(errors.Error); ok {
 		w.Header().Set("Content-Type", contentType)
-
-		errMsg := errorVal.Msg()
-		if errorVal.Err() != nil {
-			errMsg = fmt.Sprintf("%s : %s", errMsg, errorVal.Err().Msg())
-		}
-
-		if err := json.NewEncoder(w).Encode(apiutil.ErrorRes{Err: errMsg}); err != nil {
+		if err := json.NewEncoder(w).Encode(errorVal); err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 		}
 	}