From 05d9531c86b4286546e0a7c08e08162fe4e67844 Mon Sep 17 00:00:00 2001 From: John Starich Date: Sat, 22 Oct 2022 22:37:04 -0500 Subject: [PATCH] Add introspection retrier option (#31) Allows a custom Retrier during introspection. The included CountRetrier stops retries after a number of attempts. Also refactored IntrospectOptions a bit to handle more options with less complexity. --- go.mod | 3 +- go.sum | 10 +++++ introspection.go | 72 +++++++++++++++++++++---------- introspection_test.go | 98 +++++++++++++++++++++++++++++++++++++++++++ retrier.go | 31 ++++++++++++++ retrier_test.go | 20 +++++++++ 6 files changed, 210 insertions(+), 24 deletions(-) create mode 100644 retrier.go create mode 100644 retrier_test.go diff --git a/go.mod b/go.mod index 6665523..290df53 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,8 @@ require ( github.com/graph-gophers/dataloader v5.0.0+incompatible github.com/mitchellh/mapstructure v1.1.2 github.com/opentracing/opentracing-go v1.0.2 // indirect - github.com/stretchr/testify v1.4.0 + github.com/pkg/errors v0.9.1 + github.com/stretchr/testify v1.8.0 github.com/vektah/gqlparser/v2 v2.0.1 golang.org/x/net v0.0.0-20190213061140-3a22650c66bd // indirect ) diff --git a/go.sum b/go.sum index 4b1422e..9ca8a56 100644 --- a/go.sum +++ b/go.sum @@ -16,13 +16,20 @@ github.com/mitchellh/mapstructure v1.1.2 h1:fmNYVwqnSfB9mZU6OS2O6GsXM+wcskZDuKQz github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= github.com/opentracing/opentracing-go v1.0.2 h1:3jA2P6O1F9UOrWVpwrIo17pu01KWvNWg4X946/Y5Zwg= github.com/opentracing/opentracing-go v1.0.2/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= +github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4= +github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/sergi/go-diff v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0= github.com/sergi/go-diff v1.1.0/go.mod h1:STckp+ISIX8hZLjrqAeVduY0gWCT9IjLuqbuNXdaHfM= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/objx v0.4.0 h1:M2gUjqZET1qApGOWNSnZ49BAIMX4F/1plDv3+l31EJ4= +github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= +github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.8.0 h1:pSgiaMZlXftHpm5L7V1+rVB+AZJydKsMxsQBIJw4PKk= +github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/vektah/gqlparser/v2 v2.0.1 h1:xgl5abVnsd4hkN9rk65OJID9bfcLSMuTaTcZj777q1o= github.com/vektah/gqlparser/v2 v2.0.1/go.mod h1:SyUiHgLATUR8BiYURfTirrTcGpcE+4XkV2se04Px1Ms= golang.org/x/net v0.0.0-20190213061140-3a22650c66bd h1:HuTn7WObtcDo9uEEU7rEqL0jYthdXAmZ6PP+meazmaU= @@ -34,3 +41,6 @@ gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4 h1:/eiJrUcujPVeJ3xlSWaiNi3uSVmDGBK1pDHUHAnao1I= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/introspection.go b/introspection.go index 01f88bc..0eae53a 100644 --- a/introspection.go +++ b/introspection.go @@ -2,18 +2,23 @@ package graphql import ( "context" - "errors" "fmt" "net/http" + "github.com/pkg/errors" "github.com/vektah/gqlparser/v2/ast" ) // IntrospectOptions represents the options for the IntrospectAPI function type IntrospectOptions struct { - wares []NetworkMiddleware - client *http.Client - ctx context.Context + // mergeFunc is an option-specific merger. This makes adding new options easier. + // If non-nil (i.e. created by an introspection func here), then sets its own options into opts. + mergeFunc func(opts *IntrospectOptions) + + client *http.Client + ctx context.Context + retrier Retrier + wares []NetworkMiddleware } // Context returns either a given context or an instance of the context.Background @@ -38,14 +43,8 @@ func (o *IntrospectOptions) Apply(queryer Queryer) Queryer { func mergeIntrospectOptions(opts ...*IntrospectOptions) *IntrospectOptions { res := &IntrospectOptions{} for _, opt := range opts { - if len(opt.wares) > 0 { - res.wares = append(res.wares, opt.wares...) - } - if opt.client != nil { - res.client = opt.client - } - if opt.ctx != nil { - res.ctx = opt.ctx + if opt.mergeFunc != nil { // Verify non-nil. Previously did not require mergeFuncs. so could panic if client code uses raw "&IntrospectOptions{}". + opt.mergeFunc(res) } } return res @@ -55,6 +54,9 @@ func mergeIntrospectOptions(opts ...*IntrospectOptions) *IntrospectOptions { // to be pass to an instance of a graphql.Queryer by the IntrospectOptions.Apply function func IntrospectWithMiddlewares(wares ...NetworkMiddleware) *IntrospectOptions { return &IntrospectOptions{ + mergeFunc: func(opts *IntrospectOptions) { + opts.wares = append(opts.wares, wares...) + }, wares: wares, } } @@ -63,16 +65,31 @@ func IntrospectWithMiddlewares(wares ...NetworkMiddleware) *IntrospectOptions { // to be pass to an instance of a graphql.Queryer by the IntrospectOptions.Apply function func IntrospectWithHTTPClient(client *http.Client) *IntrospectOptions { return &IntrospectOptions{ + mergeFunc: func(opts *IntrospectOptions) { + opts.client = client + }, client: client, } } +func introspectOptsFunc(fn func(opts *IntrospectOptions)) *IntrospectOptions { + return &IntrospectOptions{mergeFunc: fn} +} + // IntrospectWithHTTPClient returns an instance of graphql.IntrospectOptions with given context // to be used as a parameter for graphql.Queryer.Query function in the graphql.IntrospectAPI function func IntrospectWithContext(ctx context.Context) *IntrospectOptions { - return &IntrospectOptions{ - ctx: ctx, - } + return introspectOptsFunc(func(opts *IntrospectOptions) { + opts.ctx = ctx + }) +} + +// IntrospectWithRetrier returns an instance of graphql.IntrospectOptions with the given Retrier. +// For a fixed number of retries, see CountRetrier. +func IntrospectWithRetrier(retrier Retrier) *IntrospectOptions { + return introspectOptsFunc(func(opts *IntrospectOptions) { + opts.retrier = retrier + }) } // IntrospectRemoteSchema is used to build a RemoteSchema by firing the introspection query @@ -124,16 +141,25 @@ func IntrospectAPI(queryer Queryer, opts ...*IntrospectOptions) (*ast.Schema, er opt := mergeIntrospectOptions(opts...) queryer = opt.Apply(queryer) - // a place to hold the result of firing the introspection query - result := IntrospectionQueryResult{} - - input := &QueryInput{ - Query: IntrospectionQuery, - OperationName: "IntrospectionQuery", + query := func() (IntrospectionQueryResult, error) { + var result IntrospectionQueryResult + input := &QueryInput{ + Query: IntrospectionQuery, + OperationName: "IntrospectionQuery", + } + err := queryer.Query(opt.Context(), input, &result) + return result, errors.WithMessage(err, "query failed") } - // fire the introspection query - err := queryer.Query(opt.Context(), input, &result) + result, err := query() + if opt.retrier != nil { + // if available, retry on failures + var attempts uint = 1 + for err != nil && opt.retrier.ShouldRetry(err, attempts) { + result, err = query() + attempts++ + } + } if err != nil { return nil, err } diff --git a/introspection_test.go b/introspection_test.go index d492436..fcefd2f 100644 --- a/introspection_test.go +++ b/introspection_test.go @@ -1057,6 +1057,7 @@ func TestIntrospectWithMiddlewares(t *testing.T) { } func Test_mergeIntrospectOptions(t *testing.T) { + t.Parallel() client1 := &http.Client{} client2 := &http.Client{} wares1 := []NetworkMiddleware{ @@ -1077,6 +1078,15 @@ func Test_mergeIntrospectOptions(t *testing.T) { Options: nil, Expected: IntrospectOptions{}, }, + { + Message: "zero value", + Options: []*IntrospectOptions{ + // Zero values. Was previously supported, so don't break back compatibility. + {}, + {}, + }, + Expected: IntrospectOptions{}, + }, { Message: "full case", Options: []*IntrospectOptions{ @@ -1122,7 +1132,9 @@ func Test_mergeIntrospectOptions(t *testing.T) { }, } for _, row := range table { + row := row // enable parallel sub-tests t.Run(row.Message, func(t *testing.T) { + t.Parallel() opt := mergeIntrospectOptions(row.Options...) assert.Equal(t, row.Expected.client, opt.client) assert.Equal(t, row.Expected.ctx, opt.ctx) @@ -1133,3 +1145,89 @@ func Test_mergeIntrospectOptions(t *testing.T) { }) } } + +// mockJSONErrorQueryer unmarshals the internal JSONResult into the receiver. +// Like mockJSONQueryer but can return failures for X attempts. +type mockJSONErrorQueryer struct { + FailuresRemaining int + FailureErr error + JSONResult string +} + +func (q *mockJSONErrorQueryer) Query(ctx context.Context, input *QueryInput, receiver interface{}) error { + if q.FailuresRemaining > 0 { + q.FailuresRemaining-- + err := q.FailureErr + if err == nil { + err = errors.New("some error") + } + return err + } + return json.Unmarshal([]byte(q.JSONResult), receiver) +} + +func TestIntrospectAPI_retry(t *testing.T) { + t.Parallel() + makeQueryer := func() *mockJSONErrorQueryer { + return &mockJSONErrorQueryer{ + FailureErr: errors.New("foo"), + JSONResult: `{ + "__schema": { + "queryType": { + "name": "Query" + }, + "directives": [ + { + "name": "deprecated", + "args": [ + {"name": "reason"} + ] + } + ] + } + }`, + } + } + + t.Run("no retrier", func(t *testing.T) { + t.Parallel() + queryer := makeQueryer() + queryer.FailuresRemaining = 1 + _, err := IntrospectAPI(queryer) + assert.Zero(t, queryer.FailuresRemaining) + require.EqualError(t, err, "query failed: foo") + assert.ErrorIs(t, err, queryer.FailureErr) + }) + + t.Run("retry more than once", func(t *testing.T) { + t.Parallel() + queryer := makeQueryer() + queryer.FailuresRemaining = 10 + schema, err := IntrospectAPI(queryer, IntrospectWithRetrier(NewCountRetrier(10))) + assert.Zero(t, queryer.FailuresRemaining) + assert.NoError(t, err) + + assert.Equal(t, &ast.Schema{ + Types: map[string]*ast.Definition{}, + Directives: map[string]*ast.DirectiveDefinition{ + "deprecated": { + Name: "deprecated", + Arguments: ast.ArgumentDefinitionList{ + { + Name: "reason", + Type: &ast.Type{ + Position: &ast.Position{}, + }, + }, + }, + Locations: []ast.DirectiveLocation{}, + Position: &ast.Position{ + Src: &ast.Source{BuiltIn: true}, + }, + }, + }, + PossibleTypes: map[string][]*ast.Definition{}, + Implements: map[string][]*ast.Definition{}, + }, schema) + }) +} diff --git a/retrier.go b/retrier.go new file mode 100644 index 0000000..399329f --- /dev/null +++ b/retrier.go @@ -0,0 +1,31 @@ +package graphql + +// Retrier indicates whether or not to retry and attempt another query. +type Retrier interface { + // ShouldRetry returns true if another attempt should run, + // given 'err' from the previous attempt and the total attempt count (starts at 1). + // + // Consider the 'errors' package to unwrap the error. e.g. errors.As(), errors.Is() + ShouldRetry(err error, attempts uint) bool +} + +var _ Retrier = CountRetrier{} + +// CountRetrier is a Retrier that stops after a number of attempts. +type CountRetrier struct { + // maxAttempts is the maximum number of attempts allowed before retries should stop. + // A value of 0 has undefined behavior. + maxAttempts uint +} + +// NewCountRetrier returns a CountRetrier with the given maximum number of retries +// beyond the first attempt. +func NewCountRetrier(maxRetries uint) CountRetrier { + return CountRetrier{ + maxAttempts: 1 + maxRetries, + } +} + +func (c CountRetrier) ShouldRetry(err error, attempts uint) bool { + return attempts < c.maxAttempts +} diff --git a/retrier_test.go b/retrier_test.go new file mode 100644 index 0000000..180cb27 --- /dev/null +++ b/retrier_test.go @@ -0,0 +1,20 @@ +package graphql + +import ( + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" +) + +func TestCountRetrier(t *testing.T) { + t.Parallel() + retrier := NewCountRetrier(1) + someErr := errors.New("some error") + + assert.Equal(t, CountRetrier{ + maxAttempts: 2, + }, retrier) + assert.True(t, retrier.ShouldRetry(someErr, 1)) + assert.False(t, retrier.ShouldRetry(someErr, 2)) +}