Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: omit custom fields in subgraph errors #912

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions v2/pkg/engine/resolve/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ type Loader struct {
attachServiceNameToErrorExtension bool
allowedErrorExtensionFields map[string]struct{}
defaultErrorExtensionCode string
omitCustomSubgraphErrorFields bool
}

func (l *Loader) Free() {
Expand Down Expand Up @@ -687,6 +688,8 @@ func (l *Loader) mergeErrors(res *result, fetchItem *FetchItem, value *astjson.V
// Allow to delete extensions entirely
l.optionallyOmitErrorExtensions(values)

l.optionallyOmitCustomErrorFields(values)

if len(values) > 0 {
// Append the subgraph errors to the response payload
if err := l.appendSubgraphError(res, fetchItem, value, values); err != nil {
Expand Down Expand Up @@ -813,6 +816,29 @@ func (l *Loader) optionallyOmitErrorExtensions(values []*astjson.Value) {
}
}

// optionallyOmitCustomErrorFields removes all custom fields from the subgraph error
func (l *Loader) optionallyOmitCustomErrorFields(values []*astjson.Value) {
if !l.omitCustomSubgraphErrorFields {
return
}
for _, value := range values {
if value.Type() == astjson.TypeObject {
obj := value.GetObject()
var keysToDelete []string
obj.Visit(func(k []byte, v *astjson.Value) {
key := unsafebytes.BytesToString(k)
if key != "message" && key != "extensions" {
jensneuse marked this conversation as resolved.
Show resolved Hide resolved
keysToDelete = append(keysToDelete, key)
}
})
for _, key := range keysToDelete {
fmt.Println(key)
thisisnithin marked this conversation as resolved.
Show resolved Hide resolved
obj.Del(key)
}
}
}
}

// optionallyOmitErrorLocations removes the locations object from all values
func (l *Loader) optionallyOmitErrorLocations(values []*astjson.Value) {
if !l.omitSubgraphErrorLocations {
Expand Down
3 changes: 3 additions & 0 deletions v2/pkg/engine/resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ type ResolverOptions struct {
MaxRecyclableParserSize int
// ResolvableOptions are configuration options for the Resolbable struct
ResolvableOptions ResolvableOptions
// OmitCustomSubgraphErrorFields omits all additional fields except message and extensions
OmitCustomSubgraphErrorFields bool
}

// New returns a new Resolver, ctx.Done() is used to cancel all active subscriptions & streams
Expand Down Expand Up @@ -158,6 +160,7 @@ func New(ctx context.Context, options ResolverOptions) *Resolver {
allowedErrorExtensionFields: allowedExtensionFields,
attachServiceNameToErrorExtension: options.AttachServiceNameToErrorExtensions,
defaultErrorExtensionCode: options.DefaultErrorExtensionCode,
omitCustomSubgraphErrorFields: options.OmitCustomSubgraphErrorFields,
},
}
},
Expand Down
68 changes: 68 additions & 0 deletions v2/pkg/engine/resolve/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1849,6 +1849,36 @@ func testFnWithError(fn func(t *testing.T, ctrl *gomock.Controller) (node *Graph
}
}

func testFnSubgraphErrorsPassthroughAndOmitCustomFields(fn func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx Context, expectedOutput string)) func(t *testing.T) {
return func(t *testing.T) {
t.Helper()

ctrl := gomock.NewController(t)
rCtx, cancel := context.WithCancel(context.Background())
defer cancel()
r := New(rCtx, ResolverOptions{
MaxConcurrency: 1024,
Debug: false,
PropagateSubgraphErrors: true,
PropagateSubgraphStatusCodes: true,
SubgraphErrorPropagationMode: SubgraphErrorPropagationModePassThrough,
OmitCustomSubgraphErrorFields: true,
AllowedErrorExtensionFields: []string{"code"},
})
node, ctx, expectedOutput := fn(t, ctrl)

if t.Skipped() {
return
}

buf := &bytes.Buffer{}
_, err := r.ResolveGraphQLResponse(&ctx, node, nil, buf)
assert.NoError(t, err)
assert.Equal(t, expectedOutput, buf.String())
ctrl.Finish()
}
}

func TestResolver_ResolveGraphQLResponse(t *testing.T) {

t.Run("empty graphql response", testFn(func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx Context, expectedOutput string) {
Expand Down Expand Up @@ -2358,6 +2388,44 @@ func TestResolver_ResolveGraphQLResponse(t *testing.T) {
},
}, Context{ctx: context.Background()}, `{"errors":[{"message":"errorMessage"}],"data":{"name":null}}`
}))
t.Run("fetch with pass through mode and omit custom fields", testFnSubgraphErrorsPassthroughAndOmitCustomFields(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) error {
_, err := w.Write([]byte(`{"errors":[{"message":"errorMessage","longMessage":"This is a long message","extensions":{"code":"GRAPHQL_VALIDATION_FAILED"}}],"data":{"name":null}}`))
return err
})
return &GraphQLResponse{
Info: &GraphQLResponseInfo{
OperationType: ast.OperationTypeQuery,
},
Fetches: SingleWithPath(&SingleFetch{
FetchConfiguration: FetchConfiguration{
DataSource: mockDataSource,
PostProcessing: PostProcessingConfiguration{
SelectResponseErrorsPath: []string{"errors"},
},
},
Info: &FetchInfo{
DataSourceID: "Users",
DataSourceName: "Users",
},
}, "query"),
Data: &Object{
Nullable: false,
Fields: []*Field{
{
Name: []byte("name"),
Value: &String{
Path: []string{"name"},
Nullable: true,
},
},
},
},
}, Context{ctx: context.Background()}, `{"errors":[{"message":"errorMessage","extensions":{"code":"GRAPHQL_VALIDATION_FAILED"}}],"data":{"name":null}}`
}))
t.Run("fetch with returned err (with DataSourceID)", testFn(func(t *testing.T, ctrl *gomock.Controller) (node *GraphQLResponse, ctx Context, expectedOutput string) {
mockDataSource := NewMockDataSource(ctrl)
mockDataSource.EXPECT().
Expand Down
Loading