Repository: newrelic/newrelic-client-go
Pull request #540 (2020-11-09 22:41:23)

fix(http): include INTERNAL_SERVER_ERROR as a retry reason
Without this change, a downstream error returned by GraphQL of
INTERNAL_SERVER_ERROR is not included in the reasons to retry an call.  Here we
rename the IsTimeout method to IsRetryableError to be a bit more generic, and
include another condition in which we might want to retry the call.


Patch:
diff --git a/internal/http/client.go b/internal/http/client.go
index 6e8fddc..f0b17cc 100644
--- a/internal/http/client.go
+++ b/internal/http/client.go
@@ -280,7 +280,7 @@ func (c *Client) Do(req *Request) (*http.Response, error) {
 		return nil, nrErrors.NewNotFound("resource not found")
 	}
 
-	if errorValue.IsTimeout() {
+	if errorValue.IsRetryableError() {
 		return nil, nrErrors.NewTimeout("server timeout")
 	}
 
@@ -367,7 +367,7 @@ func (c *Client) innerDo(req *Request, errorValue ErrorResponse, i int) (*http.R
 
 	_ = json.Unmarshal(body, &errorValue)
 
-	if errorValue.IsTimeout() {
+	if errorValue.IsRetryableError() {
 		shouldRetry = true
 	}
 
diff --git a/internal/http/errors.go b/internal/http/errors.go
index 899e566..3dd9f1e 100644
--- a/internal/http/errors.go
+++ b/internal/http/errors.go
@@ -9,7 +9,7 @@ import (
 // a single error message from an error response object.
 type ErrorResponse interface {
 	IsNotFound() bool
-	IsTimeout() bool
+	IsRetryableError() bool
 	Error() string
 	New() ErrorResponse
 }
@@ -38,7 +38,7 @@ func (e *DefaultErrorResponse) IsNotFound() bool {
 	return false
 }
 
-func (e *DefaultErrorResponse) IsTimeout() bool {
+func (e *DefaultErrorResponse) IsRetryableError() bool {
 	return false
 }
 
diff --git a/internal/http/graphql.go b/internal/http/graphql.go
index 54660c3..44058d5 100644
--- a/internal/http/graphql.go
+++ b/internal/http/graphql.go
@@ -66,8 +66,8 @@ func (r *GraphQLErrorResponse) IsNotFound() bool {
 	return false
 }
 
-// IsTimeout determines if the error is due to a server timeout.
-func (r *GraphQLErrorResponse) IsTimeout() bool {
+// IsRetryableError determines if the error is due to a server timeout, or another error that we might want to retry.
+func (r *GraphQLErrorResponse) IsRetryableError() bool {
 	if len(r.Errors) == 0 {
 		return false
 	}
@@ -76,6 +76,10 @@ func (r *GraphQLErrorResponse) IsTimeout() bool {
 		if err.Extensions.ErrorClass == "TIMEOUT" {
 			return true
 		}
+
+		if err.Extensions.ErrorClass == "INTERNAL_SERVER_ERROR" {
+			return true
+		}
 	}
 
 	return false
diff --git a/pkg/infrastructure/infrastructure.go b/pkg/infrastructure/infrastructure.go
index 3399f49..3f4a94a 100644
--- a/pkg/infrastructure/infrastructure.go
+++ b/pkg/infrastructure/infrastructure.go
@@ -39,6 +39,6 @@ func (e *ErrorResponse) IsNotFound() bool {
 	return false
 }
 
-func (e *ErrorResponse) IsTimeout() bool {
+func (e *ErrorResponse) IsRetryableError() bool {
 	return false
 }
diff --git a/pkg/synthetics/synthetics.go b/pkg/synthetics/synthetics.go
index cebe927..98756da 100644
--- a/pkg/synthetics/synthetics.go
+++ b/pkg/synthetics/synthetics.go
@@ -59,7 +59,7 @@ func (e *ErrorResponse) IsNotFound() bool {
 	return false
 }
 
-func (e *ErrorResponse) IsTimeout() bool {
+func (e *ErrorResponse) IsRetryableError() bool {
 	return false
 }
 


Test patch:
diff --git a/internal/http/client_test.go b/internal/http/client_test.go
index 918cce8..b7f60c5 100644
--- a/internal/http/client_test.go
+++ b/internal/http/client_test.go
@@ -83,7 +83,7 @@ func (c *CustomErrorResponse) IsNotFound() bool {
 	return false
 }
 
-func (c *CustomErrorResponse) IsTimeout() bool {
+func (c *CustomErrorResponse) IsRetryableError() bool {
 	return false
 }