Repository: newrelic/newrelic-client-go
Pull request #603 (2021-01-11 20:08:08)

fix(http): update error message for 401 status code to account for missing API key AND invalid API key
Resolves: #605

Note: We'll need to release the Terraform provider with this update to satisfy the parent ticket referenced in #605 


Patch:
diff --git a/internal/http/client.go b/internal/http/client.go
index 5b9bf1b..7acefd8 100644
--- a/internal/http/client.go
+++ b/internal/http/client.go
@@ -339,6 +339,10 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 	}
 
 	if !isResponseSuccess(resp) {
+		if errorValue.IsUnauthorized(resp) {
+			return nil, nrErrors.NewUnauthorizedError()
+		}
+
 		return nil, nrErrors.NewUnexpectedStatusCode(resp.StatusCode, errorValue.Error())
 	}
 
diff --git a/internal/http/errors.go b/internal/http/errors.go
index 3dd9f1e..d7f50ce 100644
--- a/internal/http/errors.go
+++ b/internal/http/errors.go
@@ -2,6 +2,7 @@ package http
 
 import (
 	"fmt"
+	"net/http"
 	"strings"
 )
 
@@ -10,6 +11,7 @@ import (
 type ErrorResponse interface {
 	IsNotFound() bool
 	IsRetryableError() bool
+	IsUnauthorized(resp *http.Response) bool
 	Error() string
 	New() ErrorResponse
 }
@@ -42,6 +44,11 @@ func (e *DefaultErrorResponse) IsRetryableError() bool {
 	return false
 }
 
+// IsUnauthorized checks a response for a 401 Unauthorize HTTP status code.
+func (e *DefaultErrorResponse) IsUnauthorized(resp *http.Response) bool {
+	return resp.StatusCode == http.StatusUnauthorized
+}
+
 // New creates a new instance of the DefaultErrorResponse struct.
 func (e *DefaultErrorResponse) New() ErrorResponse {
 	return &DefaultErrorResponse{}
diff --git a/internal/http/graphql.go b/internal/http/graphql.go
index 262f488..357ead8 100644
--- a/internal/http/graphql.go
+++ b/internal/http/graphql.go
@@ -2,6 +2,7 @@ package http
 
 import (
 	"encoding/json"
+	"net/http"
 	"strings"
 )
 
@@ -20,6 +21,7 @@ type GraphQLError struct {
 	Path       []string `json:"path,omitempty"`
 	Extensions struct {
 		ErrorClass string `json:"errorClass,omitempty"`
+		ErrorCode  string `json:"error_code,omitempty"`
 	} `json:"extensions,omitempty"`
 	DownstreamResponse []GraphQLDownstreamResponse `json:"downstreamResponse,omitempty"`
 }
@@ -91,6 +93,27 @@ func (r *GraphQLErrorResponse) IsRetryableError() bool {
 	return false
 }
 
+// IsUnauthorized checks a NerdGraph response for a 401 Unauthorize HTTP status code,
+// then falls back to check the nested extensions error_code field for `BAD_API_KEY`.
+func (r *GraphQLErrorResponse) IsUnauthorized(resp *http.Response) bool {
+	if len(r.Errors) == 0 {
+		return false
+	}
+
+	if resp.StatusCode == http.StatusUnauthorized {
+		return true
+	}
+
+	// Handle invalid or missing API key
+	for _, err := range r.Errors {
+		if err.Extensions.ErrorCode == "BAD_API_KEY" {
+			return true
+		}
+	}
+
+	return false
+}
+
 // New creates a new instance of GraphQLErrorRepsonse.
 func (r *GraphQLErrorResponse) New() ErrorResponse {
 	return &GraphQLErrorResponse{}
diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go
index 2e601db..8b1a1de 100644
--- a/pkg/errors/errors.go
+++ b/pkg/errors/errors.go
@@ -3,6 +3,7 @@ package errors
 
 import (
 	"fmt"
+	"net/http"
 )
 
 // NewNotFound returns a new instance of NotFound with an optional custom error message.
@@ -92,3 +93,29 @@ func (e *UnexpectedStatusCode) Error() string {
 
 	return msg
 }
+
+// NewUnauthorizedError returns a new instance of UnauthorizedError
+// with an optional custom message.
+func NewUnauthorizedError() *UnauthorizedError {
+	return &UnauthorizedError{
+		err:        "Invalid credentials provided. Missing API key or an invalid API key was provided.",
+		statusCode: http.StatusUnauthorized,
+	}
+}
+
+// UnauthorizedError is returned when a 401 HTTP status code is returned
+// from New Relic's APIs.
+type UnauthorizedError struct {
+	err        string
+	statusCode int
+}
+
+func (e *UnauthorizedError) Error() string {
+	msg := fmt.Sprintf("%d response returned", e.statusCode)
+
+	if e.err != "" {
+		msg += fmt.Sprintf(": %s", e.err)
+	}
+
+	return msg
+}
diff --git a/pkg/infrastructure/infrastructure.go b/pkg/infrastructure/infrastructure.go
index 3f4a94a..a569d7d 100644
--- a/pkg/infrastructure/infrastructure.go
+++ b/pkg/infrastructure/infrastructure.go
@@ -2,7 +2,9 @@
 package infrastructure
 
 import (
-	"github.com/newrelic/newrelic-client-go/internal/http"
+	"net/http"
+
+	nrhttp "github.com/newrelic/newrelic-client-go/internal/http"
 )
 
 // ErrorResponse represents an error response from New Relic Infrastructure.
@@ -31,7 +33,7 @@ func (e *ErrorResponse) Error() string {
 }
 
 // New creates a new instance of ErrorResponse.
-func (e *ErrorResponse) New() http.ErrorResponse {
+func (e *ErrorResponse) New() nrhttp.ErrorResponse {
 	return &ErrorResponse{}
 }
 
@@ -42,3 +44,8 @@ func (e *ErrorResponse) IsNotFound() bool {
 func (e *ErrorResponse) IsRetryableError() bool {
 	return false
 }
+
+// IsUnauthorized checks a response for a 401 Unauthorize HTTP status code.
+func (e *ErrorResponse) IsUnauthorized(resp *http.Response) bool {
+	return resp.StatusCode == http.StatusUnauthorized
+}
diff --git a/pkg/synthetics/synthetics.go b/pkg/synthetics/synthetics.go
index 98756da..d7e145f 100644
--- a/pkg/synthetics/synthetics.go
+++ b/pkg/synthetics/synthetics.go
@@ -2,19 +2,20 @@
 package synthetics
 
 import (
+	"net/http"
 	"strings"
 
-	"github.com/newrelic/newrelic-client-go/internal/http"
+	nrhttp "github.com/newrelic/newrelic-client-go/internal/http"
 	"github.com/newrelic/newrelic-client-go/internal/logging"
 	"github.com/newrelic/newrelic-client-go/pkg/config"
 )
 
 // Synthetics is used to communicate with the New Relic Synthetics product.
 type Synthetics struct {
-	client http.Client
+	client nrhttp.Client
 	config config.Config
 	logger logging.Logger
-	pager  http.Pager
+	pager  nrhttp.Pager
 }
 
 // ErrorResponse represents an error response from New Relic Synthetics.
@@ -51,7 +52,7 @@ func (e *ErrorResponse) Error() string {
 }
 
 // New creates a new instance of ErrorResponse.
-func (e *ErrorResponse) New() http.ErrorResponse {
+func (e *ErrorResponse) New() nrhttp.ErrorResponse {
 	return &ErrorResponse{}
 }
 
@@ -63,17 +64,22 @@ func (e *ErrorResponse) IsRetryableError() bool {
 	return false
 }
 
+// IsUnauthorized checks the response for a 401 Unauthorize HTTP status code.
+func (e *ErrorResponse) IsUnauthorized(resp *http.Response) bool {
+	return resp.StatusCode == http.StatusUnauthorized
+}
+
 // New is used to create a new Synthetics client instance.
 func New(config config.Config) Synthetics {
-	client := http.NewClient(config)
-	client.SetAuthStrategy(&http.PersonalAPIKeyCapableV2Authorizer{})
+	client := nrhttp.NewClient(config)
+	client.SetAuthStrategy(&nrhttp.PersonalAPIKeyCapableV2Authorizer{})
 	client.SetErrorValue(&ErrorResponse{})
 
 	pkg := Synthetics{
 		client: client,
 		config: config,
 		logger: config.GetLogger(),
-		pager:  &http.LinkHeaderPager{},
+		pager:  &nrhttp.LinkHeaderPager{},
 	}
 
 	return pkg


Test patch:
diff --git a/internal/http/client_test.go b/internal/http/client_test.go
index b7f60c5..8d1c0fc 100644
--- a/internal/http/client_test.go
+++ b/internal/http/client_test.go
@@ -67,6 +67,20 @@ func TestDefaultErrorValue(t *testing.T) {
 	assert.Contains(t, err.Error(), "error message")
 }
 
+func TestUnauthorizedErrorValue(t *testing.T) {
+	t.Parallel()
+	c := NewTestAPIClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusUnauthorized)
+		_, _ = w.Write([]byte(`{"error":{"title": "No API key specified"}}`)) // REST API 401 response body
+	}))
+
+	_, err := c.Get(c.config.Region().RestURL("path"), nil, nil)
+
+	// Ensure our custom 401 unauthorized error message is returned
+	assert.Contains(t, err.Error(), "Invalid credentials provided")
+}
+
 type CustomErrorResponse struct {
 	CustomError string `json:"custom"`
 }
@@ -87,6 +101,10 @@ func (c *CustomErrorResponse) IsRetryableError() bool {
 	return false
 }
 
+func (c *CustomErrorResponse) IsUnauthorized(resp *http.Response) bool {
+	return resp.StatusCode == http.StatusUnauthorized
+}
+
 func TestCustomErrorValue(t *testing.T) {
 	t.Parallel()
 	c := NewTestAPIClient(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go
index f8d3c1d..465fb45 100644
--- a/pkg/errors/errors_test.go
+++ b/pkg/errors/errors_test.go
@@ -3,6 +3,7 @@
 package errors
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -23,3 +24,12 @@ func TestErrorUnexpectedStatusCode(t *testing.T) {
 
 	assert.Equal(t, "99 response returned: wat", e.Error())
 }
+
+func TestErrorUnauthorized(t *testing.T) {
+	t.Parallel()
+
+	e := NewUnauthorizedError()
+
+	assert.Equal(t, 401, e.statusCode)
+	assert.True(t, strings.Contains(e.Error(), "Invalid credentials provided"))
+}
diff --git a/pkg/nerdgraph/nerdgraph_integration_test.go b/pkg/nerdgraph/nerdgraph_integration_test.go
index ad8159a..7ecdc0e 100644
--- a/pkg/nerdgraph/nerdgraph_integration_test.go
+++ b/pkg/nerdgraph/nerdgraph_integration_test.go
@@ -1,4 +1,4 @@
-// +build sometimes
+// +build integration
 
 package nerdgraph
 
diff --git a/pkg/synthetics/synthetics_test.go b/pkg/synthetics/synthetics_test.go
index ee02c4f..fa0a99b 100644
--- a/pkg/synthetics/synthetics_test.go
+++ b/pkg/synthetics/synthetics_test.go
@@ -36,7 +36,6 @@ func TestError(t *testing.T) {
 	// 1st Server Error Message
 	e.ServerErrorMessage = "server message"
 	assert.Equal(t, "server message", e.Error())
-
 }
 
 func newTestClient(t *testing.T, handler http.Handler) Synthetics {