Repository: honeycombio/terraform-provider-honeycombio
Pull request #392 (2023-11-13 22:55:03)

client: Fix bug where API errors weren't including the field or getting separated
## Short description of the changes
This PR fixes a bug where the errors returned from the client weren't showing the impacted field and weren't getting separated when there were multiple errors associated with 1 request. This made any server-side errors a little difficult to read/fix.

I'm not super familiar with all the different forms the Details section of DetailedError can take, so if there are checks I can remove, please let me know. 

## How to verify that this has the expected result
Pre-requisites:
- You'll need Go 1.20 and the latest Terraform installed.
- You'll need a team in Honeycomb to test against.
- You'll need a dataset, derived column, and SLO for that team.
- You'll need an API key for that team.

#### Setup:
1. Create a new directory called `providers` somewhere you can easily access (but not inside the same folder as your Terraform config).
1. Build the provider by checking out this branch and running `make build`
1. Move the executable created to the `providers` directory you created earlier.
   ```
   mv terraform-provider-honeycombio /PATH/TO/YOUR/DIRECTORY/providers
   ```
1. Add the following to your ~/.terraformrc file:
   ```
   provider_installation {

     # Use /PATH/TO/YOUR/DIRECTORY/providers/terraform-provider-honeycombio
     # as an overridden package directory for the honeycombio/honeycombio provider. 
     # This disables the version and checksum verifications for this provider and 
     # forces Terraform to look for the null provider plugin in the given directory.
     dev_overrides {
       "honeycombio/honeycombio" = "/PATH/TO/YOUR/DIRECTORY/providers"
     }

     # For all other providers, install them directly from their origin provider
     # registries as normal. If you omit this, Terraform will _only_ use
     # the dev_overrides block, and so no other providers will be available.
     direct {}
   }
   ```
1. Create another new directory (all Terraform commands will be run in this directory)
1.  [Download and save this file](https://github.com/honeycombio/terraform-provider-honeycombio/files/13342818/error-display.tf.txt) as `main.tf` in your new directory. You'll be modifying it in the steps below. 
1. Set the following environment variables:
   ```
   export HONEYCOMB_API_KEY=YOUR_API_KEY_HERE
   export TF_VAR_dataset=YOUR_DATASET_SLUG_HERE
   export TF_VAR_slo_id=YOUR_SLO_ID_HERE
   ```
1. Run `terraform init`. You should see a yellow warning block telling you that development overrides are in place. It should look like this:
   <img width="1293" alt="Screenshot 2023-11-13 at 3 34 46 PM" src="https://github.com/honeycombio/terraform-provider-honeycombio/assets/12189856/7a1d4383-2581-493d-88ab-923476baafc2">

#### Provision resources for r/burn_alert:
1. Run `terraform apply`. This should fail with an error that look like the following:
   <img width="868" alt="Screenshot 2023-11-13 at 5 02 16 PM" src="https://github.com/honeycombio/terraform-provider-honeycombio/assets/12189856/458697fc-2928-4d5c-8b4a-ddac232bfeab">
   - If you check in the UI, you should see that the burn alert was not created.
 
## Related links
- [API documentation](https://docs.honeycomb.io/api/tag/Errors#schema/ValidationError)

## Screenshots
### Before:
<img width="878" alt="Screenshot 2023-11-13 at 5 04 57 PM" src="https://github.com/honeycombio/terraform-provider-honeycombio/assets/12189856/14dcefa3-c2c4-4cfb-a691-86a76e6e42eb">

### After:
<img width="868" alt="Screenshot 2023-11-13 at 5 02 16 PM" src="https://github.com/honeycombio/terraform-provider-honeycombio/assets/12189856/4e83fc8d-5fd1-4680-8fd3-9a411f90c56d">


codecov-commenter: ## [Codecov](https://app.codecov.io/gh/honeycombio/terraform-provider-honeycombio/pull/392?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=honeycombio) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`bd9eef5`)](https://app.codecov.io/gh/honeycombio/terraform-provider-honeycombio/commit/bd9eef5c17961c91c638ee478e298d0fb99adc3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=honeycombio) 79.24% compared to head [(`869d08f`)](https://app.codecov.io/gh/honeycombio/terraform-provider-honeycombio/pull/392?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=honeycombio) 79.55%.
> Report is 5 commits behind head on main.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #392      +/-   ##
==========================================
+ Coverage   79.24%   79.55%   +0.30%     
==========================================
  Files          55       55              
  Lines        4654     4675      +21     
==========================================
+ Hits         3688     3719      +31     
+ Misses        810      800      -10     
  Partials      156      156              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/honeycombio/terraform-provider-honeycombio/pull/392?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=honeycombio).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=honeycombio).


Patch:
diff --git a/client/errors.go b/client/errors.go
index 3c95393..68dfca9 100644
--- a/client/errors.go
+++ b/client/errors.go
@@ -18,23 +18,54 @@ type DetailedError struct {
 	// Title is a human-readable summary that explains the type of the problem.
 	Title string `json:"title,omitempty"`
 	// Details is an array of structured objects that give details about the error.
-	Details []struct {
-		Code        string `json:"code"`
-		Description string `json:"description"`
-		Field       string `json:"field"`
-	} `json:"type_detail,omitempty"`
+	Details []ErrorTypeDetail `json:"type_detail,omitempty"`
+}
+
+type ErrorTypeDetail struct {
+	Code        string `json:"code"`
+	Description string `json:"description"`
+	Field       string `json:"field"`
+}
+
+func (td ErrorTypeDetail) String() string {
+	response := ""
+	if td.Code != "" {
+		response += td.Code
+		// If any of the fields that could come next are present, add a space separator
+		if td.Field != "" || td.Description != "" {
+			response += " "
+		}
+	}
+
+	if td.Field != "" {
+		response += td.Field
+		// If any of the fields that could come next are present, add a dash separator
+		if td.Description != "" {
+			response += " - "
+		}
+	}
+
+	if td.Description != "" {
+		response += td.Description
+	}
+
+	return response
 }
 
 // Error returns a pretty-printed representation of the error
 func (e DetailedError) Error() string {
 	if len(e.Details) > 0 {
 		var response string
-		for i, d := range e.Details {
-			response += d.Code + " - " + d.Description
-			if i > len(e.Details)-1 {
-				response += ", "
+
+		for index, details := range e.Details {
+			response += details.String()
+
+			// If we haven't reached the end of the list of error details, add a newline separator between each error
+			if index < len(e.Details)-1 {
+				response += "\n"
 			}
 		}
+
 		return response
 	}
 


Test patch:
diff --git a/client/errors_test.go b/client/errors_test.go
index 4f754d7..7b24bda 100644
--- a/client/errors_test.go
+++ b/client/errors_test.go
@@ -33,4 +33,154 @@ func TestClient_ParseDetailedError(t *testing.T) {
 		assert.Equal(t, de.Title, "The requested resource cannot be found.")
 		assert.Equal(t, de.Message, "Dataset not found")
 	})
+
+	t.Run("Creating a dataset without a name should return a validation error", func(t *testing.T) {
+		createDatasetRequest := &Dataset{}
+		_, err := c.Datasets.Create(ctx, createDatasetRequest)
+		require.Error(t, err)
+		assert.ErrorAs(t, err, &de)
+		assert.Equal(t, http.StatusUnprocessableEntity, de.Status)
+		assert.Equal(t, "https://api.honeycomb.io/problems/validation-failed", de.Type)
+		assert.Equal(t, "The provided input is invalid.", de.Title)
+		assert.Equal(t, "The provided input is invalid.", de.Message)
+		assert.Equal(t, 1, len(de.Details))
+		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()
+
+	testCases := []struct {
+		name           string
+		input          DetailedError
+		expectedOutput string
+	}{
+		{
+			name: "multiple details get separated by newline",
+			input: DetailedError{
+				Message: "test message",
+				Details: []ErrorTypeDetail{
+					{
+						Code:        "test code1",
+						Field:       "test_field1",
+						Description: "test description1",
+					},
+					{
+						Code:        "test code2",
+						Field:       "test_field2",
+						Description: "test description2",
+					},
+				},
+			},
+			expectedOutput: "test code1 test_field1 - test description1\ntest code2 test_field2 - test description2",
+		},
+		{
+			name: "empty details returns message",
+			input: DetailedError{
+				Message: "test message",
+				Details: []ErrorTypeDetail{},
+			},
+			expectedOutput: "test message",
+		},
+		{
+			name: "one item in details has no newlines",
+			input: DetailedError{
+				Message: "test message",
+				Details: []ErrorTypeDetail{
+					{
+						Code:        "test code",
+						Field:       "test_field",
+						Description: "test description",
+					},
+				},
+			},
+			expectedOutput: "test code test_field - test description",
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			actualOutput := testCase.input.Error()
+			assert.Equal(t, testCase.expectedOutput, actualOutput)
+		})
+	}
+}
+
+func TestErrors_ErrorTypeDetail_String(t *testing.T) {
+	testCases := []struct {
+		name           string
+		input          ErrorTypeDetail
+		expectedOutput string
+	}{
+		{
+			name: "happy path: Code, Field, and Description present",
+			input: ErrorTypeDetail{
+				Code:        "test code",
+				Field:       "test_field",
+				Description: "test description",
+			},
+			expectedOutput: "test code test_field - test description",
+		},
+		{
+			name:           "all fields blank returns empty string",
+			input:          ErrorTypeDetail{},
+			expectedOutput: "",
+		},
+		{
+			name: "empty Code",
+			input: ErrorTypeDetail{
+				Field:       "test_field",
+				Description: "test description",
+			},
+			expectedOutput: "test_field - test description",
+		},
+		{
+			name: "empty Code and Field",
+			input: ErrorTypeDetail{
+				Description: "test description",
+			},
+			expectedOutput: "test description",
+		},
+		{
+			name: "empty Code and Description",
+			input: ErrorTypeDetail{
+				Field: "test_field",
+			},
+			expectedOutput: "test_field",
+		},
+		{
+			name: "empty Field",
+			input: ErrorTypeDetail{
+				Code:        "test code",
+				Description: "test description",
+			},
+			expectedOutput: "test code test description",
+		},
+		{
+			name: "empty Field and Description",
+			input: ErrorTypeDetail{
+				Code: "test code",
+			},
+			expectedOutput: "test code",
+		},
+		{
+			name: "empty Description",
+			input: ErrorTypeDetail{
+				Code:  "test code",
+				Field: "test_field",
+			},
+			expectedOutput: "test code test_field",
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			actualOutput := testCase.input.String()
+			assert.Equal(t, testCase.expectedOutput, actualOutput)
+		})
+	}
 }