Repository: jensneuse/graphql-go-tools (go)

Issue:
proxy.RequestConfigProvider  GetRequestConfig should return error as second return value

Pull request description:
Feature/requestconfigprovider return err
This PR will close #63 
A dynamic RequestConfigProvider might not return a valid RequestConfig.
Therefore the interface was redesigned to return an optional RequestConfig & an error.
The proxy is now able to deal with a failing RequestConfigProvider.

Patch:
diff --git a/hack/fasthttp/fasthttpproxy.go b/hack/fasthttp/fasthttpproxy.go
index 1d70265a..53a95bed 100644
--- a/hack/fasthttp/fasthttpproxy.go
+++ b/hack/fasthttp/fasthttpproxy.go
@@ -9,6 +9,7 @@ import (
 	"github.com/tidwall/sjson"
 	"github.com/valyala/fasthttp"
 	"io"
+	"net/http"
 )
 
 type Proxy proxy.Proxy
@@ -38,7 +39,12 @@ func (f *Proxy) RewriteQuery(config proxy.RequestConfig, ctx context.Context, re
 
 func (f *Proxy) HandleRequest(ctx *fasthttp.RequestCtx) {
 
-	config := f.RequestConfigProvider.GetRequestConfig(ctx)
+	config, err := f.RequestConfigProvider.GetRequestConfig(ctx)
+	if err != nil {
+		ctx.Error(err.Error(), http.StatusInternalServerError)
+		return
+	}
+
 	goctx := f.SetContextValues(ctx, &ctx.Request.Header, config.AddHeadersToContext)
 
 	body := ctx.Request.Body()
@@ -58,7 +64,7 @@ func (f *Proxy) HandleRequest(ctx *fasthttp.RequestCtx) {
 	buff.Reset()
 	defer f.BufferPool.Put(buff)
 
-	err := f.RewriteQuery(config, goctx, ctx.RequestURI(), query, buff)
+	err = f.RewriteQuery(*config, goctx, ctx.RequestURI(), query, buff)
 	if err != nil {
 		ctx.Error(err.Error(), fasthttp.StatusInternalServerError)
 		return
diff --git a/pkg/proxy/config.go b/pkg/proxy/config.go
index e207acc3..b1907b8a 100644
--- a/pkg/proxy/config.go
+++ b/pkg/proxy/config.go
@@ -13,7 +13,7 @@ import (
 // After that it's up to the RequestConfigProvider implementation how to set the configuration
 // This should give the user enough flexibility
 type RequestConfigProvider interface {
-	GetRequestConfig(ctx context.Context) RequestConfig
+	GetRequestConfig(ctx context.Context) (*RequestConfig, error)
 }
 
 // RequestConfig configures how the proxy should handle a request
@@ -34,8 +34,8 @@ type StaticRequestConfigProvider struct {
 	config RequestConfig
 }
 
-func (s *StaticRequestConfigProvider) GetRequestConfig(ctx context.Context) RequestConfig {
-	return s.config
+func (s *StaticRequestConfigProvider) GetRequestConfig(ctx context.Context) (*RequestConfig, error) {
+	return &s.config, nil
 }
 
 func NewStaticRequestConfigProvider(config RequestConfig) *StaticRequestConfigProvider {
diff --git a/pkg/proxy/http/proxy.go b/pkg/proxy/http/proxy.go
index 20f864ae..61ae8376 100644
--- a/pkg/proxy/http/proxy.go
+++ b/pkg/proxy/http/proxy.go
@@ -96,16 +96,22 @@ func (p *Proxy) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	buff := p.BufferPool.Get().(*bytes.Buffer)
 	buff.Reset()
 
-	config := p.RequestConfigProvider.GetRequestConfig(r.Context())
+	config, err := p.RequestConfigProvider.GetRequestConfig(r.Context())
+	if err != nil {
+		p.BufferPool.Put(buff)
+		p.HandleError(err, w)
+		return
+	}
+
 	pr := ProxyRequest{
 		Proxy: p,
 	}
-	pr.Config = &config
+	pr.Config = config
 	pr.RequestURL = *r.URL
 	pr.Body = r.Body
 	pr.Context = p.SetContextValues(r.Context(), r.Header, config.AddHeadersToContext)
 
-	err := json.NewDecoder(pr.Body).Decode(&pr.GraphQLRequest)
+	err = json.NewDecoder(pr.Body).Decode(&pr.GraphQLRequest)
 	if err != nil {
 		p.BufferPool.Put(buff)
 		p.HandleError(err, w)


Test patch:
diff --git a/pkg/proxy/http/proxy_test.go b/pkg/proxy/http/proxy_test.go
index a86fa291..4db6ab74 100644
--- a/pkg/proxy/http/proxy_test.go
+++ b/pkg/proxy/http/proxy_test.go
@@ -2,6 +2,7 @@ package http
 
 import (
 	"context"
+	"errors"
 	hackmiddleware "github.com/jensneuse/graphql-go-tools/hack/middleware"
 	"github.com/jensneuse/graphql-go-tools/pkg/middleware"
 	"github.com/jensneuse/graphql-go-tools/pkg/proxy"
@@ -41,6 +42,8 @@ type ProxyTestCase struct {
 	BackendOnBeforeRequestMiddleware HttpMiddleware
 	// BackendHeaders are the headers that should be statically set on requests to the backend
 	BackendHeaders map[string][]string
+	// RequestConfigProviderFactory if set (optional) could override the request config provider
+	RequestConfigProviderFactory func(config proxy.RequestConfig) proxy.RequestConfigProvider
 }
 
 /* HttpMiddleware wraps a http handler to add additional logic for certain tests
@@ -107,6 +110,18 @@ func TestProxy(t *testing.T) {
 			WantProxyErrorHandlerInvocation: false,
 		})
 	})
+	t.Run("failing request config provider", func(t *testing.T) {
+		RunTestCase(t, ProxyTestCase{
+			Schema:                          assetSchema,
+			ClientRequest:                   variableAssetInput,
+			ExpectedProxiedRequest:          variableAssetOutput,
+			WantClientResponseStatusCode:    http.StatusOK,
+			WantProxyErrorHandlerInvocation: true,
+			RequestConfigProviderFactory: func(config proxy.RequestConfig) proxy.RequestConfigProvider {
+				return failingRequestConfigProvider{}
+			},
+		})
+	})
 	t.Run("handle request response e2e", func(t *testing.T) {
 		RunTestCase(t, ProxyTestCase{
 			Schema: publicSchema,
@@ -195,7 +210,12 @@ func RunTestCase(t *testing.T, testCase ProxyTestCase) {
 		BackendHeaders: testCase.BackendHeaders,
 	}
 
-	requestConfigProvider := proxy.NewStaticRequestConfigProvider(requestConfig)
+	var requestConfigProvider proxy.RequestConfigProvider
+	if testCase.RequestConfigProviderFactory != nil {
+		requestConfigProvider = testCase.RequestConfigProviderFactory(requestConfig)
+	} else {
+		requestConfigProvider = proxy.NewStaticRequestConfigProvider(requestConfig)
+	}
 
 	graphqlProxy := NewDefaultProxy(requestConfigProvider, testCase.MiddleWares...)
 
@@ -335,3 +355,11 @@ type Document implements Node {
 	userKey           = "user"
 	userValue         = "jsmith@example.org"
 )
+
+// failing request config provider
+
+type failingRequestConfigProvider struct{}
+
+func (failingRequestConfigProvider) GetRequestConfig(ctx context.Context) (*proxy.RequestConfig, error) {
+	return nil, errors.New("failing")
+}