Enhance Portainer API error handling

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)

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
+}

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))
+		})
+	}
+}

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
-}

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)
-		})
-	}
-}