Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
8 changes: 4 additions & 4 deletions router-tests/error_handling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1380,7 +1380,7 @@ func TestErrorPropagation(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
}, func(t *testing.T, xEnv *testenv.Environment) {
resp, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
Expand Down Expand Up @@ -1412,7 +1412,7 @@ func TestErrorPropagation(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
}, func(t *testing.T, xEnv *testenv.Environment) {
resp, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
Expand Down Expand Up @@ -1444,7 +1444,7 @@ func TestErrorPropagation(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
}, func(t *testing.T, xEnv *testenv.Environment) {
resp, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
Expand Down Expand Up @@ -1476,7 +1476,7 @@ func TestErrorPropagation(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
}, func(t *testing.T, xEnv *testenv.Environment) {
resp, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
Expand Down
2 changes: 1 addition & 1 deletion router-tests/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ require (
github.com/docker/docker-credential-helpers v0.9.3 // indirect
github.com/docker/go-units v0.5.0 // indirect
github.com/dustin/go-humanize v1.0.1 // indirect
github.com/expr-lang/expr v1.17.3 // indirect
github.com/expr-lang/expr v1.17.6 // indirect
Comment thread
SkArchon marked this conversation as resolved.
github.com/fatih/color v1.18.0 // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/go-chi/chi/v5 v5.2.2 // indirect
Expand Down
4 changes: 2 additions & 2 deletions router-tests/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4
github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk=
github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkpeCY=
github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto=
github.com/expr-lang/expr v1.17.3 h1:myeTTuDFz7k6eFe/JPlep/UsiIjVhG61FMHFu63U7j0=
github.com/expr-lang/expr v1.17.3/go.mod h1:8/vRC7+7HBzESEqt5kKpYXxrxkr31SaO8r40VO/1IT4=
github.com/expr-lang/expr v1.17.6 h1:1h6i8ONk9cexhDmowO/A64VPxHScu7qfSl2k8OlINec=
github.com/expr-lang/expr v1.17.6/go.mod h1:8/vRC7+7HBzESEqt5kKpYXxrxkr31SaO8r40VO/1IT4=
github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk=
github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM=
github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU=
Expand Down
4 changes: 2 additions & 2 deletions router-tests/panic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestEnginePanic(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
}, func(t *testing.T, xEnv *testenv.Environment) {
res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
Expand Down Expand Up @@ -80,7 +80,7 @@ func TestEnginePanic(t *testing.T) {
EnableSingleFlight: true,
ParseKitPoolSize: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
}, func(t *testing.T, xEnv *testenv.Environment) {
res, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{
Expand Down
10 changes: 5 additions & 5 deletions router-tests/structured_logging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -709,7 +709,7 @@ func TestFlakyAccessLogs(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
Comment thread
SkArchon marked this conversation as resolved.
LogObservation: testenv.LogObservationConfig{
Enabled: true,
Expand Down Expand Up @@ -828,7 +828,7 @@ func TestFlakyAccessLogs(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
LogObservation: testenv.LogObservationConfig{
Enabled: true,
Expand Down Expand Up @@ -960,7 +960,7 @@ func TestFlakyAccessLogs(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
LogObservation: testenv.LogObservationConfig{
Enabled: true,
Expand Down Expand Up @@ -1096,7 +1096,7 @@ func TestFlakyAccessLogs(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
LogObservation: testenv.LogObservationConfig{
Enabled: true,
Expand Down Expand Up @@ -2211,7 +2211,7 @@ func TestFlakyAccessLogs(t *testing.T) {
EnableSingleFlight: true,
MaxConcurrentResolvers: 1,
}),
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
LogObservation: testenv.LogObservationConfig{
Enabled: true,
Expand Down
2 changes: 1 addition & 1 deletion router-tests/telemetry/telemetry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8706,7 +8706,7 @@ func TestFlakyTelemetry(t *testing.T) {
},
},
RouterOptions: []core.Option{
core.WithSubgraphRetryOptions(false, 0, 0, 0),
core.WithSubgraphRetryOptions(false, 0, 0, 0, ""),
},
Subgraphs: testenv.SubgraphsConfig{
Products: testenv.SubgraphConfig{
Expand Down
9 changes: 0 additions & 9 deletions router/core/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,15 +634,6 @@ func (o *operationContext) QueryPlanStats() (QueryPlanStats, error) {
return qps, nil
}

// isMutationRequest returns true if the current request is a mutation request
func isMutationRequest(ctx context.Context) bool {
op := getRequestContext(ctx)
if op == nil {
return false
}
return op.Operation().Type() == "mutation"
}

type SubgraphResolver struct {
subgraphsByURL map[string]*Subgraph
subgraphsByID map[string]*Subgraph
Expand Down
35 changes: 21 additions & 14 deletions router/core/graph_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,21 @@ func (s *graphServer) buildGraphMux(
baseConnMetricStore = s.connectionMetrics
}

// Build retry options and handle any expression compilation errors
shouldRetryFunc, err := BuildRetryFunction(s.retryOptions.Expression)
if err != nil {
return nil, fmt.Errorf("failed to build retry function: %w", err)
}

retryOptions := retrytransport.RetryOptions{
Enabled: s.retryOptions.Enabled,
MaxRetryCount: s.retryOptions.MaxRetryCount,
MaxDuration: s.retryOptions.MaxDuration,
Interval: s.retryOptions.Interval,
Expression: s.retryOptions.Expression,
ShouldRetry: shouldRetryFunc,
}

Comment thread
SkArchon marked this conversation as resolved.
Comment thread
SkArchon marked this conversation as resolved.
ecb := &ExecutorConfigurationBuilder{
introspection: s.introspection,
baseURL: s.baseURL,
Expand All @@ -1171,20 +1186,12 @@ func (s *graphServer) buildGraphMux(
FrameTimeout: s.engineExecutionConfiguration.WebSocketClientFrameTimeout,
},
transportOptions: &TransportOptions{
SubgraphTransportOptions: s.subgraphTransportOptions,
PreHandlers: s.preOriginHandlers,
PostHandlers: s.postOriginHandlers,
MetricStore: gm.metricStore,
ConnectionMetricStore: baseConnMetricStore,
RetryOptions: retrytransport.RetryOptions{
Enabled: s.retryOptions.Enabled,
MaxRetryCount: s.retryOptions.MaxRetryCount,
MaxDuration: s.retryOptions.MaxDuration,
Interval: s.retryOptions.Interval,
ShouldRetry: func(err error, req *http.Request, resp *http.Response) bool {
return retrytransport.IsRetryableError(err, resp) && !isMutationRequest(req.Context())
},
},
SubgraphTransportOptions: s.subgraphTransportOptions,
PreHandlers: s.preOriginHandlers,
PostHandlers: s.postOriginHandlers,
MetricStore: gm.metricStore,
ConnectionMetricStore: baseConnMetricStore,
RetryOptions: retryOptions,
TracerProvider: s.tracerProvider,
TracePropagators: s.compositePropagator,
LocalhostFallbackInsideDocker: s.localhostFallbackInsideDocker,
Expand Down
58 changes: 58 additions & 0 deletions router/core/retry_expression_builder.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package core

import (
"fmt"
"net/http"

"github.com/wundergraph/cosmo/router/internal/expr"
"github.com/wundergraph/cosmo/router/internal/retrytransport"
"go.uber.org/zap"
)
Comment thread
SkArchon marked this conversation as resolved.

const DefaultRetryExpression = "IsRetryableStatusCode() || IsConnectionError() || IsTimeout()"
Comment thread
Noroth marked this conversation as resolved.
Outdated

// BuildRetryFunction creates a ShouldRetry function based on the provided expression
func BuildRetryFunction(expression string) (retrytransport.ShouldRetryFunc, error) {
// Use default expression if empty string is passed
if expression == "" {
expression = DefaultRetryExpression
}

// Create the retry expression manager
manager, err := expr.NewRetryExpressionManager(expression)
if err != nil {
return nil, fmt.Errorf("failed to compile retry expression: %w", err)
Comment thread
Noroth marked this conversation as resolved.
Outdated
}

// Return expression-based retry function
return func(err error, req *http.Request, resp *http.Response) bool {
reqContext := getRequestContext(req.Context())

if reqContext == nil {
return false
}

// Never retry mutations, regardless of expression result
if reqContext.Operation().Type() == "mutation" {
return false
}

// Create retry context
ctx := expr.LoadRetryContext(err, resp)

// Evaluate the expression
shouldRetry, evalErr := manager.ShouldRetry(ctx)
if evalErr != nil {

reqContext.logger.Error("Failed to evaluate retry expression",
zap.Error(evalErr),
zap.String("expression", expression),
)

// Disable retries on evaluation error
return false
}

return shouldRetry
}, nil
}
Loading
Loading