From 5fa2bf4229c5ec2d7c295421ecc8fdf5bf548047 Mon Sep 17 00:00:00 2001 From: Ian Ngethe Muchiri <100555904+ianmuchyri@users.noreply.github.com> Date: Thu, 24 Aug 2023 16:09:23 +0300 Subject: [PATCH] 1890 - Update error encoding (#1891) * update error encoding Signed-off-by: ianmuchyri * fix semaphore fail Signed-off-by: ianmuchyri * update encode error Signed-off-by: ianmuchyri * update bootstraptests Signed-off-by: ianmuchyri * Update notifiers endpoint_test Signed-off-by: ianmuchyri * Update tokens_test Signed-off-by: ianmuchyri * Update json.unmarshal into expected struct Signed-off-by: ianmuchyri * update .env Signed-off-by: ianmuchyri * update sdk_error to check if err is empty Signed-off-by: ianmuchyri * update message_test Signed-off-by: ianmuchyri * Update error marshaling Signed-off-by: ianmuchyri * update tests Signed-off-by: ianmuchyri * Redo makefile Signed-off-by: ianmuchyri * Add fullstop to comments Signed-off-by: ianmuchyri * Update rebase error Signed-off-by: ianmuchyri --------- Signed-off-by: ianmuchyri --- bootstrap/api/endpoint_test.go | 10 +++--- bootstrap/api/transport.go | 9 +----- certs/api/transport.go | 9 +----- consumers/notifiers/api/endpoint_test.go | 8 ++--- consumers/notifiers/api/transport.go | 9 +----- http/api/transport.go | 9 +----- internal/api/common.go | 7 +---- internal/apiutil/responses.go | 1 + opcua/api/transport.go | 9 +----- pkg/errors/errors.go | 22 +++++++++++-- pkg/errors/sdk_errors.go | 39 ++++++++++++++++++------ pkg/sdk/go/message_test.go | 14 ++++++--- pkg/sdk/go/tokens_test.go | 9 +++--- provision/api/transport.go | 9 +----- readers/api/transport.go | 10 +----- twins/api/http/transport.go | 9 +----- users/clients/service.go | 2 +- 17 files changed, 82 insertions(+), 103 deletions(-) diff --git a/bootstrap/api/endpoint_test.go b/bootstrap/api/endpoint_test.go index 89bbd8f9..13a8126b 100644 --- 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 index a2643865..5a6cce4e 100644 --- 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 index 4ec0fb43..cedd46d5 100644 --- 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 index 918d22e6..f9d4e888 100644 --- 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 index 6aee5ce2..d3128d63 100644 --- 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 index 7d0d11d4..14596519 100644 --- 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 index 4bcc1b8a..3b63c9ea 100644 --- 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 index 12bc8be2..e531273a 100644 --- 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 index f64ef876..7f000fd9 100644 --- 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 index ed70792d..47e0ff05 100644 --- 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 index ac8b6ad5..42d9d9df 100644 --- 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 index 0d810270..9866b9f7 100644 --- 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 index 1ff25d88..7139b348 100644 --- 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,10 +60,10 @@ 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", @@ -72,7 +71,7 @@ func TestIssueToken(t *testing.T) { }, }, 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 index 9855becf..019ee518 100644 --- 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) } } diff --git a/readers/api/transport.go b/readers/api/transport.go index 2025364c..55aaecbf 100644 --- a/readers/api/transport.go +++ b/readers/api/transport.go @@ -6,7 +6,6 @@ package api import ( "context" "encoding/json" - "fmt" "net/http" kithttp "github.com/go-kit/kit/transport/http" @@ -209,16 +208,9 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) { if wrapper != nil { err = errors.Wrap(wrapper, err) } - 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/twins/api/http/transport.go b/twins/api/http/transport.go index 50195f02..ba89b22b 100644 --- a/twins/api/http/transport.go +++ b/twins/api/http/transport.go @@ -6,7 +6,6 @@ package http import ( "context" "encoding/json" - "fmt" "net/http" "strings" @@ -236,13 +235,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/users/clients/service.go b/users/clients/service.go index da904c0c..d7ffed55 100644 --- a/users/clients/service.go +++ b/users/clients/service.go @@ -112,7 +112,7 @@ func (svc service) RegisterClient(ctx context.Context, token string, cli mfclien func (svc service) IssueToken(ctx context.Context, identity, secret string) (jwt.Token, error) { dbUser, err := svc.clients.RetrieveByIdentity(ctx, identity) if err != nil { - return jwt.Token{}, errors.Wrap(errors.ErrAuthentication, err) + return jwt.Token{}, err } if err := svc.hasher.Compare(secret, dbUser.Credentials.Secret); err != nil { return jwt.Token{}, errors.Wrap(errors.ErrLogin, err)