Enhance reliability of how we make requests and parse errors (#108)

management/anomaly.go

@@ -18,27 +18,28 @@ func newAnomalyManager(m *Management) *AnomalyManager {
 //
 // See: https://auth0.com/docs/api/management/v2#!/Anomaly/get_ips_by_id
 func (m *AnomalyManager) CheckIP(ip string, opts ...RequestOption) (isBlocked bool, err error) {
-	req, err := m.NewRequest("GET", m.URI("anomaly", "blocks", "ips", ip), nil, opts...)
+	request, err := m.NewRequest("GET", m.URI("anomaly", "blocks", "ips", ip), nil, opts...)
 	if err != nil {
 		return false, err
 	}
 
-	res, err := m.Do(req)
+	response, err := m.Do(request)
 	if err != nil {
 		return false, err
 	}
+	defer response.Body.Close()
 
 	// 200: IP address specified is currently blocked.
-	if res.StatusCode == http.StatusOK {
+	if response.StatusCode == http.StatusOK {
 		return true, nil
 	}
 
 	// 404: IP address specified is not currently blocked.
-	if res.StatusCode == http.StatusNotFound {
+	if response.StatusCode == http.StatusNotFound {
 		return false, nil
 	}
 
-	return false, newError(res.Body)
+	return false, newError(response)
 }
 
 // UnblockIP unblocks an IP address currently blocked by the multiple

management/branding.go

@@ -3,7 +3,6 @@ package management
 import (
 	"encoding/json"
 	"fmt"
-	"net/http"
 )
 
 // Branding is used to customize the look and feel of Auth0 to align
@@ -168,21 +167,7 @@ func (m *BrandingManager) UniversalLogin(opts ...RequestOption) (ul *BrandingUni
 //
 // See: https://auth0.com/docs/api/management/v2#!/Branding/put_universal_login
 func (m *BrandingManager) SetUniversalLogin(ul *BrandingUniversalLogin, opts ...RequestOption) (err error) {
-	req, err := m.NewRequest("PUT", m.URI("branding", "templates", "universal-login"), ul.Body, opts...)
-	if err != nil {
-		return err
-	}
-
-	res, err := m.Do(req)
-	if err != nil {
-		return err
-	}
-
-	if res.StatusCode >= http.StatusBadRequest {
-		return newError(res.Body)
-	}
-
-	return nil
+	return m.Request("PUT", m.URI("branding", "templates", "universal-login"), ul.Body, opts...)
 }
 
 // DeleteUniversalLogin deletes the template for the New Universal Login Experience.

management/guardian.go

@@ -146,23 +146,23 @@ type EnrollmentTicket struct {
 //
 // See: https://auth0.com/docs/api/management/v2#!/Guardian/post_ticket
 func (m *EnrollmentManager) CreateTicket(t *CreateEnrollmentTicket, opts ...RequestOption) (EnrollmentTicket, error) {
-	req, err := m.NewRequest("POST", m.URI("guardian", "enrollments", "ticket"), t, opts...)
+	request, err := m.NewRequest("POST", m.URI("guardian", "enrollments", "ticket"), t, opts...)
 	if err != nil {
 		return EnrollmentTicket{}, err
 	}
 
-	res, err := m.Do(req)
+	response, err := m.Do(request)
 	if err != nil {
 		return EnrollmentTicket{}, err
 	}
-	defer res.Body.Close()
+	defer response.Body.Close()
 
-	if res.StatusCode != http.StatusOK {
-		return EnrollmentTicket{}, newError(res.Body)
+	if response.StatusCode != http.StatusOK {
+		return EnrollmentTicket{}, newError(response)
 	}
 
 	var out EnrollmentTicket
-	err = json.NewDecoder(res.Body).Decode(&out)
+	err = json.NewDecoder(response.Body).Decode(&out)
 	return out, err
 }
 

management/job.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"mime/multipart"
-	"net/http"
 	"net/textproto"
 	"strconv"
 	"time"
@@ -126,29 +125,7 @@ func (m *JobManager) ImportUsers(j *Job, opts ...RequestOption) error {
 	}
 	mp.Close()
 
-	req, err := http.NewRequest("POST", m.URI("jobs", "users-imports"), &payload)
-	if err != nil {
-		return err
-	}
-	req.Header.Add("Content-Type", mp.FormDataContentType())
-
-	for _, option := range opts {
-		option.apply(req)
-	}
-
-	res, err := m.Do(req)
-	if err != nil {
-		return err
-	}
-
-	if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest {
-		return newError(res.Body)
-	}
-
-	if res.StatusCode != http.StatusNoContent {
-		defer res.Body.Close()
-		return json.NewDecoder(res.Body).Decode(j)
-	}
+	opts = append(opts, Header("Content-Type", mp.FormDataContentType()))
 
-	return nil
+	return m.Request("POST", m.URI("jobs", "users-imports"), &payload, opts...)
 }

management/management_error.go

@@ -3,7 +3,7 @@ package management
 import (
 	"encoding/json"
 	"fmt"
-	"io"
+	"net/http"
 )
 
 // Error is an interface describing any error which
@@ -21,13 +21,25 @@ type managementError struct {
 	Message    string `json:"message"`
 }
 
-func newError(r io.Reader) error {
-	m := &managementError{}
-	if err := json.NewDecoder(r).Decode(m); err != nil {
-		return err
+func newError(response *http.Response) error {
+	apiError := &managementError{}
+
+	if err := json.NewDecoder(response.Body).Decode(apiError); err != nil {
+		return &managementError{
+			StatusCode: response.StatusCode,
+			Err:        http.StatusText(response.StatusCode),
+			Message:    fmt.Errorf("failed to decode json error response payload: %w", err).Error(),
+		}
+	}
+
+	// This can happen in case the error message structure changes.
+	// If that happens we still want to display the correct code.
+	if apiError.Status() == 0 {
+		apiError.StatusCode = response.StatusCode
+		apiError.Err = http.StatusText(response.StatusCode)
 	}
 
-	return m
+	return apiError
 }
 
 // Error formats the error into a string representation.

management/management_error_test.go

@@ -0,0 +1,62 @@
+package management
+
+import (
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestNewError(t *testing.T) {
+	var testCases = []struct {
+		name          string
+		givenResponse http.Response
+		expectedError managementError
+	}{
+		{
+			name: "it fails to decode if body is not a json",
+			givenResponse: http.Response{
+				StatusCode: http.StatusForbidden,
+				Body:       io.NopCloser(strings.NewReader("Hello, I'm not a JSON.")),
+			},
+			expectedError: managementError{
+				StatusCode: 403,
+				Err:        "Forbidden",
+				Message:    "failed to decode json error response payload: invalid character 'H' looking for beginning of value",
+			},
+		},
+		{
+			name: "it correctly decodes the error response payload",
+			givenResponse: http.Response{
+				StatusCode: http.StatusBadRequest,
+				Body:       io.NopCloser(strings.NewReader(`{"statusCode":400,"error":"Bad Request","message":"One of 'client_id' or 'name' is required."}`)),
+			},
+			expectedError: managementError{
+				StatusCode: 400,
+				Err:        "Bad Request",
+				Message:    "One of 'client_id' or 'name' is required.",
+			},
+		},
+		{
+			name: "it will still post the correct status code if the body doesn't have the correct structure",
+			givenResponse: http.Response{
+				StatusCode: http.StatusInternalServerError,
+				Body:       io.NopCloser(strings.NewReader(`{"errorMessage":"wrongStruct"}`)),
+			},
+			expectedError: managementError{
+				StatusCode: 500,
+				Err:        "Internal Server Error",
+				Message:    "",
+			},
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			actualError := newError(&testCase.givenResponse)
+			assert.Equal(t, &testCase.expectedError, actualError)
+		})
+	}
+}

management/management_request.go

@@ -84,27 +84,32 @@ func (m *Management) Do(req *http.Request) (*http.Response, error) {
 }
 
 // Request combines NewRequest and Do, while also handling decoding of response payload.
-func (m *Management) Request(method, uri string, v interface{}, options ...RequestOption) error {
-	request, err := m.NewRequest(method, uri, v, options...)
+func (m *Management) Request(method, uri string, payload interface{}, options ...RequestOption) error {
+	request, err := m.NewRequest(method, uri, payload, options...)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to create a new request: %w", err)
 	}
 
 	response, err := m.Do(request)
 	if err != nil {
-		return fmt.Errorf("request failed: %w", err)
+		return fmt.Errorf("failed to send the request: %w", err)
 	}
+	defer response.Body.Close()
 
-	if response.StatusCode < http.StatusOK || response.StatusCode >= http.StatusBadRequest {
-		return newError(response.Body)
+	// If the response contains a client or a server error then return the error.
+	if response.StatusCode >= http.StatusBadRequest {
+		return newError(response)
 	}
 
-	if response.StatusCode != http.StatusNoContent && response.StatusCode != http.StatusAccepted {
-		if err := json.NewDecoder(response.Body).Decode(v); err != nil {
-			return fmt.Errorf("decoding response payload failed: %w", err)
-		}
+	responseBody, err := io.ReadAll(response.Body)
+	if err != nil {
+		return fmt.Errorf("failed to read the response body: %w", err)
+	}
 
-		return response.Body.Close()
+	if len(responseBody) > 0 && string(responseBody) != "{}" {
+		if err = json.Unmarshal(responseBody, &payload); err != nil {
+			return fmt.Errorf("failed to unmarshal response payload: %w", err)
+		}
 	}
 
 	return nil

management/management_test.go

@@ -129,7 +129,7 @@ func TestOptionParameter(t *testing.T) {
 	assert.Equal(t, "xyz", bar)
 }
 
-func TestOptionDefauls(t *testing.T) {
+func TestOptionDefaults(t *testing.T) {
 	r, _ := http.NewRequest("GET", "/", nil)
 
 	applyListDefaults([]RequestOption{

management/user.go

@@ -2,6 +2,8 @@ package management
 
 import (
 	"encoding/json"
+	"fmt"
+	"io"
 	"net/http"
 	"reflect"
 	"strconv"
@@ -530,26 +532,30 @@ func (m *UserManager) InvalidateRememberBrowser(id string, opts ...RequestOption
 //
 // See: https://auth0.com/docs/api/management/v2#!/Users/post_identities
 func (m *UserManager) Link(id string, il *UserIdentityLink, opts ...RequestOption) (uIDs []UserIdentity, err error) {
-	req, err := m.NewRequest("POST", m.URI("users", id, "identities"), il, opts...)
+	request, err := m.NewRequest("POST", m.URI("users", id, "identities"), il, opts...)
 	if err != nil {
 		return uIDs, err
 	}
 
-	res, err := m.Do(req)
+	response, err := m.Do(request)
 	if err != nil {
 		return uIDs, err
 	}
+	defer response.Body.Close()
 
-	if res.StatusCode < http.StatusOK || res.StatusCode >= http.StatusBadRequest {
-		return uIDs, newError(res.Body)
+	if response.StatusCode >= http.StatusBadRequest {
+		return uIDs, newError(response)
 	}
 
-	if res.StatusCode != http.StatusNoContent && res.StatusCode != http.StatusAccepted {
-		err := json.NewDecoder(res.Body).Decode(&uIDs)
-		if err != nil {
-			return uIDs, err
+	responseBody, err := io.ReadAll(response.Body)
+	if err != nil {
+		return uIDs, fmt.Errorf("failed to read the response body: %w", err)
+	}
+
+	if len(responseBody) > 0 {
+		if err = json.Unmarshal(responseBody, &uIDs); err != nil {
+			return uIDs, fmt.Errorf("failed to unmarshal response payload: %w", err)
 		}
-		return uIDs, res.Body.Close()
 	}
 
 	return uIDs, nil