The client.do (unexported) function should create a new request

[cyberhck]

Currently, in the client.Post method, we create a request, then run the request through interceptor chain, if one interceptor were to change GQLRequestInfo, it won't get reflected.

If we moved the logic of creating the body of the request from client.Post to client.do function, then it'd be possible to change the GQLRequestInfo as well as it'd make retrying much more easy since for every chained intercept calls, it'd create a new request.

This currently won't work:

graphqlClient := graphql.NewClient(http.DefaultClient, "https://service-name.namespace.svc.cluster.local", &clientv2.Options{}, func(ctx context.Context, req *http.Request, gqlInfo *clientv2.GQLRequestInfo, res interface{}, next clientv2.RequestInterceptorFunc) error {
			gqlInfo.Request.OperationName = "myOperationName"

			return backoff.Retry(func() error {
				return next(ctx, req, gqlInfo, res)
			}, backoff.NewConstantBackOff(time.Millisecond*100))
		})

In the above example, neither the retry works, nor the operation name.

If we moved the logic of creating the body from client.Post to client.do function it'd work.

I know we can use a custom do function, but that's a little complicated since we generate code in a repository that's trigerred from another repository and we use the interface name generation, which means we do not have the access to the clientv2.Client either.

I'm willing to send a PR if you'd accept this.

[Yamashou]

Thanks a issues.
I don't intend to overwrite the contents of gqlInfo. I intend to perform additional processing based on the contents.

[cyberhck]

I don't understand what you mean, you don't intend to overwrite the contents of gqlinfo. But we as users might want to override.

We also want to be able to retry easily. This wouldn't be a breaking change, just a new feature.

Actually as long as I can call next multiple times, we're fine, but looks like moving all that logic to do function seems the cleanest way. What do you think?

[Yamashou]

I am using a retry mechanism similar to the implementation below, and retries are occurring without any issues. In cases where retries are not happening, even if you implement it straightforwardly using a for loop, wouldn’t it be called multiple times?

I prefer not to overwrite gqlInfo because doing so would mean the behavior changes depending on the order of interceptors, due to whether the information is overwritten or not. As a result, interceptors that were previously working correctly might stop functioning properly due to the influence of other interceptors.

func RetryInterceptor(ctx context.Context, req *http.Request, gqlInfo *clientv2.GQLRequestInfo, res any, next clientv2.RequestInterceptorFunc) error {
	err := next(ctx, req, gqlInfo, res)
	if err == nil {
		return nil
	}

	var gqlError *clientv2.ErrorResponse
	if errors.As(err, &gqlError) {
		if gqlError.NetworkError != nil && (gqlError.NetworkError.Code >= 500 || gqlError.NetworkError.Code == 400) {
			time.Sleep(time.Second)
			err = next(ctx, req, gqlInfo, res)
		}
	}

	return xerrors.Errorf(": %w", err)
}

[Yamashou]

@cyberhck
Hi! Does this Pull Request solve your problem?
#244

[cyberhck]

I think it looks good, thanks.