Repository: newrelic/newrelic-client-go
Pull request #89 (2020-01-13 20:17:31)

fix: make use of ErrorNotFound type for Get methods that are based on List methods
The `ErrorNotFound` type is meaningful for downstream use in the Terraform provider.  It initially represented a 404 response from a REST Show endpoint, but it should also be used for Get methods that do not have corresponding Show endpoints and need to iterate over results returned from a List endpoint .

This PR puts the ErrorNotFound type in place for these scenarios, and also allows for custom error messages.


Patch:
diff --git a/internal/http/client.go b/internal/http/client.go
index ed3a0c3..111f71a 100644
--- a/internal/http/client.go
+++ b/internal/http/client.go
@@ -228,7 +228,7 @@ func (c *NewRelicClient) do(
 	defer resp.Body.Close()
 
 	if resp.StatusCode == http.StatusNotFound {
-		return nil, &errors.ErrorNotFound{}
+		return nil, &errors.NotFound{}
 	}
 
 	body, readErr := ioutil.ReadAll(resp.Body)
@@ -240,9 +240,7 @@ func (c *NewRelicClient) do(
 		errorValue := c.errorValue
 		_ = json.Unmarshal(body, &errorValue)
 
-		return nil, &errors.ErrorUnexpectedStatusCode{
-			StatusCode: resp.StatusCode,
-			Err:        c.errorValue.Error()}
+		return nil, errors.NewUnexpectedStatusCode(resp.StatusCode, c.errorValue.Error())
 	}
 
 	if value == nil {
diff --git a/pkg/alerts/channels.go b/pkg/alerts/channels.go
index 34a6bfa..570aadc 100644
--- a/pkg/alerts/channels.go
+++ b/pkg/alerts/channels.go
@@ -2,6 +2,8 @@ package alerts
 
 import (
 	"fmt"
+
+	"github.com/newrelic/newrelic-client-go/pkg/errors"
 )
 
 // ListChannels returns all alert channels for a given account.
@@ -38,8 +40,7 @@ func (alerts *Alerts) GetChannel(id int) (*Channel, error) {
 			return channel, nil
 		}
 	}
-
-	return nil, fmt.Errorf("no channel found for id %d", id)
+	return nil, errors.NewNotFoundf("no channel found for id %d", id)
 }
 
 // CreateChannel creates an alert channel within a given account.
diff --git a/pkg/alerts/conditions.go b/pkg/alerts/conditions.go
index 6b3f59d..91b041c 100644
--- a/pkg/alerts/conditions.go
+++ b/pkg/alerts/conditions.go
@@ -2,6 +2,8 @@ package alerts
 
 import (
 	"fmt"
+
+	"github.com/newrelic/newrelic-client-go/pkg/errors"
 )
 
 // ListConditions returns alert conditions for a specified policy.
@@ -47,7 +49,7 @@ func (alerts *Alerts) GetCondition(policyID int, id int) (*Condition, error) {
 		}
 	}
 
-	return nil, fmt.Errorf("no condition found for policy %d and condition ID %d", policyID, id)
+	return nil, errors.NewNotFoundf("no condition found for policy %d and condition ID %d", policyID, id)
 }
 
 // CreateCondition creates an alert condition for a specified policy.
diff --git a/pkg/alerts/nrql_conditions.go b/pkg/alerts/nrql_conditions.go
index 6474d7d..f0a76bf 100644
--- a/pkg/alerts/nrql_conditions.go
+++ b/pkg/alerts/nrql_conditions.go
@@ -2,6 +2,8 @@ package alerts
 
 import (
 	"fmt"
+
+	"github.com/newrelic/newrelic-client-go/pkg/errors"
 )
 
 // ListNrqlConditions returns NRQL alert conditions for a specified policy.
@@ -48,7 +50,7 @@ func (alerts *Alerts) GetNrqlCondition(policyID int, id int) (*NrqlCondition, er
 		}
 	}
 
-	return nil, fmt.Errorf("no condition found for policy %d and condition ID %d", policyID, id)
+	return nil, errors.NewNotFoundf("no condition found for policy %d and condition ID %d", policyID, id)
 }
 
 // CreateNrqlCondition creates a NRQL alert condition.
diff --git a/pkg/alerts/plugins_conditions.go b/pkg/alerts/plugins_conditions.go
index 6967899..8e3e0ba 100644
--- a/pkg/alerts/plugins_conditions.go
+++ b/pkg/alerts/plugins_conditions.go
@@ -1,6 +1,10 @@
 package alerts
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/newrelic/newrelic-client-go/pkg/errors"
+)
 
 // ListPluginsConditions returns alert conditions for New Relic plugins for a given alert policy.
 func (alerts *Alerts) ListPluginsConditions(policyID int) ([]*PluginsCondition, error) {
@@ -47,7 +51,7 @@ func (alerts *Alerts) GetPluginsCondition(policyID int, pluginID int) (*PluginsC
 		}
 	}
 
-	return nil, fmt.Errorf("no condition found for policy %d and condition ID %d", policyID, pluginID)
+	return nil, errors.NewNotFoundf("no condition found for policy %d and condition ID %d", policyID, pluginID)
 }
 
 // CreatePluginsCondition creates an alert condition for a plugin.
diff --git a/pkg/alerts/policies.go b/pkg/alerts/policies.go
index 4f502f5..31f45e4 100644
--- a/pkg/alerts/policies.go
+++ b/pkg/alerts/policies.go
@@ -2,6 +2,8 @@ package alerts
 
 import (
 	"fmt"
+
+	"github.com/newrelic/newrelic-client-go/pkg/errors"
 )
 
 // ListPoliciesParams represents a set of filters to be
@@ -46,7 +48,7 @@ func (alerts *Alerts) GetPolicy(id int) (*Policy, error) {
 		}
 	}
 
-	return nil, fmt.Errorf("no alert policy found for id %d", id)
+	return nil, errors.NewNotFoundf("no alert policy found for id %d", id)
 }
 
 // CreatePolicy creates a new alert policy for a given account.
diff --git a/pkg/apm/labels.go b/pkg/apm/labels.go
index efa7a22..23bc849 100644
--- a/pkg/apm/labels.go
+++ b/pkg/apm/labels.go
@@ -1,6 +1,10 @@
 package apm
 
-import "fmt"
+import (
+	"fmt"
+
+	"github.com/newrelic/newrelic-client-go/pkg/errors"
+)
 
 // ListLabels returns the labels within an account.
 func (apm *APM) ListLabels() ([]*Label, error) {
@@ -39,7 +43,7 @@ func (apm *APM) GetLabel(key string) (*Label, error) {
 		}
 	}
 
-	return nil, fmt.Errorf("no label found with key %s", key)
+	return nil, errors.NewNotFoundf("no label found with key %s", key)
 }
 
 // CreateLabel creates a new label within an account.
diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go
index 665b35d..659af36 100644
--- a/pkg/errors/errors.go
+++ b/pkg/errors/errors.go
@@ -5,27 +5,63 @@ import (
 	"fmt"
 )
 
-// ErrorNotFound is returned when a 404 response is returned
-// from New Relic's APIs.
-type ErrorNotFound struct{}
+// NewNotFound returns a new instance of NotFound
+// with an optional custom error message.
+func NewNotFound(err string) *NotFound {
+	e := NotFound{
+		err: err,
+	}
 
-func (e *ErrorNotFound) Error() string {
-	return fmt.Sprintf("404 not found")
+	return &e
 }
 
-// ErrorUnexpectedStatusCode is returned when an unexpected
-// status code is returned from New Relic's APIs.
-type ErrorUnexpectedStatusCode struct {
-	Err        string
-	StatusCode int
+// NewNotFoundf returns a new instance of NotFound
+// with an optional formatted custom error message.
+func NewNotFoundf(format string, args ...interface{}) *NotFound {
+	return NewNotFound(fmt.Sprintf(format, args...))
+}
+
+// NotFound is returned when the target resource
+// cannot be located.
+type NotFound struct {
+	err string
+}
+
+func (e *NotFound) Error() string {
+	if e.err == "" {
+		return "resource not found"
+	}
+
+	return e.err
 }
 
-func (e *ErrorUnexpectedStatusCode) Error() string {
+// NewUnexpectedStatusCode returns a new instance of UnexpectedStatusCode
+// with an optional custom message.
+func NewUnexpectedStatusCode(statusCode int, err string) *UnexpectedStatusCode {
+	return &UnexpectedStatusCode{
+		err:        err,
+		statusCode: statusCode,
+	}
+}
+
+// NewUnexpectedStatusCodef returns a new instance of UnexpectedStatusCode
+// with an optional formatted custom message.
+func NewUnexpectedStatusCodef(statusCode int, format string, args ...interface{}) *UnexpectedStatusCode {
+	return NewUnexpectedStatusCode(statusCode, fmt.Sprintf(format, args...))
+}
+
+// UnexpectedStatusCode is returned when an unexpected
+// status code is returned from New Relic's APIs.
+type UnexpectedStatusCode struct {
+	err        string
+	statusCode int
+}
 
-	msg := fmt.Sprintf("%d response returned", e.StatusCode)
+func (e *UnexpectedStatusCode) Error() string {
+	msg := fmt.Sprintf("%d response returned", e.statusCode)
 
-	if e.Err != "" {
-		msg += fmt.Sprintf(": %s", e.Err)
+	if e.err != "" {
+		msg += fmt.Sprintf(": %s", e.err)
 	}
 
 	return msg


Test patch:
diff --git a/internal/http/client_test.go b/internal/http/client_test.go
index d8aaf13..113e869 100644
--- a/internal/http/client_test.go
+++ b/internal/http/client_test.go
@@ -54,7 +54,7 @@ func TestDefaultErrorValue(t *testing.T) {
 
 	_, err := c.Get("/path", nil, nil)
 
-	assert.Equal(t, err.(*errors.ErrorUnexpectedStatusCode).Err, "error message")
+	assert.Contains(t, err.(*errors.UnexpectedStatusCode).Error(), "error message")
 }
 
 type CustomErrorResponse struct {
@@ -77,7 +77,7 @@ func TestCustomErrorValue(t *testing.T) {
 
 	_, err := c.Get("/path", nil, nil)
 
-	assert.Equal(t, err.(*errors.ErrorUnexpectedStatusCode).Err, "error message")
+	assert.Contains(t, err.(*errors.UnexpectedStatusCode).Error(), "error message")
 }
 
 type CustomResponseValue struct {
@@ -239,7 +239,7 @@ func TestErrNotFound(t *testing.T) {
 
 	_, err := c.Get("/path", nil, nil)
 
-	assert.IsType(t, &errors.ErrorNotFound{}, err)
+	assert.IsType(t, &errors.NotFound{}, err)
 }
 
 func TestInternalServerError(t *testing.T) {
@@ -250,7 +250,7 @@ func TestInternalServerError(t *testing.T) {
 
 	_, err := c.Get("/path", nil, nil)
 
-	assert.IsType(t, &errors.ErrorUnexpectedStatusCode{}, err)
+	assert.IsType(t, &errors.UnexpectedStatusCode{}, err)
 }
 
 func TestPost(t *testing.T) {
diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go
index f4912b1..f8d3c1d 100644
--- a/pkg/errors/errors_test.go
+++ b/pkg/errors/errors_test.go
@@ -11,18 +11,15 @@ import (
 func TestErrorNotFound(t *testing.T) {
 	t.Parallel()
 
-	var e ErrorNotFound
+	var e NotFound
 
-	assert.Equal(t, "404 not found", e.Error())
+	assert.Equal(t, "resource not found", e.Error())
 }
 
 func TestErrorUnexpectedStatusCode(t *testing.T) {
 	t.Parallel()
 
-	e := ErrorUnexpectedStatusCode{
-		StatusCode: 99,
-		Err:        "wat",
-	}
+	e := NewUnexpectedStatusCode(99, "wat")
 
 	assert.Equal(t, "99 response returned: wat", e.Error())
 }