feat: make error handling more verbose and collect more error information from API errors, support API V2 error format (#8)

feat: make error handling more verbose and collect more error information from Unifi Controller, support API V2 error format

README.md

@@ -177,7 +177,7 @@ user, err := c.CreateUser(ctx, "site-name", &unifi.User{
 ## Plans
 
 - [ ] Increase API coverage, or modify code generation to rely on the official UniFi Controller API specifications
-- [ ] Improve error handling (currently only basic error handling is implemented and some of the errors are swallowed)
+- [x] Improve error handling (currently only basic error handling is implemented and error details are not propagated)
 - [x] Improve client code for better usability
 - [x] Support API Key authentication
 - [ ] Generate client code for currently generated API structures, for use within or outside the Terraform provider

unifi/unifi.go

@@ -42,57 +42,15 @@ const (
 	UserAgentHeader   = "User-Agent"
 	AcceptHeader      = "Accept"
 	ContentTypeHeader = "Content-Type"
-)
 
-var (
-	ErrAuthenticationFailed = errors.New("authentication failed")
-	ErrNotFound             = errors.New("not found")
-)
-
-type APIError struct {
-	RC      string
-	Message string
-}
-
-func (err *APIError) Error() string {
-	return err.Message
-}
-
-func (err *APIError) Is(target error) bool {
-	var apiError *APIError
-	if errors.As(target, &apiError) {
-		if err.RC == apiError.RC && err.Message == apiError.Message {
-			return true
-		}
-	}
-	return false
-}
-
-type Meta struct {
-	RC      string `json:"rc"`
-	Message string `json:"msg"`
-}
-
-func (m *Meta) error() error {
-	if m.RC != "ok" {
-		return &APIError{
-			RC:      m.RC,
-			Message: m.Message,
-		}
-	}
-
-	return nil
-}
-
-type validationMode string
-
-const (
 	SoftValidation    validationMode = "soft"
 	HardValidation    validationMode = "hard"
 	DisableValidation validationMode = "disable"
 	DefaultValidation validationMode = SoftValidation // TODO: change to hard in next major version
 )
 
+type validationMode string
+
 type ClientConfig struct {
 	URL            string        `validate:"required,http_url"`
 	APIKey         string        `validate:"required_without_all=User Pass"`
@@ -147,8 +105,8 @@ var (
 
 type ServerInfo struct {
 	Up            bool   `json:"up"`
-	ServerVersion string `fake:"{appversion}" json:"server_version"`
+	ServerVersion string `json:"server_version"`
-	UUID          string `fake:"{uuid}"       json:"uuid"`
+	UUID          string `json:"uuid"`
 }
 
 type HttpCustomizer func(transport *http.Transport) error
@@ -214,39 +172,6 @@ type ResponseErrorHandler interface {
 	HandleError(resp *http.Response) error
 }
 
-type DefaultResponseErrorHandler struct{}
-
-func (d *DefaultResponseErrorHandler) HandleError(resp *http.Response) error {
-	switch resp.StatusCode {
-	case http.StatusOK:
-		return nil
-	case http.StatusNotFound:
-		return ErrNotFound
-	case http.StatusUnauthorized:
-		return ErrAuthenticationFailed
-	}
-	errBody := struct {
-		Meta Meta `json:"Meta"`
-		Data []struct {
-			Meta Meta `json:"Meta"`
-		} `json:"data"`
-	}{}
-	if err := json.NewDecoder(resp.Body).Decode(&errBody); err != nil {
-		return err
-	}
-	var apiErr error
-	if len(errBody.Data) > 0 && errBody.Data[0].Meta.RC == "error" {
-		// check first error in data, should we look for more than one?
-		apiErr = errBody.Data[0].Meta.error()
-	}
-	if apiErr == nil {
-		apiErr = errBody.Meta.error()
-	}
-
-	// TODO: check rc in addition to status code?
-	return fmt.Errorf("%w (%s) for %s %s", apiErr, resp.Status, resp.Request.Method, resp.Request.URL.String())
-}
-
 // NewClient creates a http.Client with authenticated cookies.
 // Used to make additional, authenticated requests to the APIs.
 // Start here.
@@ -258,9 +183,6 @@ func NewClient(config *ClientConfig) (*Client, error) {
 	if err := v.Validate(config); err != nil {
 		return nil, fmt.Errorf("failed validating config: %w", err)
 	}
-	if config.ValidationMode == "" {
-		config.ValidationMode = DefaultValidation
-	}
 	u, err := newUnifi(config, v)
 	if err != nil {
 		return nil, fmt.Errorf("failed creating unifi client: %w", err)
@@ -351,6 +273,9 @@ func newUnifi(config *ClientConfig, v *validator) (*Client, error) {
 	} else {
 		errorHandler = &DefaultResponseErrorHandler{}
 	}
+	if config.ValidationMode == "" {
+		config.ValidationMode = DefaultValidation
+	}
 	u := &Client{
 		BaseURL:      baseURL,
 		config:       config,

unifi/unifi_errors.go

@@ -0,0 +1,169 @@
+package unifi
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"strings"
+)
+
+var ErrNotFound = errors.New("not found")
+
+// TODO old-style error handling to be removed in future versions.
+type APIError struct {
+	RC      string
+	Message string
+}
+
+func (err *APIError) Error() string {
+	return err.Message
+}
+
+func (err *APIError) Is(target error) bool {
+	var apiError *APIError
+	if errors.As(target, &apiError) {
+		if err.RC == apiError.RC && err.Message == apiError.Message {
+			return true
+		}
+	}
+	return false
+}
+
+type Meta struct {
+	RC      string `json:"rc"`
+	Message string `json:"msg"`
+}
+
+func (m *Meta) error() error {
+	if m.RC != "ok" {
+		return &APIError{
+			RC:      m.RC,
+			Message: m.Message,
+		}
+	}
+
+	return nil
+}
+
+type DefaultResponseErrorHandler struct{}
+
+type apiV2ResponseError struct {
+	Code      string                    `json:"code"`
+	ErrorCode int                       `json:"errorCode"`
+	Message   string                    `json:"message"`
+	Details   apiV2ResponseErrorDetails `json:"details"`
+}
+
+type apiV2ResponseErrorDetails struct {
+	// probably there are more fields, but I didn't get any response with more fields
+	InvalidFields []string `json:"invalid_fields"`
+}
+
+type apiV1ResponseError struct {
+	Meta Meta                     `json:"Meta"`
+	Data []apiV1ResponseErrorData `json:"data"`
+}
+
+type apiV1ResponseErrorData struct {
+	Meta            Meta                  `json:"Meta"`
+	ValidationError ServerValidationError `json:"validationError"`
+	RC              string                `json:"rc"`
+	Message         string                `json:"msg"`
+}
+
+type apiResponseError struct {
+	apiV1ResponseError
+	apiV2ResponseError
+}
+
+type ServerValidationError struct {
+	Field   string `json:"field"`
+	Pattern string `json:"pattern"`
+}
+
+type ServerErrorDetails struct {
+	Message         string
+	ValidationError ServerValidationError
+}
+
+type ServerError struct {
+	StatusCode    int
+	RequestMethod string
+	RequestURL    string
+	Message       string
+	ErrorCode     string
+	Details       []ServerErrorDetails
+}
+
+func (s *ServerError) Error() string {
+	var b strings.Builder
+	b.WriteString(fmt.Sprintf("Server error (%d) for %s %s: %s", s.StatusCode, s.RequestMethod, s.RequestURL, s.Message))
+	for _, detail := range s.Details {
+		b.WriteString("\n")
+		if detail.Message != "" {
+			b.WriteString(detail.Message + ": ")
+		}
+		if detail.ValidationError.Field != "" && detail.ValidationError.Pattern != "" {
+			b.WriteString(fmt.Sprintf("field '%s' should match '%s'", detail.ValidationError.Field, detail.ValidationError.Pattern))
+		} else if detail.ValidationError.Field != "" {
+			b.WriteString(fmt.Sprintf("field '%s' is invalid", detail.ValidationError.Field))
+		} else if detail.ValidationError.Pattern != "" {
+			b.WriteString(fmt.Sprintf("field should match '%s'", detail.ValidationError.Pattern))
+		}
+	}
+	return b.String()
+}
+
+func parseApiV2Error(err apiV2ResponseError, serverError *ServerError) {
+	serverError.Message = err.Message
+	serverError.ErrorCode = err.Code
+	for _, field := range err.Details.InvalidFields {
+		details := ServerErrorDetails{}
+		details.ValidationError.Field = field
+		serverError.Details = append(serverError.Details, details)
+	}
+}
+
+func parseApiV1Error(err apiV1ResponseError, serverError *ServerError) {
+	for _, d := range err.Data {
+		if d.Meta.RC == "error" || d.RC == "error" {
+			details := ServerErrorDetails{}
+			details.Message = d.Message
+			if details.Message == "" {
+				details.Message = d.Meta.Message
+			}
+			if d.ValidationError.Field != "" || d.ValidationError.Pattern != "" {
+				details.ValidationError = d.ValidationError
+			}
+			serverError.Details = append(serverError.Details, details)
+		}
+	}
+	if serverError.Message == "" {
+		serverError.Message = err.Meta.Message
+	}
+	serverError.ErrorCode = err.Meta.RC
+}
+
+func (d *DefaultResponseErrorHandler) HandleError(resp *http.Response) error {
+	if resp.StatusCode >= 200 && resp.StatusCode < 300 {
+		return nil
+	}
+
+	var errBody apiResponseError
+	if err := json.NewDecoder(resp.Body).Decode(&errBody); err != nil {
+		return err
+	}
+	serverError := ServerError{
+		StatusCode:    resp.StatusCode,
+		RequestMethod: resp.Request.Method,
+		RequestURL:    resp.Request.URL.String(),
+	}
+	if errBody.apiV2ResponseError.Code != "" || errBody.apiV2ResponseError.Message != "" {
+		parseApiV2Error(errBody.apiV2ResponseError, &serverError)
+	} else {
+		parseApiV1Error(errBody.apiV1ResponseError, &serverError)
+	}
+
+	return &serverError
+}

unifi/unifi_errors_test.go

@@ -0,0 +1,189 @@
+package unifi //nolint: testpackage
+
+import (
+	"bytes"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestServerError_Error(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name     string
+		input    ServerError
+		expected string
+	}{
+		{
+			name: "Error with message and validation error",
+			input: ServerError{
+				StatusCode:    400,
+				RequestMethod: "GET",
+				RequestURL:    "http://example.com",
+				Message:       "Bad Request",
+				Details: []ServerErrorDetails{
+					{
+						Message: "Invalid input",
+						ValidationError: ServerValidationError{
+							Field:   "username",
+							Pattern: "^[a-zA-Z0-9]+$",
+						},
+					},
+				},
+			},
+			expected: "Server error (400) for GET http://example.com: Bad Request\nInvalid input: field 'username' should match '^[a-zA-Z0-9]+$'",
+		},
+		{
+			name: "Error with message only",
+			input: ServerError{
+				StatusCode:    404,
+				RequestMethod: "POST",
+				RequestURL:    "http://example.com",
+				Message:       "Not Found",
+			},
+			expected: "Server error (404) for POST http://example.com: Not Found",
+		},
+		{
+			name: "Error with multiple validation errors",
+			input: ServerError{
+				StatusCode:    422,
+				RequestMethod: "PUT",
+				RequestURL:    "http://example.com",
+				Message:       "Unprocessable Entity",
+				Details: []ServerErrorDetails{
+					{
+						Message: "Invalid username",
+						ValidationError: ServerValidationError{
+							Field:   "username",
+							Pattern: "^[a-zA-Z0-9]+$",
+						},
+					},
+					{
+						Message: "Invalid email",
+						ValidationError: ServerValidationError{
+							Field:   "email",
+							Pattern: "^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$",
+						},
+					},
+				},
+			},
+			expected: "Server error (422) for PUT http://example.com: Unprocessable Entity\nInvalid username: field 'username' should match '^[a-zA-Z0-9]+$'\nInvalid email: field 'email' should match '^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$'",
+		},
+		{
+			name: "Error with no details",
+			input: ServerError{
+				StatusCode:    500,
+				RequestMethod: "DELETE",
+				RequestURL:    "http://example.com",
+				Message:       "Internal Server Error",
+			},
+			expected: "Server error (500) for DELETE http://example.com: Internal Server Error",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			a := assert.New(t)
+			a.Equal(tt.expected, tt.input.Error())
+		})
+	}
+}
+
+func TestHandleError(t *testing.T) {
+	t.Parallel()
+	tests := []struct {
+		name          string
+		responseBody  string
+		statusCode    int
+		expectedError *ServerError
+	}{
+		{
+			name: "V2 error with invalid fields",
+			responseBody: `{
+				"code": "error_code",
+				"message": "error message",
+				"details": {
+					"invalid_fields": ["field1", "field2"]
+				}
+			}`,
+			statusCode: 400,
+			expectedError: &ServerError{
+				StatusCode:    400,
+				RequestMethod: "GET",
+				RequestURL:    "http://example.com",
+				Message:       "error message",
+				ErrorCode:     "error_code",
+				Details: []ServerErrorDetails{
+					{ValidationError: ServerValidationError{Field: "field1"}},
+					{ValidationError: ServerValidationError{Field: "field2"}},
+				},
+			},
+		},
+		{
+			name: "V1 error with validation error",
+			responseBody: `{
+				"Meta": {"rc": "error", "msg": "meta error message"},
+				"data": [{
+					"Meta": {"rc": "error", "msg": "data meta error message"},
+					"validationError": {"field": "field1", "pattern": "pattern1"},
+					"rc": "error",
+					"msg": "data error message"
+				}]
+			}`,
+			statusCode: 400,
+			expectedError: &ServerError{
+				StatusCode:    400,
+				RequestMethod: "GET",
+				RequestURL:    "http://example.com",
+				Message:       "meta error message",
+				ErrorCode:     "error",
+				Details: []ServerErrorDetails{
+					{
+						Message: "data error message",
+						ValidationError: ServerValidationError{
+							Field:   "field1",
+							Pattern: "pattern1",
+						},
+					},
+				},
+			},
+		},
+		{
+			name:          "No error",
+			responseBody:  `{"Meta": {"rc": "ok", "msg": "success"}}`,
+			statusCode:    200,
+			expectedError: nil,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			a := assert.New(t)
+			handler := &DefaultResponseErrorHandler{}
+
+			// Create a new HTTP response recorder
+			recorder := httptest.NewRecorder()
+			recorder.WriteHeader(tt.statusCode)
+			recorder.Body = bytes.NewBufferString(tt.responseBody)
+
+			// Create a new HTTP request
+			req := httptest.NewRequest(http.MethodGet, "http://example.com", nil)
+			recorder.Result().Request = req
+
+			// Call the HandleError function
+			err := handler.HandleError(recorder.Result())
+
+			if tt.expectedError == nil {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				a.EqualValues(tt.expectedError, err)
+			}
+		})
+	}
+}

unifi/user.go

@@ -58,6 +58,7 @@ func (c *Client) CreateUser(ctx context.Context, site string, d *User) (*User, e
 		return nil, errors.New("malformed group response")
 	}
 
+	// TODO verify if this is still needed cause it's the only place where old-style error handling is used
 	if err := respBody.Data[0].Meta.error(); err != nil {
 		return nil, err
 	}