Skip to content

Commit a5ca15d

Browse files
authored
fix: verify that errors are propagated on engine hooks for correct subgraphs (#2399)
1 parent d2d00cd commit a5ca15d

File tree

7 files changed

+166
-34
lines changed

7 files changed

+166
-34
lines changed

router-tests/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ require (
2626
github.com/wundergraph/cosmo/demo/pkg/subgraphs/projects v0.0.0-20250715110703-10f2e5f9c79e
2727
github.com/wundergraph/cosmo/router v0.0.0-20251125205644-175f80c4e6d9
2828
github.com/wundergraph/cosmo/router-plugin v0.0.0-20250808194725-de123ba1c65e
29-
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.240
29+
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241
3030
go.opentelemetry.io/otel v1.36.0
3131
go.opentelemetry.io/otel/sdk v1.36.0
3232
go.opentelemetry.io/otel/sdk/metric v1.36.0

router-tests/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,8 @@ github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/
352352
github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw=
353353
github.com/wundergraph/astjson v0.0.0-20250106123708-be463c97e083 h1:8/D7f8gKxTBjW+SZK4mhxTTBVpxcqeBgWF1Rfmltbfk=
354354
github.com/wundergraph/astjson v0.0.0-20250106123708-be463c97e083/go.mod h1:eOTL6acwctsN4F3b7YE+eE2t8zcJ/doLm9sZzsxxxrE=
355-
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.240 h1:xnYwsUrmDcQnrZQ4+WZ8oODktsKJyAJWiYplWfmiQuk=
356-
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.240/go.mod h1:mX25ASEQiKamxaFSK6NZihh0oDCigIuzro30up4mFH4=
355+
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241 h1:ch/8hfDaw4oz1Cx3Wb+OUl4qiAo17OdGhYMdRYnX8Is=
356+
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241/go.mod h1:mX25ASEQiKamxaFSK6NZihh0oDCigIuzro30up4mFH4=
357357
github.com/xrash/smetrics v0.0.0-20250705151800-55b8f293f342 h1:FnBeRrxr7OU4VvAzt5X7s6266i6cSVkkFPS0TuXWbIg=
358358
github.com/xrash/smetrics v0.0.0-20250705151800-55b8f293f342/go.mod h1:Ohn+xnUBiLI6FVj/9LpzZWtj1/D6lUovWYBkxHVV3aM=
359359
github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4=

router-tests/telemetry/telemetry_test.go

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11254,6 +11254,139 @@ func TestFlakyTelemetry(t *testing.T) {
1125411254
})
1125511255
})
1125611256

11257+
t.Run("verify errors being attached to unrelated span subgraphs", func(t *testing.T) {
11258+
simulateConnectionFailureOnClose := func(w http.ResponseWriter) {
11259+
hj, ok := w.(http.Hijacker)
11260+
if !ok {
11261+
// If the hijacker is not available, we switch to panic
11262+
// to simulate a failure
11263+
panic("service failure")
11264+
}
11265+
conn, _, err := hj.Hijack()
11266+
if err != nil {
11267+
// Hijacking failed, switch to panic
11268+
// to simulate a failure
11269+
panic(err)
11270+
}
11271+
_ = conn.Close()
11272+
}
11273+
11274+
t.Run("with one subgraph giving an error", func(t *testing.T) {
11275+
t.Parallel()
11276+
11277+
exporter := tracetest.NewInMemoryExporter(t)
11278+
testenv.Run(t, &testenv.Config{
11279+
TraceExporter: exporter,
11280+
Subgraphs: testenv.SubgraphsConfig{
11281+
Products: testenv.SubgraphConfig{
11282+
Middleware: func(_ http.Handler) http.Handler {
11283+
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
11284+
simulateConnectionFailureOnClose(w)
11285+
})
11286+
},
11287+
},
11288+
},
11289+
}, func(t *testing.T, xEnv *testenv.Environment) {
11290+
xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
11291+
Query: `query { employees { id isAvailable products derivedMood } }`,
11292+
})
11293+
11294+
sn := exporter.GetSpans().Snapshots()
11295+
11296+
subgraphThatShouldHaveError := "products"
11297+
11298+
for _, span := range sn {
11299+
if slices.Contains([]string{"Engine - Fetch"}, span.Name()) {
11300+
attributes := span.Attributes()
11301+
events := span.Events()
11302+
11303+
hasErrorEvent := false
11304+
subgraphName := ""
11305+
11306+
if len(events) > 0 {
11307+
require.Len(t, events, 1)
11308+
require.Equal(t, "exception", events[0].Name)
11309+
hasErrorEvent = true
11310+
}
11311+
11312+
for _, attributeEntry := range attributes {
11313+
if attributeEntry.Key == otel.WgSubgraphName {
11314+
subgraphName = attributeEntry.Value.AsString()
11315+
}
11316+
}
11317+
11318+
if subgraphName == subgraphThatShouldHaveError {
11319+
require.True(t, hasErrorEvent)
11320+
} else {
11321+
require.False(t, hasErrorEvent)
11322+
}
11323+
}
11324+
}
11325+
})
11326+
})
11327+
11328+
t.Run("with multiple subgraphs giving an error", func(t *testing.T) {
11329+
t.Parallel()
11330+
11331+
exporter := tracetest.NewInMemoryExporter(t)
11332+
testenv.Run(t, &testenv.Config{
11333+
TraceExporter: exporter,
11334+
Subgraphs: testenv.SubgraphsConfig{
11335+
Products: testenv.SubgraphConfig{
11336+
Middleware: func(_ http.Handler) http.Handler {
11337+
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
11338+
simulateConnectionFailureOnClose(w)
11339+
})
11340+
},
11341+
},
11342+
Availability: testenv.SubgraphConfig{
11343+
Middleware: func(_ http.Handler) http.Handler {
11344+
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
11345+
simulateConnectionFailureOnClose(w)
11346+
})
11347+
},
11348+
},
11349+
},
11350+
}, func(t *testing.T, xEnv *testenv.Environment) {
11351+
xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
11352+
Query: `query { employees { id isAvailable derivedMood products } }`,
11353+
})
11354+
11355+
sn := exporter.GetSpans().Snapshots()
11356+
11357+
subgraphsThatShouldHaveError := []string{"products", "availability"}
11358+
11359+
for _, span := range sn {
11360+
if slices.Contains([]string{"Engine - Fetch"}, span.Name()) {
11361+
attributes := span.Attributes()
11362+
events := span.Events()
11363+
11364+
hasErrorEvent := false
11365+
subgraphName := ""
11366+
11367+
if len(events) > 0 {
11368+
require.Len(t, events, 1)
11369+
require.Equal(t, "exception", events[0].Name)
11370+
hasErrorEvent = true
11371+
}
11372+
11373+
for _, attributeEntry := range attributes {
11374+
if attributeEntry.Key == otel.WgSubgraphName {
11375+
subgraphName = attributeEntry.Value.AsString()
11376+
}
11377+
}
11378+
11379+
if slices.Contains(subgraphsThatShouldHaveError, subgraphName) {
11380+
require.True(t, hasErrorEvent)
11381+
} else {
11382+
require.False(t, hasErrorEvent)
11383+
}
11384+
}
11385+
}
11386+
})
11387+
})
11388+
})
11389+
1125711390
}
1125811391

1125911392
func TestExcludeAttributesWithCustomExporter(t *testing.T) {

router/core/graphql_handler.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -141,21 +141,19 @@ func (h *GraphQLHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
141141
)
142142
defer graphqlExecutionSpan.End()
143143

144-
resolveCtx := &resolve.Context{
145-
Variables: reqCtx.operation.variables,
146-
RemapVariables: reqCtx.operation.remapVariables,
147-
Files: reqCtx.operation.files,
148-
Request: resolve.Request{
149-
Header: r.Header,
150-
},
151-
RenameTypeNames: h.executor.RenameTypeNames,
152-
TracingOptions: reqCtx.operation.traceOptions,
153-
InitialPayload: reqCtx.operation.initialPayload,
154-
Extensions: reqCtx.operation.extensions,
155-
ExecutionOptions: reqCtx.operation.executionOptions,
144+
resolveCtx := resolve.NewContext(executionContext)
145+
resolveCtx.Variables = reqCtx.operation.variables
146+
resolveCtx.RemapVariables = reqCtx.operation.remapVariables
147+
resolveCtx.Files = reqCtx.operation.files
148+
resolveCtx.Request = resolve.Request{
149+
Header: r.Header,
156150
}
151+
resolveCtx.RenameTypeNames = h.executor.RenameTypeNames
152+
resolveCtx.TracingOptions = reqCtx.operation.traceOptions
153+
resolveCtx.InitialPayload = reqCtx.operation.initialPayload
154+
resolveCtx.Extensions = reqCtx.operation.extensions
155+
resolveCtx.ExecutionOptions = reqCtx.operation.executionOptions
157156

158-
resolveCtx = resolveCtx.WithContext(executionContext)
159157
if h.authorizer != nil {
160158
resolveCtx = WithAuthorizationExtension(resolveCtx)
161159
resolveCtx.SetAuthorizer(h.authorizer)

router/core/websocket.go

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,20 +1007,6 @@ func (h *WebSocketConnectionHandler) executeSubscription(registration *Subscript
10071007
return
10081008
}
10091009
}
1010-
resolveCtx := &resolve.Context{
1011-
Variables: operationCtx.Variables(),
1012-
Request: resolve.Request{
1013-
Header: registration.clientRequest.Header,
1014-
ID: h.initRequestID,
1015-
},
1016-
RenameTypeNames: h.graphqlHandler.executor.RenameTypeNames,
1017-
RemapVariables: operationCtx.remapVariables,
1018-
TracingOptions: operationCtx.traceOptions,
1019-
Extensions: operationCtx.extensions,
1020-
}
1021-
if h.forwardInitialPayload && operationCtx.initialPayload != nil {
1022-
resolveCtx.InitialPayload = operationCtx.initialPayload
1023-
}
10241010

10251011
reqContext := buildRequestContext(requestContextOptions{
10261012
operationContext: operationCtx,
@@ -1033,7 +1019,22 @@ func (h *WebSocketConnectionHandler) executeSubscription(registration *Subscript
10331019
if origCtx := getRequestContext(h.request.Context()); origCtx != nil {
10341020
reqContext.expressionContext = *origCtx.expressionContext.Clone()
10351021
}
1036-
resolveCtx = resolveCtx.WithContext(withRequestContext(h.ctx, reqContext))
1022+
1023+
resolveCtx := resolve.NewContext(withRequestContext(h.ctx, reqContext))
1024+
resolveCtx.Variables = operationCtx.Variables()
1025+
resolveCtx.Request = resolve.Request{
1026+
Header: registration.clientRequest.Header,
1027+
ID: h.initRequestID,
1028+
}
1029+
resolveCtx.RenameTypeNames = h.graphqlHandler.executor.RenameTypeNames
1030+
resolveCtx.RemapVariables = operationCtx.remapVariables
1031+
resolveCtx.TracingOptions = operationCtx.traceOptions
1032+
resolveCtx.Extensions = operationCtx.extensions
1033+
1034+
if h.forwardInitialPayload && operationCtx.initialPayload != nil {
1035+
resolveCtx.InitialPayload = operationCtx.initialPayload
1036+
}
1037+
10371038
if h.graphqlHandler.authorizer != nil {
10381039
resolveCtx = WithAuthorizationExtension(resolveCtx)
10391040
resolveCtx.SetAuthorizer(h.graphqlHandler.authorizer)

router/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ require (
3131
github.com/tidwall/gjson v1.18.0
3232
github.com/tidwall/sjson v1.2.5
3333
github.com/twmb/franz-go v1.16.1
34-
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.240
34+
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241
3535
// Do not upgrade, it renames attributes we rely on
3636
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.58.0
3737
go.opentelemetry.io/contrib/propagators/b3 v1.23.0

router/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,8 +322,8 @@ github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/
322322
github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw=
323323
github.com/wundergraph/astjson v0.0.0-20250106123708-be463c97e083 h1:8/D7f8gKxTBjW+SZK4mhxTTBVpxcqeBgWF1Rfmltbfk=
324324
github.com/wundergraph/astjson v0.0.0-20250106123708-be463c97e083/go.mod h1:eOTL6acwctsN4F3b7YE+eE2t8zcJ/doLm9sZzsxxxrE=
325-
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.240 h1:xnYwsUrmDcQnrZQ4+WZ8oODktsKJyAJWiYplWfmiQuk=
326-
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.240/go.mod h1:mX25ASEQiKamxaFSK6NZihh0oDCigIuzro30up4mFH4=
325+
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241 h1:ch/8hfDaw4oz1Cx3Wb+OUl4qiAo17OdGhYMdRYnX8Is=
326+
github.com/wundergraph/graphql-go-tools/v2 v2.0.0-rc.241/go.mod h1:mX25ASEQiKamxaFSK6NZihh0oDCigIuzro30up4mFH4=
327327
github.com/yosida95/uritemplate/v3 v3.0.2 h1:Ed3Oyj9yrmi9087+NczuL5BwkIc4wvTb5zIM+UJPGz4=
328328
github.com/yosida95/uritemplate/v3 v3.0.2/go.mod h1:ILOh0sOhIJR3+L/8afwt/kE++YT040gmv5BQTMR2HP4=
329329
github.com/yuin/gopher-lua v1.1.1 h1:kYKnWBjvbNP4XLT3+bPEwAXJx262OhaHDWDVOPjL46M=

0 commit comments

Comments
 (0)