From eaf7d2e5cfbd72d87a6605f80697c4d53919302c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Carlos=20Mej=C3=ADas=20Rodr=C3=ADguez?= Date: Mon, 14 Oct 2019 01:52:03 -0400 Subject: [PATCH] Enhance Portainer API error handling --- client/client.go | 8 +-- client/errors.go | 61 +++++++++++++++- client/errors_test.go | 161 ++++++++++++++++++++++++++++++++++++++++++ client/utils.go | 27 ------- client/utils_test.go | 51 ------------- 5 files changed, 223 insertions(+), 85 deletions(-) diff --git a/client/client.go b/client/client.go index de1cdb5..6dd8d0d 100644 --- a/client/client.go +++ b/client/client.go @@ -125,6 +125,9 @@ func (n *portainerClientImp) do(uri, method string, requestBody io.Reader, heade } } + // Check for HTTP error status codes + err = getResponseHTTPError(resp) + return } @@ -164,11 +167,6 @@ func (n *portainerClientImp) doJSON(uri, method string, headers http.Header, req return err } - err = checkResponseForErrors(resp) - if err != nil { - return err - } - // Decode response body, if any if responseBody != nil { d := json.NewDecoder(resp.Body) diff --git a/client/errors.go b/client/errors.go index 6a77642..876415a 100644 --- a/client/errors.go +++ b/client/errors.go @@ -1,16 +1,73 @@ package client -import "fmt" +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "io/ioutil" + "net/http" +) // GenericError represents the body of a generic error returned by the Portainer API type GenericError struct { + Code int Err string Details string } -func (e *GenericError) Error() string { +func (e GenericError) Error() string { if e.Details != "" { return fmt.Sprintf("%s: %s", e.Err, e.Details) } return fmt.Sprintf("%s", e.Err) } + +// Get an http.Response's error (if any) +func getResponseHTTPError(resp *http.Response) error { + if resp.StatusCode < 300 { + // There is no error + return nil + } + + switch resp.StatusCode { + // Error codes found in the Portainer API 1.22.0 Swagger spec + case http.StatusBadRequest, http.StatusForbidden, http.StatusNotFound, http.StatusConflict, http.StatusInternalServerError, http.StatusServiceUnavailable: + // Guess it's a GenericError + genericError, err := getResponseGenericHTTPError(resp) + if err != nil { + // It's not a GenericError + return getResponseNonGenericHTTPError(resp) + } + return &genericError + default: + return getResponseNonGenericHTTPError(resp) + } +} + +func getResponseGenericHTTPError(resp *http.Response) (genericError GenericError, err error) { + genericError = GenericError{ + Code: resp.StatusCode, + } + err = json.NewDecoder(resp.Body).Decode(&genericError) + return +} + +func getResponseNonGenericHTTPError(resp *http.Response) error { + bodyString, err := getResponseBodyAsString(resp) + if err != nil { + return err + } + return errors.New(bodyString) +} + +func getResponseBodyAsString(resp *http.Response) (bodyString string, err error) { + bodyBytes, err := ioutil.ReadAll(resp.Body) + defer resp.Body.Close() + if err != nil { + return + } + bodyString = string(bodyBytes) + resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) + return +} diff --git a/client/errors_test.go b/client/errors_test.go index a0ebaef..f5b0868 100644 --- a/client/errors_test.go +++ b/client/errors_test.go @@ -1,6 +1,11 @@ package client import ( + "bytes" + "encoding/json" + "errors" + "io/ioutil" + "net/http" "testing" "github.com/stretchr/testify/assert" @@ -54,3 +59,159 @@ func TestGenericError_Error(t *testing.T) { }) } } + +func Test_getResponseHTTPError(t *testing.T) { + type args struct { + resp *http.Response + } + tests := []struct { + name string + args args + wantErr error + }{ + { + name: "bad request (generic) error", + args: args{ + resp: func() (resp *http.Response) { + resp = &http.Response{ + StatusCode: http.StatusBadRequest, + } + bodyBytes, _ := json.Marshal(map[string]interface{}{ + "Err": "Error", + "Details": "Bad request", + }) + resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) + return + }(), + }, + wantErr: &GenericError{ + Code: http.StatusBadRequest, + Err: "Error", + Details: "Bad request", + }, + }, + { + name: "forbidden (generic) error", + args: args{ + resp: func() (resp *http.Response) { + resp = &http.Response{ + StatusCode: http.StatusForbidden, + } + bodyBytes, _ := json.Marshal(map[string]interface{}{ + "Err": "Error", + "Details": "Forbidden", + }) + resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) + return + }(), + }, + wantErr: &GenericError{ + Code: http.StatusForbidden, + Err: "Error", + Details: "Forbidden", + }, + }, + { + name: "not found (generic) error", + args: args{ + resp: func() (resp *http.Response) { + resp = &http.Response{ + StatusCode: http.StatusNotFound, + } + bodyBytes, _ := json.Marshal(map[string]interface{}{ + "Err": "Error", + "Details": "Not found", + }) + resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) + return + }(), + }, + wantErr: &GenericError{ + Code: http.StatusNotFound, + Err: "Error", + Details: "Not found", + }, + }, + { + name: "conflict (generic) error", + args: args{ + resp: func() (resp *http.Response) { + resp = &http.Response{ + StatusCode: http.StatusConflict, + } + bodyBytes, _ := json.Marshal(map[string]interface{}{ + "Err": "Error", + "Details": "Conflict", + }) + resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) + return + }(), + }, + wantErr: &GenericError{ + Code: http.StatusConflict, + Err: "Error", + Details: "Conflict", + }, + }, + { + name: "internal server error (generic) error", + args: args{ + resp: func() (resp *http.Response) { + resp = &http.Response{ + StatusCode: http.StatusInternalServerError, + } + bodyBytes, _ := json.Marshal(map[string]interface{}{ + "Err": "Error", + "Details": "Internal server error", + }) + resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) + return + }(), + }, + wantErr: &GenericError{ + Code: http.StatusInternalServerError, + Err: "Error", + Details: "Internal server error", + }, + }, + { + name: "service unavailable (generic) error", + args: args{ + resp: func() (resp *http.Response) { + resp = &http.Response{ + StatusCode: http.StatusServiceUnavailable, + } + bodyBytes, _ := json.Marshal(map[string]interface{}{ + "Err": "Error", + "Details": "Service unavailable", + }) + resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) + return + }(), + }, + wantErr: &GenericError{ + Code: http.StatusServiceUnavailable, + Err: "Error", + Details: "Service unavailable", + }, + }, + { + name: "method not allowed (non generic) error", + args: args{ + resp: func() (resp *http.Response) { + resp = &http.Response{ + StatusCode: http.StatusMethodNotAllowed, + Body: ioutil.NopCloser(bytes.NewReader([]byte("Err"))), + } + return + }(), + }, + wantErr: errors.New("Err"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantErr, getResponseHTTPError(tt.args.resp)) + }) + } +} diff --git a/client/utils.go b/client/utils.go index 173b31f..e120548 100644 --- a/client/utils.go +++ b/client/utils.go @@ -1,12 +1,6 @@ package client import ( - "bytes" - "encoding/json" - "errors" - "io/ioutil" - "net/http" - portainer "github.com/portainer/portainer/api" ) @@ -21,24 +15,3 @@ func GetTranslatedStackType(t portainer.StackType) string { return "" } } - -// Check if an http.Response object has errors -func checkResponseForErrors(resp *http.Response) error { - if 300 <= resp.StatusCode { - // Guess it's a GenericError - respBody := GenericError{} - err := json.NewDecoder(resp.Body).Decode(&respBody) - if err != nil { - // It's not a GenericError - bodyBytes, err := ioutil.ReadAll(resp.Body) - defer resp.Body.Close() - if err != nil { - return err - } - resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) - return errors.New(string(bodyBytes)) - } - return &respBody - } - return nil -} diff --git a/client/utils_test.go b/client/utils_test.go index a4f225e..4c9caff 100644 --- a/client/utils_test.go +++ b/client/utils_test.go @@ -1,10 +1,6 @@ package client import ( - "bytes" - "encoding/json" - "io/ioutil" - "net/http" "testing" portainer "github.com/portainer/portainer/api" @@ -48,50 +44,3 @@ func TestGetTranslatedStackType(t *testing.T) { }) } } - -func Test_checkResponseForErrors(t *testing.T) { - type args struct { - resp *http.Response - } - tests := []struct { - name string - args args - wantErr bool - }{ - { - name: "generic error", - args: args{ - resp: func() (resp *http.Response) { - resp = &http.Response{ - StatusCode: http.StatusNotFound, - } - bodyBytes, _ := json.Marshal(map[string]interface{}{ - "Err": "Error", - "Details": "Not found", - }) - resp.Body = ioutil.NopCloser(bytes.NewReader(bodyBytes)) - return - }(), - }, - wantErr: true, - }, - { - name: "non generic error", - args: args{ - resp: func() (resp *http.Response) { - resp = &http.Response{ - StatusCode: http.StatusNotFound, - Body: ioutil.NopCloser(bytes.NewReader([]byte("Err"))), - } - return - }(), - }, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - assert.Equal(t, tt.wantErr, checkResponseForErrors(tt.args.resp) != nil) - }) - } -}