Skip to content

Commit

Permalink
fix(router): ensure subgraph timeouts/nil responses are handled corre…
Browse files Browse the repository at this point in the history
…ctly (#1449)
  • Loading branch information
df-wg authored Dec 17, 2024
1 parent 65a6b31 commit 5402152
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 0 deletions.
27 changes: 27 additions & 0 deletions router-tests/header_propagation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,33 @@ func TestHeaderPropagation(t *testing.T) {
})
})

t.Run("works with unresponsive subgraph", func(t *testing.T) {
t.Parallel()
testenv.Run(t, &testenv.Config{
RouterOptions: global(config.ResponseHeaderRuleAlgorithmLastWrite, customHeader, ""),
Subgraphs: testenv.SubgraphsConfig{
Employees: testenv.SubgraphConfig{
Middleware: func(handler http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set(customHeader, employeeVal)
handler.ServeHTTP(w, r)
})
},
},
Hobbies: testenv.SubgraphConfig{
CloseOnStart: true,
},
},
}, func(t *testing.T, xEnv *testenv.Environment) {
res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
Query: queryEmployeeWithHobby,
})
ch := strings.Join(res.Response.Header.Values(customHeader), ",")
require.Equal(t, employeeVal, ch)
require.Equal(t, `{"errors":[{"message":"Failed to fetch from Subgraph 'hobbies' at Path 'employee'."}],"data":{"employee":{"id":1,"hobbies":null}}}`, res.Body)
})
})

t.Run("local last write wins", func(t *testing.T) {
t.Parallel()
testenv.Run(t, &testenv.Config{
Expand Down
5 changes: 5 additions & 0 deletions router/core/header_rule_engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,11 @@ func (h *HeaderPropagation) OnOriginRequest(request *http.Request, ctx RequestCo
}

func (h *HeaderPropagation) OnOriginResponse(resp *http.Response, ctx RequestContext) *http.Response {
// In the case of an error response, it is possible that the response is nil
if resp == nil {
return nil
}

propagation := getResponseHeaderPropagation(resp.Request.Context())
if propagation == nil {
return resp
Expand Down
16 changes: 16 additions & 0 deletions router/core/header_rule_engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,22 @@ func TestPropagateHeaderRule(t *testing.T) {
assert.Equal(t, "test1", updatedClientReq.Header.Get("X-Test-1"))

})

t.Run("Should handle nil resonses", func(t *testing.T) {
ht, err := NewHeaderPropagation(&config.HeaderRules{
All: &config.GlobalHeaderRule{},
})
assert.Nil(t, err)

rr := httptest.NewRecorder()
resp := ht.OnOriginResponse(nil, &requestContext{
logger: zap.NewNop(),
responseWriter: rr,
operation: &operationContext{},
subgraphResolver: NewSubgraphResolver(nil),
})
require.Nil(t, resp)
})
}

func TestRenamePropagateHeaderRule(t *testing.T) {
Expand Down

0 comments on commit 5402152

Please sign in to comment.