Enhancement: standardise error handling

internal/common/error.go

@@ -0,0 +1,48 @@
+// Copyright Splunk, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package common
+
+import (
+	"context"
+	"net/http"
+
+	"github.com/hashicorp/terraform-plugin-log/tflog"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/signalfx/signalfx-go"
+
+	tfext "github.com/splunk-terraform/terraform-provider-signalfx/internal/tfextension"
+)
+
+// OnError handles the general case when the signalfx api returns
+// an error, and it uses that information to determine what needs to happen.
+// This will ensure that the state is cleaned up given the error condition.
+// To help simplify error handling, it will always return the error provided.
+func OnError(ctx context.Context, err error, data *schema.ResourceData) error {
+	re, ok := signalfx.AsResponseError(err)
+	if !ok {
+		// Not a response error, pass it back
+		return err
+	}
+	switch re.Code() {
+	case http.StatusNotFound:
+		tflog.Info(ctx, "Resource has been removed externally, removing from state", tfext.NewLogFields().
+			Field("route", re.Route()),
+		)
+		// Clear the id from the state when 404 is returned.
+		data.SetId("")
+	case http.StatusUnauthorized:
+		tflog.Error(ctx, "Token is not authorized", tfext.NewLogFields().
+			Field("route", re.Route()).
+			Field("code", re.Code()).
+			Field("details", re.Details()),
+		)
+	default:
+		tflog.Debug(ctx, "Issue trying to work with the API", tfext.NewLogFields().
+			Field("route", re.Route()).
+			Field("code", re.Code()).
+			Field("details", re.Details()),
+		)
+	}
+	return err
+}

internal/common/error_test.go

@@ -0,0 +1,54 @@
+// Copyright Splunk, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package common
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/signalfx/signalfx-go"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestOnError(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name   string
+		err    error
+		expect string
+	}{
+		{
+			name:   "no error provided",
+			err:    nil,
+			expect: "id",
+		},
+		{
+			name:   "not a response rror",
+			err:    errors.New("derp"),
+			expect: "id",
+		},
+		{
+			name:   "uncatalogue response error",
+			err:    &signalfx.ResponseError{},
+			expect: "id",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			data := schema.TestResourceDataRaw(
+				t,
+				map[string]*schema.Schema{},
+				map[string]any{},
+			)
+			data.SetId("id")
+
+			assert.ErrorIs(t, tc.err, OnError(context.Background(), tc.err, data), "Must return the same error")
+			assert.Equal(t, tc.expect, data.Id(), "Must have the expected id")
+		})
+	}
+}

internal/definition/detector/resource.go

@@ -12,6 +12,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/signalfx/signalfx-go/detector"
 
+	"github.com/splunk-terraform/terraform-provider-signalfx/internal/common"
 	pmeta "github.com/splunk-terraform/terraform-provider-signalfx/internal/providermeta"
 	tfext "github.com/splunk-terraform/terraform-provider-signalfx/internal/tfextension"
 )
@@ -63,7 +64,7 @@ func resourceCreate(ctx context.Context, data *schema.ResourceData, meta any) (i
 		ParentDetectorId:     dt.ParentDetectorId,
 		DetectorOrigin:       dt.DetectorOrigin,
 	})
-	if err != nil {
+	if common.OnError(ctx, err, data) != nil {
 		return tfext.AsErrorDiagnostics(err)
 	}
 
@@ -88,7 +89,7 @@ func resourceRead(ctx context.Context, data *schema.ResourceData, meta any) (iss
 	}
 
 	dt, err := client.GetDetector(ctx, data.Id())
-	if err != nil {
+	if common.OnError(ctx, err, data) != nil {
 		return tfext.AsErrorDiagnostics(err)
 	}
 
@@ -133,7 +134,7 @@ func resourceUpdate(ctx context.Context, data *schema.ResourceData, meta any) (i
 		ParentDetectorId:     dt.ParentDetectorId,
 		DetectorOrigin:       dt.DetectorOrigin,
 	})
-	if err != nil {
+	if common.OnError(ctx, err, data) != nil {
 		return tfext.AsErrorDiagnostics(err)
 	}
 
@@ -154,5 +155,6 @@ func resourceDelete(ctx context.Context, data *schema.ResourceData, meta any) di
 	if err != nil {
 		return tfext.AsErrorDiagnostics(err)
 	}
-	return tfext.AsErrorDiagnostics(client.DeleteDetector(ctx, data.Id()))
+	err = common.OnError(ctx, client.DeleteDetector(ctx, data.Id()), data)
+	return tfext.AsErrorDiagnostics(err)
 }

internal/definition/team/resource.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/signalfx/signalfx-go/team"
 
+	"github.com/splunk-terraform/terraform-provider-signalfx/internal/common"
 	pmeta "github.com/splunk-terraform/terraform-provider-signalfx/internal/providermeta"
 )
 
@@ -49,7 +50,7 @@ func newResourceCreate() schema.CreateContextFunc {
 			Members:           payload.Members,
 			NotificationLists: payload.NotificationLists,
 		})
-		if err != nil {
+		if common.OnError(ctx, err, rd) != nil {
 			return diag.FromErr(err)
 		}
 
@@ -73,9 +74,10 @@ func newResourceRead() schema.ReadContextFunc {
 		}
 
 		tm, err := client.GetTeam(ctx, rd.Id())
-		if err != nil {
+		if common.OnError(ctx, err, rd) != nil {
 			return diag.FromErr(err)
 		}
+
 		tflog.Debug(ctx, "Successfully fetched team data")
 
 		if err := rd.Set("url", pmeta.LoadApplicationURL(ctx, meta, AppPath, tm.Id)); err != nil {
@@ -104,7 +106,7 @@ func newResourceUpdate() schema.UpdateContextFunc {
 			Members:           payload.Members,
 			NotificationLists: payload.NotificationLists,
 		})
-		if err != nil {
+		if common.OnError(ctx, err, rd) != nil {
 			return diag.FromErr(err)
 		}
 
@@ -128,6 +130,8 @@ func newResourceDelete() schema.DeleteContextFunc {
 			"team-id": rd.Id(),
 		})
 
-		return diag.FromErr(client.DeleteTeam(ctx, rd.Id()))
+		err = common.OnError(ctx, client.DeleteTeam(ctx, rd.Id()), rd)
+
+		return diag.FromErr(err)
 	}
 }