Skip to content

Commit

Permalink
feat: improve error reporting for subgraphs (#705)
Browse files Browse the repository at this point in the history
  • Loading branch information
jensneuse authored Dec 18, 2023
1 parent fca783c commit 92f315e
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 40 deletions.
6 changes: 4 additions & 2 deletions v2/examples/federation/go.mod
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
module github.com/wundergraph/graphql-go-tools/v2/examples/federation

go 1.18
go 1.21

toolchain go1.21.0

require (
github.com/99designs/gqlgen v0.17.22
Expand All @@ -24,7 +26,7 @@ require (
github.com/eclipse/paho.mqtt.golang v1.2.0 // indirect
github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee // indirect
github.com/gobwas/pool v0.2.0 // indirect
github.com/google/uuid v1.1.1 // indirect
github.com/google/uuid v1.4.0 // indirect
github.com/hashicorp/golang-lru v0.5.4 // indirect
github.com/huandu/xstrings v1.2.1 // indirect
github.com/imdario/mergo v0.3.8 // indirect
Expand Down
1 change: 1 addition & 0 deletions v2/examples/federation/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/
github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
github.com/google/uuid v1.1.1 h1:Gkbcsh/GbpXz7lPftLA3P6TYMwjCLYm83jiFQZF/3gY=
github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/google/uuid v1.4.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
Expand Down
2 changes: 2 additions & 0 deletions v2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ require (
github.com/agnivade/levenshtein v1.1.1 // indirect
github.com/gobwas/httphead v0.0.0-20180130184737-2c6c146eadee // indirect
github.com/gobwas/pool v0.2.0 // indirect
github.com/hashicorp/errwrap v1.0.0 // indirect
github.com/hashicorp/go-multierror v1.1.1 // indirect
github.com/klauspost/compress v1.14.4 // indirect
github.com/kr/pretty v0.3.1 // indirect
github.com/logrusorgru/aurora/v3 v3.0.0 // indirect
Expand Down
4 changes: 4 additions & 0 deletions v2/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ github.com/google/uuid v1.4.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+
github.com/gorilla/websocket v1.4.1/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA=
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc=
github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4=
github.com/jensneuse/abstractlogger v0.0.4 h1:sa4EH8fhWk3zlTDbSncaWKfwxYM8tYSlQ054ETLyyQY=
Expand Down
12 changes: 12 additions & 0 deletions v2/pkg/engine/resolve/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"net/http"
"time"

"github.com/hashicorp/go-multierror"
"go.uber.org/atomic"
)

Expand All @@ -17,6 +18,16 @@ type Context struct {
InitialPayload []byte
Extensions []byte
Stats Stats

subgraphErrors error
}

func (c *Context) SubgraphErrors() error {
return c.subgraphErrors
}

func (c *Context) appendSubgraphError(err error) {
c.subgraphErrors = multierror.Append(c.subgraphErrors, err)
}

type Stats struct {
Expand Down Expand Up @@ -78,6 +89,7 @@ func (c *Context) Free() {
c.RequestTracingOptions.DisableAll()
c.Extensions = nil
c.Stats.Reset()
c.subgraphErrors = nil
}

type traceStartKey struct{}
Expand Down
79 changes: 42 additions & 37 deletions v2/pkg/engine/resolve/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ func (l *Loader) renderPath() string {
builder.WriteString("mutation")
case ast.OperationTypeSubscription:
builder.WriteString("subscription")
case ast.OperationTypeUnknown:
}
}
if len(l.path) == 0 {
Expand Down Expand Up @@ -342,7 +343,10 @@ func (l *Loader) mergeErrors(ref int) {

func (l *Loader) mergeResult(res *result, items []int) error {
defer pool.BytesBuffer.Put(res.out)
if res.fetchAborted {
if res.err != nil {
return l.renderErrorsFailedToFetch(res)
}
if res.fetchSkipped {
return nil
}
if res.out.Len() == 0 {
Expand Down Expand Up @@ -452,8 +456,18 @@ type result struct {
postProcessing PostProcessingConfiguration
out *bytes.Buffer
batchStats [][]int
fetchAborted bool
fetchSkipped bool
nestedMergeItems []*result

err error
subgraphName string
}

func (r *result) init(postProcessing PostProcessingConfiguration, info *FetchInfo) {
r.postProcessing = postProcessing
if info != nil {
r.subgraphName = info.DataSourceID
}
}

var (
Expand All @@ -475,26 +489,27 @@ func (l *Loader) renderErrorsInvalidInput(out *bytes.Buffer) error {
return nil
}

var (
errorsFailedToFetchHeader = []byte(`{"errors":[{"message":"failed to fetch","path":[`)
errorsFailedToFetchFooter = []byte(`]}]}`)
)

func (l *Loader) renderErrorsFailedToFetch(out *bytes.Buffer) error {
_, _ = out.Write(errorsFailedToFetchHeader)
for i := range l.path {
if i != 0 {
_, _ = out.Write(comma)
func (l *Loader) renderErrorsFailedToFetch(res *result) error {
path := l.renderPath()
l.ctx.appendSubgraphError(errors.Wrap(res.err, fmt.Sprintf("failed to fetch from subgraph '%s' at path '%s'", res.subgraphName, path)))
if res.subgraphName == "" {
errorObject, err := l.data.AppendObject([]byte(fmt.Sprintf(`{"message":"Failed to fetch from Subgraph at path '%s'."}`, path)))
if err != nil {
return errors.WithStack(err)
}
_, _ = out.Write(quote)
_, _ = out.WriteString(l.path[i])
_, _ = out.Write(quote)
l.data.Nodes[l.errorsRoot].ArrayValues = append(l.data.Nodes[l.errorsRoot].ArrayValues, errorObject)
} else {
errorObject, err := l.data.AppendObject([]byte(fmt.Sprintf(`{"message":"Failed to fetch from Subgraph '%s' at path '%s'."}`, res.subgraphName, path)))
if err != nil {
return errors.WithStack(err)
}
l.data.Nodes[l.errorsRoot].ArrayValues = append(l.data.Nodes[l.errorsRoot].ArrayValues, errorObject)
}
_, _ = out.Write(errorsFailedToFetchFooter)
return nil
}

func (l *Loader) loadSingleFetch(ctx context.Context, fetch *SingleFetch, items []int, res *result) error {
res.init(fetch.PostProcessing, fetch.Info)
input := pool.BytesBuffer.Get()
defer pool.BytesBuffer.Put(input)
preparedInput := pool.BytesBuffer.Get()
Expand All @@ -515,15 +530,12 @@ func (l *Loader) loadSingleFetch(ctx context.Context, fetch *SingleFetch, items
if err != nil {
return l.renderErrorsInvalidInput(res.out)
}
err = l.executeSourceLoad(ctx, fetch.DataSource, preparedInput.Bytes(), res.out, fetch.Trace)
if err != nil {
return l.renderErrorsFailedToFetch(res.out)
}
res.postProcessing = fetch.PostProcessing
res.err = l.executeSourceLoad(ctx, fetch.DataSource, preparedInput.Bytes(), res.out, fetch.Trace)
return nil
}

func (l *Loader) loadEntityFetch(ctx context.Context, fetch *EntityFetch, items []int, res *result) error {
res.init(fetch.PostProcessing, fetch.Info)
itemData := pool.BytesBuffer.Get()
defer pool.BytesBuffer.Put(itemData)
preparedInput := pool.BytesBuffer.Get()
Expand Down Expand Up @@ -566,15 +578,15 @@ func (l *Loader) loadEntityFetch(ctx context.Context, fetch *EntityFetch, items
renderedItem := item.Bytes()
if bytes.Equal(renderedItem, null) {
// skip fetch if item is null
res.fetchAborted = true
res.fetchSkipped = true
if l.traceOptions.Enable {
fetch.Trace.LoadSkipped = true
}
return nil
}
if bytes.Equal(renderedItem, emptyObject) {
// skip fetch if item is empty
res.fetchAborted = true
res.fetchSkipped = true
if l.traceOptions.Enable {
fetch.Trace.LoadSkipped = true
}
Expand All @@ -590,17 +602,13 @@ func (l *Loader) loadEntityFetch(ctx context.Context, fetch *EntityFetch, items
if err != nil {
return errors.WithStack(err)
}

err = l.executeSourceLoad(ctx, fetch.DataSource, preparedInput.Bytes(), res.out, fetch.Trace)
if err != nil {
return errors.WithStack(err)
}
res.postProcessing = fetch.PostProcessing
res.err = l.executeSourceLoad(ctx, fetch.DataSource, preparedInput.Bytes(), res.out, fetch.Trace)
return nil
}

func (l *Loader) loadBatchEntityFetch(ctx context.Context, fetch *BatchEntityFetch, items []int, res *result) error {
res.postProcessing = fetch.PostProcessing
res.init(fetch.PostProcessing, fetch.Info)

if l.traceOptions.Enable {
fetch.Trace = &DataSourceLoadTrace{}
if !l.traceOptions.ExcludeRawInputData {
Expand Down Expand Up @@ -691,7 +699,7 @@ WithNextItem:

if len(itemHashes) == 0 {
// all items were skipped - discard fetch
res.fetchAborted = true
res.fetchSkipped = true
if l.traceOptions.Enable {
fetch.Trace.LoadSkipped = true
}
Expand All @@ -707,10 +715,7 @@ WithNextItem:
if err != nil {
return errors.WithStack(err)
}
err = l.executeSourceLoad(ctx, fetch.DataSource, preparedInput.Bytes(), res.out, fetch.Trace)
if err != nil {
return errors.WithStack(err)
}
res.err = l.executeSourceLoad(ctx, fetch.DataSource, preparedInput.Bytes(), res.out, fetch.Trace)
return nil
}

Expand Down Expand Up @@ -916,13 +921,13 @@ func (l *Loader) executeSourceLoad(ctx context.Context, source DataSource, input
trace.DurationLoadPretty = time.Duration(trace.DurationLoadNano).String()
}
}
l.ctx.Stats.NumberOfFetches.Inc()
l.ctx.Stats.CombinedResponseSize.Add(int64(out.Len()))
if err != nil {
if l.traceOptions.Enable {
trace.LoadError = err.Error()
}
return errors.WithStack(err)
}
l.ctx.Stats.NumberOfFetches.Inc()
l.ctx.Stats.CombinedResponseSize.Add(int64(out.Len()))
return nil
}
64 changes: 64 additions & 0 deletions v2/pkg/engine/resolve/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"errors"
"fmt"
"io"
"net"
"net/http"
"sync"
"sync/atomic"
Expand Down Expand Up @@ -1682,6 +1683,69 @@ func TestResolver_ResolveGraphQLResponse(t *testing.T) {
},
}, Context{ctx: context.Background()}, `{"errors":[{"message":"errorMessage"}],"data":{"name":null}}`
}))
t.Run("fetch with returned err", testFn(true, func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx Context, expectedOutput string) {
mockDataSource := NewMockDataSource(ctrl)
mockDataSource.EXPECT().
Load(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&bytes.Buffer{})).
DoAndReturn(func(ctx context.Context, input []byte, w io.Writer) (err error) {
return &net.AddrError{}
})
return &GraphQLResponse{
Data: &Object{
Nullable: false,
Fetch: &SingleFetch{
FetchConfiguration: FetchConfiguration{
DataSource: mockDataSource,
PostProcessing: PostProcessingConfiguration{
SelectResponseErrorsPath: []string{"errors"},
},
},
Info: &FetchInfo{
DataSourceID: "Users",
},
},
Fields: []*Field{
{
Name: []byte("name"),
Value: &String{
Path: []string{"name"},
Nullable: true,
},
},
},
},
}, Context{ctx: context.Background()}, `{"errors":[{"message":"Failed to fetch from Subgraph 'Users' at path 'query'."}],"data":null}`
}))
t.Run("fetch with returned err", testFn(true, func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx Context, expectedOutput string) {
mockDataSource := NewMockDataSource(ctrl)
mockDataSource.EXPECT().
Load(gomock.Any(), gomock.Any(), gomock.AssignableToTypeOf(&bytes.Buffer{})).
DoAndReturn(func(ctx context.Context, input []byte, w io.Writer) (err error) {
return &net.AddrError{}
})
return &GraphQLResponse{
Data: &Object{
Nullable: false,
Fetch: &SingleFetch{
FetchConfiguration: FetchConfiguration{
DataSource: mockDataSource,
PostProcessing: PostProcessingConfiguration{
SelectResponseErrorsPath: []string{"errors"},
},
},
},
Fields: []*Field{
{
Name: []byte("name"),
Value: &String{
Path: []string{"name"},
Nullable: true,
},
},
},
},
}, Context{ctx: context.Background()}, `{"errors":[{"message":"Failed to fetch from Subgraph at path 'query'."}],"data":null}`
}))
t.Run("fetch with two Errors", testFn(true, func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx Context, expectedOutput string) {
mockDataSource := NewMockDataSource(ctrl)
mockDataSource.EXPECT().
Expand Down
2 changes: 1 addition & 1 deletion v2/pkg/graphql/execution_engine_v2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ func TestExecutionEngineV2_Execute(t *testing.T) {
},
},
},
expectedResponse: `{"errors":[{"message":"Cannot return null for non-nullable field 'Query.hero'.","path":["hero"]}],"data":null}`,
expectedResponse: `{"errors":[{"message":"invalid input","path":[]}],"data":null}`,
}))

t.Run("execute operation and apply input coercion for lists without variables", runWithoutError(ExecutionEngineV2TestCase{
Expand Down

0 comments on commit 92f315e

Please sign in to comment.