diff --git a/v2/pkg/engine/datasource/httpclient/nethttpclient.go b/v2/pkg/engine/datasource/httpclient/nethttpclient.go index 53789718f3..bf154abdce 100644 --- a/v2/pkg/engine/datasource/httpclient/nethttpclient.go +++ b/v2/pkg/engine/datasource/httpclient/nethttpclient.go @@ -18,7 +18,7 @@ import ( "time" "github.com/buger/jsonparser" - + pkgErrors "github.com/pkg/errors" "github.com/wundergraph/graphql-go-tools/v2/pkg/lexer/literal" "github.com/wundergraph/graphql-go-tools/v2/pkg/pool" ) @@ -204,7 +204,7 @@ func makeHTTPRequest(client *http.Client, ctx context.Context, url, method, head response, err := client.Do(request) if err != nil { - return err + return pkgErrors.WithStack(err) } defer response.Body.Close() @@ -262,6 +262,7 @@ func Do(client *http.Client, ctx context.Context, requestInput []byte, out *byte pool.Hash64.Put(h) ctx = context.WithValue(ctx, bodyHashContextKey{}, bodyHash) return makeHTTPRequest(client, ctx, url, method, headers, queryParams, bytes.NewReader(body), enableTrace, out, ContentTypeJSON) + } func DoMultipartForm( diff --git a/v2/pkg/engine/resolve/authorization_test.go b/v2/pkg/engine/resolve/authorization_test.go index 01ed1c02d8..bbd0de4111 100644 --- a/v2/pkg/engine/resolve/authorization_test.go +++ b/v2/pkg/engine/resolve/authorization_test.go @@ -72,7 +72,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(2), authorizer.(*testAuthorizer).preFetchCalls.Load()) assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) - require.Nil(t, resolveCtx.subgraphErrors) + require.Nil(t, resolveCtx.error) } })) t.Run("validate authorizer args", testFnWithPostEvaluation(func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx *Context, expectedOutput string, postEvaluation func(t *testing.T)) { @@ -139,7 +139,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(2), authorizer.(*testAuthorizer).preFetchCalls.Load()) assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) - require.Nil(t, resolveCtx.subgraphErrors) + require.Nil(t, resolveCtx.error) } })) t.Run("disallow field with extension", testFnWithPostEvaluation(func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx *Context, expectedOutput string, postEvaluation func(t *testing.T)) { @@ -167,7 +167,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(2), authorizer.(*testAuthorizer).preFetchCalls.Load()) assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) - require.Nil(t, resolveCtx.subgraphErrors) + require.Nil(t, resolveCtx.error) } })) t.Run("no authorization rules/checks", testFnWithPostEvaluation(func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx *Context, expectedOutput string, postEvaluation func(t *testing.T)) { @@ -187,7 +187,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(0), authorizer.(*testAuthorizer).preFetchCalls.Load()) assert.Equal(t, int64(0), authorizer.(*testAuthorizer).objectFieldCalls.Load()) - require.Nil(t, resolveCtx.subgraphErrors) + require.Nil(t, resolveCtx.error) } })) t.Run("disallow root fetch", testFnWithPostEvaluation(func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx *Context, expectedOutput string, postEvaluation func(t *testing.T)) { @@ -213,7 +213,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(0), authorizer.(*testAuthorizer).objectFieldCalls.Load()) var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "users", subgraphError.DataSourceInfo.Name) require.Equal(t, "query", subgraphError.Path) require.Equal(t, "Not allowed to fetch from users Subgraph", subgraphError.Reason) @@ -244,7 +244,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(0), authorizer.(*testAuthorizer).objectFieldCalls.Load()) var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "users", subgraphError.DataSourceInfo.Name) require.Equal(t, "query", subgraphError.Path) require.Equal(t, "", subgraphError.Reason) @@ -276,11 +276,11 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(2), authorizer.(*testAuthorizer).preFetchCalls.Load()) assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) - require.NotEmpty(t, resolveCtx.subgraphErrors) - require.EqualError(t, resolveCtx.subgraphErrors, "Failed to fetch from Subgraph 'products' at Path: 'query.me.reviews.@.product', Reason: Not allowed to fetch from products Subgraph.") + require.NotEmpty(t, resolveCtx.error) + require.EqualError(t, resolveCtx.error, "Failed to fetch from Subgraph 'products' at Path: 'query.me.reviews.@.product', Reason: Not allowed to fetch from products Subgraph.") var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "products", subgraphError.DataSourceInfo.Name) require.Equal(t, "query.me.reviews.@.product", subgraphError.Path) require.Equal(t, "Not allowed to fetch from products Subgraph", subgraphError.Reason) @@ -311,7 +311,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "products", subgraphError.DataSourceInfo.Name) require.Equal(t, "Query.me.reviews.product.data.name", subgraphError.Path) require.Equal(t, "Not allowed to fetch name on Product", subgraphError.Reason) @@ -344,7 +344,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "products", subgraphError.DataSourceInfo.Name) require.Equal(t, "query.me.reviews.@.product", subgraphError.Path) require.Equal(t, "Not allowed to fetch from products Subgraph", subgraphError.Reason) @@ -388,7 +388,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "reviews", subgraphError.DataSourceInfo.Name) require.Equal(t, "Query.me.reviews.body", subgraphError.Path) require.Equal(t, "Not allowed to fetch body on Review", subgraphError.Reason) @@ -417,7 +417,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "reviews", subgraphError.DataSourceInfo.Name) require.Equal(t, "Query.me.reviews.body", subgraphError.Path) require.Equal(t, "", subgraphError.Reason) @@ -448,7 +448,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "products", subgraphError.DataSourceInfo.Name) require.Equal(t, "query.me.reviews.@.product", subgraphError.Path) require.Equal(t, "Not allowed to fetch name on Product", subgraphError.Reason) @@ -479,7 +479,7 @@ func TestAuthorization(t *testing.T) { assert.Equal(t, int64(4), authorizer.(*testAuthorizer).objectFieldCalls.Load()) var subgraphError *SubgraphError - require.ErrorAs(t, resolveCtx.subgraphErrors, &subgraphError) + require.ErrorAs(t, resolveCtx.error, &subgraphError) require.Equal(t, "products", subgraphError.DataSourceInfo.Name) require.Equal(t, "Query.me.reviews.product.data.name", subgraphError.Path) require.Equal(t, "Not allowed to fetch name on Product", subgraphError.Reason) diff --git a/v2/pkg/engine/resolve/context.go b/v2/pkg/engine/resolve/context.go index a624792d2c..bec1506320 100644 --- a/v2/pkg/engine/resolve/context.go +++ b/v2/pkg/engine/resolve/context.go @@ -30,7 +30,7 @@ type Context struct { authorizer Authorizer rateLimiter RateLimiter - subgraphErrors error + error error } type ExecutionOptions struct { @@ -103,12 +103,15 @@ func (c *Context) SetRateLimiter(limiter RateLimiter) { c.rateLimiter = limiter } -func (c *Context) SubgraphErrors() error { - return c.subgraphErrors +// ExecutionError returns the error that occurred during execution. +// You can use appendError to append errors to the context e.g. from a subgraph +func (c *Context) ExecutionError() error { + return c.error } -func (c *Context) appendSubgraphError(err error) { - c.subgraphErrors = errors.Join(c.subgraphErrors, err) +// appendError appends the error to the context's error field. Should only be used to make the error public +func (c *Context) appendError(err error) { + c.error = errors.Join(c.error, err) } type Request struct { @@ -168,7 +171,7 @@ func (c *Context) Free() { c.RemapVariables = nil c.TracingOptions.DisableAll() c.Extensions = nil - c.subgraphErrors = nil + c.error = nil c.authorizer = nil c.LoaderHooks = nil } diff --git a/v2/pkg/engine/resolve/loader.go b/v2/pkg/engine/resolve/loader.go index 4b1a5e7829..19fc6345f3 100644 --- a/v2/pkg/engine/resolve/loader.go +++ b/v2/pkg/engine/resolve/loader.go @@ -5,9 +5,7 @@ import ( "context" "crypto/tls" "encoding/json" - goerrors "errors" "fmt" - "github.com/wundergraph/graphql-go-tools/v2/pkg/errorcodes" "net/http" "net/http/httptrace" "slices" @@ -16,6 +14,8 @@ import ( "sync" "time" + "github.com/wundergraph/graphql-go-tools/v2/pkg/errorcodes" + "github.com/buger/jsonparser" "github.com/cespare/xxhash/v2" "github.com/pkg/errors" @@ -57,8 +57,8 @@ type ResponseInfo struct { ResponseHeaders http.Header } -func newResponseInfo(res *result, subgraphError error) *ResponseInfo { - responseInfo := &ResponseInfo{StatusCode: res.statusCode, Err: goerrors.Join(res.err, subgraphError)} +func newResponseInfo(res *result, err error) *ResponseInfo { + responseInfo := &ResponseInfo{StatusCode: res.statusCode, Err: err} if res.httpResponseContext != nil { // We're using the response.Request here, because the body will be nil (since the response was read) and won't // cause a memory leak. @@ -197,7 +197,7 @@ func (l *Loader) resolveParallel(nodes []*FetchTreeNode) error { if l.ctx.LoaderHooks != nil && results[i].nestedMergeItems[j].loaderHookContext != nil { l.ctx.LoaderHooks.OnFinished(results[i].nestedMergeItems[j].loaderHookContext, results[i].nestedMergeItems[j].ds, - newResponseInfo(results[i].nestedMergeItems[j], l.ctx.subgraphErrors)) + newResponseInfo(results[i].nestedMergeItems[j], l.ctx.error)) } if err != nil { return errors.WithStack(err) @@ -206,7 +206,7 @@ func (l *Loader) resolveParallel(nodes []*FetchTreeNode) error { } else { err = l.mergeResult(nodes[i].Item, results[i], itemsItems[i]) if l.ctx.LoaderHooks != nil { - l.ctx.LoaderHooks.OnFinished(results[i].loaderHookContext, results[i].ds, newResponseInfo(results[i], l.ctx.subgraphErrors)) + l.ctx.LoaderHooks.OnFinished(results[i].loaderHookContext, results[i].ds, newResponseInfo(results[i], l.ctx.error)) } if err != nil { return errors.WithStack(err) @@ -242,8 +242,12 @@ func (l *Loader) resolveSingle(item *FetchItem) error { return err } err = l.mergeResult(item, res, items) + + if res.err != nil { + l.ctx.appendError(res.err) + } if l.ctx.LoaderHooks != nil { - l.ctx.LoaderHooks.OnFinished(res.loaderHookContext, res.ds, newResponseInfo(res, l.ctx.subgraphErrors)) + l.ctx.LoaderHooks.OnFinished(res.loaderHookContext, res.ds, newResponseInfo(res, l.ctx.error)) } return err @@ -256,9 +260,14 @@ func (l *Loader) resolveSingle(item *FetchItem) error { return errors.WithStack(err) } err = l.mergeResult(item, res, items) + + if res.err != nil { + l.ctx.appendError(res.err) + } if l.ctx.LoaderHooks != nil { - l.ctx.LoaderHooks.OnFinished(res.loaderHookContext, res.ds, newResponseInfo(res, l.ctx.subgraphErrors)) + l.ctx.LoaderHooks.OnFinished(res.loaderHookContext, res.ds, newResponseInfo(res, l.ctx.error)) } + return err case *EntityFetch: res := &result{ @@ -269,9 +278,14 @@ func (l *Loader) resolveSingle(item *FetchItem) error { return errors.WithStack(err) } err = l.mergeResult(item, res, items) + if res.err != nil { + l.ctx.appendError(res.err) + } + if l.ctx.LoaderHooks != nil { - l.ctx.LoaderHooks.OnFinished(res.loaderHookContext, res.ds, newResponseInfo(res, l.ctx.subgraphErrors)) + l.ctx.LoaderHooks.OnFinished(res.loaderHookContext, res.ds, newResponseInfo(res, l.ctx.error)) } + return err case *ParallelListItemFetch: results := make([]*result, len(items)) @@ -302,9 +316,14 @@ func (l *Loader) resolveSingle(item *FetchItem) error { } for i := range results { err = l.mergeResult(item, results[i], items[i:i+1]) + + if results[i].err != nil { + l.ctx.appendError(results[i].err) + } if l.ctx.LoaderHooks != nil { - l.ctx.LoaderHooks.OnFinished(results[i].loaderHookContext, results[i].ds, newResponseInfo(results[i], l.ctx.subgraphErrors)) + l.ctx.LoaderHooks.OnFinished(results[i].loaderHookContext, results[i].ds, newResponseInfo(results[i], l.ctx.error)) } + if err != nil { return errors.WithStack(err) } @@ -693,7 +712,7 @@ func (l *Loader) appendSubgraphError(res *result, fetchItem *FetchItem, value *a subgraphError.AppendDownstreamError(&gErr) } - l.ctx.appendSubgraphError(goerrors.Join(res.err, subgraphError)) + l.ctx.appendError(subgraphError) return nil } @@ -1006,7 +1025,8 @@ func (l *Loader) addApolloRouterCompatibilityError(res *result) error { } func (l *Loader) renderErrorsFailedToFetch(fetchItem *FetchItem, res *result, reason string) error { - l.ctx.appendSubgraphError(goerrors.Join(res.err, NewSubgraphError(res.ds, fetchItem.ResponsePath, reason, res.statusCode))) + l.ctx.appendError(NewSubgraphError(res.ds, fetchItem.ResponsePath, reason, res.statusCode)) + errorObject, err := astjson.ParseWithoutCache(l.renderSubgraphBaseError(res.ds, fetchItem.ResponsePath, reason)) if err != nil { return err @@ -1032,7 +1052,7 @@ func (l *Loader) renderSubgraphBaseError(ds DataSourceInfo, path, reason string) func (l *Loader) renderAuthorizationRejectedErrors(fetchItem *FetchItem, res *result) error { for i := range res.authorizationRejectedReasons { - l.ctx.appendSubgraphError(goerrors.Join(res.err, NewSubgraphError(res.ds, fetchItem.ResponsePath, res.authorizationRejectedReasons[i], res.statusCode))) + l.ctx.appendError(NewSubgraphError(res.ds, fetchItem.ResponsePath, res.authorizationRejectedReasons[i], res.statusCode)) } pathPart := l.renderAtPathErrorPart(fetchItem.ResponsePath) extensionErrorCode := fmt.Sprintf(`"extensions":{"code":"%s"}`, errorcodes.UnauthorizedFieldOrType) @@ -1073,7 +1093,7 @@ func (l *Loader) renderAuthorizationRejectedErrors(fetchItem *FetchItem, res *re } func (l *Loader) renderRateLimitRejectedErrors(fetchItem *FetchItem, res *result) error { - l.ctx.appendSubgraphError(goerrors.Join(res.err, NewRateLimitError(res.ds.Name, fetchItem.ResponsePath, res.rateLimitRejectedReason))) + l.ctx.appendError(NewRateLimitError(res.ds.Name, fetchItem.ResponsePath, res.rateLimitRejectedReason)) pathPart := l.renderAtPathErrorPart(fetchItem.ResponsePath) var ( err error diff --git a/v2/pkg/engine/resolve/loader_hooks_test.go b/v2/pkg/engine/resolve/loader_hooks_test.go index 21aea561f6..22180fc183 100644 --- a/v2/pkg/engine/resolve/loader_hooks_test.go +++ b/v2/pkg/engine/resolve/loader_hooks_test.go @@ -105,7 +105,7 @@ func TestLoaderHooks_FetchPipeline(t *testing.T) { assert.Equal(t, "errorMessage", subgraphError.DownstreamErrors[0].Message) assert.Nil(t, subgraphError.DownstreamErrors[0].Extensions) - assert.NotNil(t, resolveCtx.SubgraphErrors()) + assert.NotNil(t, resolveCtx.ExecutionError()) } })) @@ -185,7 +185,7 @@ func TestLoaderHooks_FetchPipeline(t *testing.T) { assert.Equal(t, "errorMessage", subgraphError.DownstreamErrors[0].Message) assert.Nil(t, subgraphError.DownstreamErrors[0].Extensions) - assert.NotNil(t, resolveCtx.SubgraphErrors()) + assert.NotNil(t, resolveCtx.ExecutionError()) }) t.Run("parallel fetch with simple subgraph error", testFnWithPostEvaluation(func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx *Context, expectedOutput string, postEvaluation func(t *testing.T)) { @@ -249,7 +249,7 @@ func TestLoaderHooks_FetchPipeline(t *testing.T) { assert.Equal(t, "errorMessage", subgraphError.DownstreamErrors[0].Message) assert.Nil(t, subgraphError.DownstreamErrors[0].Extensions) - assert.NotNil(t, resolveCtx.SubgraphErrors()) + assert.NotNil(t, resolveCtx.ExecutionError()) } })) @@ -314,7 +314,7 @@ func TestLoaderHooks_FetchPipeline(t *testing.T) { assert.Equal(t, "errorMessage", subgraphError.DownstreamErrors[0].Message) assert.Nil(t, subgraphError.DownstreamErrors[0].Extensions) - assert.NotNil(t, resolveCtx.SubgraphErrors()) + assert.NotNil(t, resolveCtx.ExecutionError()) } })) @@ -380,7 +380,7 @@ func TestLoaderHooks_FetchPipeline(t *testing.T) { assert.Equal(t, "errorMessage2", subgraphError.DownstreamErrors[1].Message) assert.Empty(t, subgraphError.DownstreamErrors[1].Extensions["code"]) - assert.NotNil(t, resolveCtx.SubgraphErrors()) + assert.NotNil(t, resolveCtx.ExecutionError()) } })) diff --git a/v2/pkg/engine/resolve/resolvable.go b/v2/pkg/engine/resolve/resolvable.go index 4783c68207..b3b568f3ec 100644 --- a/v2/pkg/engine/resolve/resolvable.go +++ b/v2/pkg/engine/resolve/resolvable.go @@ -703,7 +703,7 @@ func (r *Resolvable) addRejectFieldError(reason string, ds DataSourceInfo, field } else { errorMessage = fmt.Sprintf("Unauthorized to load field '%s', Reason: %s.", fieldPath, reason) } - r.ctx.appendSubgraphError(goerrors.Join(errors.New(errorMessage), + r.ctx.appendError(goerrors.Join(errors.New(errorMessage), NewSubgraphError(ds, fieldPath, reason, 0))) fastjsonext.AppendErrorWithExtensionsCodeToArray(r.astjsonArena, r.errors, errorMessage, errorcodes.UnauthorizedFieldOrType, r.path) r.popNodePathElement(nodePath)