Skip to content

Commit 848e84a

Browse files
author
Matthieu Vachon
committed
Fixed reflect.Value.Type on zero Value panic when subscription resolver itself panicks
The internal exec Subscribe method had code to deal with subscription resolver panicking when being called. But when such handling happen, the error is attached to the request object and it never checked later on. This leads to some zero checks to fail when we try to extract the type from the resolver's channel since this variable was never set. Doing this creates a second panic which is not handled and make the application die. To fix the issue, we now check if there is errors on the request object before continuing with the rest of the check, if there is errors, it's because a panic occurs and we return the response right away.
1 parent 4c772c1 commit 848e84a

File tree

3 files changed

+37
-2
lines changed

3 files changed

+37
-2
lines changed

internal/exec/exec.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ type extensionser interface {
3737
}
3838

3939
func makePanicError(value interface{}) *errors.QueryError {
40-
return errors.Errorf("graphql: panic occurred: %v", value)
40+
return errors.Errorf("panic occurred: %v", value)
4141
}
4242

4343
func (r *Request) Execute(ctx context.Context, s *resolvable.Schema, op *query.Operation) ([]byte, []*errors.QueryError) {
@@ -324,7 +324,7 @@ func (r *Request) execList(ctx context.Context, sels []selected.Selection, typ *
324324
r.execSelectionSet(ctx, sels, typ.OfType, &pathSegment{path, i}, s, resolver.Index(i), &entryouts[i])
325325
}(i)
326326
}
327-
for i := 0; i < concurrency;i++ {
327+
for i := 0; i < concurrency; i++ {
328328
sem <- struct{}{}
329329
}
330330
} else {

internal/exec/subscribe.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ func (r *Request) Subscribe(ctx context.Context, s *resolvable.Schema, op *query
5555
}
5656
}()
5757

58+
// Handles the case where the locally executed func above panicked
59+
if len(r.Request.Errs) > 0 {
60+
return sendAndReturnClosed(&Response{Errors: r.Request.Errs})
61+
}
62+
5863
if f == nil {
5964
return sendAndReturnClosed(&Response{Errors: []*errors.QueryError{err}})
6065
}

subscription_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -473,3 +473,33 @@ const schema = `
473473
hello: String!
474474
}
475475
`
476+
477+
type subscriptionsPanicInResolver struct{}
478+
479+
func (r *subscriptionsPanicInResolver) OnPanic() <-chan string {
480+
panic("subscriptionsPanicInResolver")
481+
}
482+
483+
func TestSchemaSubscribe_PanicInResolver(t *testing.T) {
484+
r := &struct {
485+
*subscriptionsPanicInResolver
486+
}{
487+
subscriptionsPanicInResolver: &subscriptionsPanicInResolver{},
488+
}
489+
gqltesting.RunSubscribe(t, &gqltesting.TestSubscription{
490+
Schema: graphql.MustParseSchema(`
491+
type Query {}
492+
type Subscription {
493+
onPanic : String!
494+
}
495+
`, r),
496+
Query: `
497+
subscription {
498+
onPanic
499+
}
500+
`,
501+
ExpectedResults: []gqltesting.TestResponse{
502+
{Errors: []*qerrors.QueryError{{Message: "panic occurred: subscriptionsPanicInResolver"}}},
503+
},
504+
})
505+
}

0 commit comments

Comments
 (0)