tidy up error refactor

honeycombio · Jul 17, 2024 · a11c290 · a11c290
1 parent ce17d27
commit a11c290
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 52 deletions.
diff --git a/client/client.go b/client/client.go
@@ -221,7 +221,7 @@ func (c *Client) Do(ctx context.Context, method, path string, requestBody, respo
 	defer resp.Body.Close()
 
 	if !(resp.StatusCode >= 200 && resp.StatusCode <= 299) {
-		return FromResponse(resp)
+		return ErrorFromResponse(resp)
 	}
 	if responseBody != nil {
 		err = json.NewDecoder(resp.Body).Decode(responseBody)

diff --git a/client/errors.go b/client/errors.go
@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/jsonapi"
 )
 
+// DetailedError is an RFC7807 'Problem Detail' formatted error message.
 type DetailedError struct {
 	// The HTTP status code of the error.
 	Status int `json:"status,omitempty"`
@@ -81,7 +82,7 @@ func (e DetailedError) Error() string {
 	return e.Message
 }
 
-func FromResponse(r *http.Response) error {
+func ErrorFromResponse(r *http.Response) error {
 	if r == nil {
 		return errors.New("invalid response")
 	}

diff --git a/client/errors_test.go b/client/errors_test.go
@@ -12,6 +12,50 @@ import (
 	"github.com/honeycombio/terraform-provider-honeycombio/client"
 )
 
+func TestClient_ParseDetailedError(t *testing.T) {
+	t.Parallel()
+
+	var de client.DetailedError
+	ctx := context.Background()
+	c := newTestClient(t)
+
+	t.Run("Post with no body should fail with 400 unparseable", func(t *testing.T) {
+		err := c.Do(ctx, "POST", "/1/boards/", nil, nil)
+		require.Error(t, err)
+		require.ErrorAs(t, err, &de)
+		assert.Equal(t, http.StatusBadRequest, de.Status)
+		assert.Equal(t, fmt.Sprintf("%s/problems/unparseable", c.EndpointURL()), de.Type)
+		assert.Equal(t, "The request body could not be parsed.", de.Title)
+		assert.Equal(t, "could not parse request body", de.Message)
+	})
+
+	t.Run("Get into non-existent dataset should fail with 404 'Dataset not found'", func(t *testing.T) {
+		_, err := c.Markers.Get(ctx, "non-existent-dataset", "abcd1234")
+		require.Error(t, err)
+		require.ErrorAs(t, err, &de)
+		assert.Equal(t, http.StatusNotFound, de.Status)
+		assert.Equal(t, fmt.Sprintf("%s/problems/not-found", c.EndpointURL()), de.Type)
+		assert.Equal(t, "The requested resource cannot be found.", de.Title)
+		assert.Equal(t, "Dataset not found", de.Message)
+	})
+
+	t.Run("Creating a dataset without a name should return a validation error", func(t *testing.T) {
+		createDatasetRequest := &client.Dataset{}
+		_, err := c.Datasets.Create(ctx, createDatasetRequest)
+		require.Error(t, err)
+		require.ErrorAs(t, err, &de)
+		assert.Equal(t, http.StatusUnprocessableEntity, de.Status)
+		assert.Equal(t, fmt.Sprintf("%s/problems/validation-failed", c.EndpointURL()), de.Type)
+		assert.Equal(t, "The provided input is invalid.", de.Title)
+		assert.Equal(t, "The provided input is invalid.", de.Message)
+		assert.Len(t, de.Details, 1)
+		assert.Equal(t, "missing", de.Details[0].Code)
+		assert.Equal(t, "name", de.Details[0].Field)
+		assert.Equal(t, "cannot be blank", de.Details[0].Description)
+		assert.Equal(t, "missing name - cannot be blank", de.Error())
+	})
+}
+
 func TestErrors_DetailedError_Error(t *testing.T) {
 	t.Parallel()
 
@@ -147,47 +191,3 @@ func TestErrors_ErrorTypeDetail_String(t *testing.T) {
 		})
 	}
 }
-
-func TestClient_Errors(t *testing.T) {
-	t.Parallel()
-
-	var de client.DetailedError
-	ctx := context.Background()
-	c := newTestClient(t)
-
-	t.Run("Post with no body should fail with 400 unparseable", func(t *testing.T) {
-		err := c.Do(ctx, "POST", "/1/boards/", nil, nil)
-		require.Error(t, err)
-		require.ErrorAs(t, err, &de)
-		assert.Equal(t, http.StatusBadRequest, de.Status)
-		assert.Equal(t, fmt.Sprintf("%s/problems/unparseable", c.EndpointURL()), de.Type)
-		assert.Equal(t, "The request body could not be parsed.", de.Title)
-		assert.Equal(t, "could not parse request body", de.Message)
-	})
-
-	t.Run("Get into non-existent dataset should fail with 404 'Dataset not found'", func(t *testing.T) {
-		_, err := c.Markers.Get(ctx, "non-existent-dataset", "abcd1234")
-		require.Error(t, err)
-		require.ErrorAs(t, err, &de)
-		assert.Equal(t, http.StatusNotFound, de.Status)
-		assert.Equal(t, fmt.Sprintf("%s/problems/not-found", c.EndpointURL()), de.Type)
-		assert.Equal(t, "The requested resource cannot be found.", de.Title)
-		assert.Equal(t, "Dataset not found", de.Message)
-	})
-
-	t.Run("Creating a dataset without a name should return a validation error", func(t *testing.T) {
-		createDatasetRequest := &client.Dataset{}
-		_, err := c.Datasets.Create(ctx, createDatasetRequest)
-		require.Error(t, err)
-		require.ErrorAs(t, err, &de)
-		assert.Equal(t, http.StatusUnprocessableEntity, de.Status)
-		assert.Equal(t, fmt.Sprintf("%s/problems/validation-failed", c.EndpointURL()), de.Type)
-		assert.Equal(t, "The provided input is invalid.", de.Title)
-		assert.Equal(t, "The provided input is invalid.", de.Message)
-		assert.Len(t, de.Details, 1)
-		assert.Equal(t, "missing", de.Details[0].Code)
-		assert.Equal(t, "name", de.Details[0].Field)
-		assert.Equal(t, "cannot be blank", de.Details[0].Description)
-		assert.Equal(t, "missing name - cannot be blank", de.Error())
-	})
-}
diff --git a/client/v2/api_keys.go b/client/v2/api_keys.go
@@ -41,7 +41,7 @@ func (a *apiKeys) Create(ctx context.Context, k *APIKey) (*APIKey, error) {
 		return nil, err
 	}
 	if r.StatusCode != http.StatusCreated {
-		return nil, hnyclient.FromResponse(r)
+		return nil, hnyclient.ErrorFromResponse(r)
 	}
 
 	key := new(APIKey)
@@ -61,7 +61,7 @@ func (a *apiKeys) Get(ctx context.Context, id string) (*APIKey, error) {
 		return nil, err
 	}
 	if r.StatusCode != http.StatusOK {
-		return nil, hnyclient.FromResponse(r)
+		return nil, hnyclient.ErrorFromResponse(r)
 	}
 
 	key := new(APIKey)
@@ -81,7 +81,7 @@ func (a *apiKeys) Update(ctx context.Context, k *APIKey) (*APIKey, error) {
 		return nil, err
 	}
 	if r.StatusCode != http.StatusOK {
-		return nil, hnyclient.FromResponse(r)
+		return nil, hnyclient.ErrorFromResponse(r)
 	}
 
 	key := new(APIKey)
@@ -101,7 +101,7 @@ func (a *apiKeys) Delete(ctx context.Context, id string) error {
 		return err
 	}
 	if r.StatusCode != http.StatusNoContent {
-		return hnyclient.FromResponse(r)
+		return hnyclient.ErrorFromResponse(r)
 	}
 	return nil
 }

diff --git a/client/v2/client.go b/client/v2/client.go
@@ -158,7 +158,7 @@ func (c *Client) AuthInfo(ctx context.Context) (*AuthMetadata, error) {
 		return nil, err
 	}
 	if r.StatusCode != http.StatusOK {
-		return nil, hnyclient.FromResponse(r)
+		return nil, hnyclient.ErrorFromResponse(r)
 	}
 
 	auth := new(AuthMetadata)

diff --git a/client/v2/pagination.go b/client/v2/pagination.go
@@ -89,7 +89,7 @@ func (p *Pager[T]) Next(ctx context.Context) ([]*T, error) {
 	}
 
 	if r.StatusCode != http.StatusOK {
-		return nil, hnyclient.FromResponse(r)
+		return nil, hnyclient.ErrorFromResponse(r)
 	}
 	pagination, err := parsePagination(r)
 	if err != nil {