From 2340f7a7ae54c33e5d065d99a1f30906f64ba229 Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 18 Feb 2020 14:50:02 +1100 Subject: [PATCH] Ensure panic handlers get applied --- codegen/testserver/panics_test.go | 26 +++++++++++++++++++++----- graphql/handler/executor.go | 2 +- graphql/recovery.go | 20 -------------------- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/codegen/testserver/panics_test.go b/codegen/testserver/panics_test.go index 85235864f0a..68903391496 100644 --- a/codegen/testserver/panics_test.go +++ b/codegen/testserver/panics_test.go @@ -2,11 +2,15 @@ package testserver import ( "context" + "fmt" "testing" + "github.com/99designs/gqlgen/graphql" + "github.com/99designs/gqlgen/client" "github.com/99designs/gqlgen/graphql/handler" "github.com/stretchr/testify/require" + "github.com/vektah/gqlparser/v2/gqlerror" ) func TestPanics(t *testing.T) { @@ -21,33 +25,45 @@ func TestPanics(t *testing.T) { return []MarshalPanic{MarshalPanic("aa"), MarshalPanic("bb")}, nil } - c := client.New(handler.NewDefaultServer(NewExecutableSchema(Config{Resolvers: resolvers}))) + srv := handler.NewDefaultServer(NewExecutableSchema(Config{Resolvers: resolvers})) + srv.SetRecoverFunc(func(ctx context.Context, err interface{}) (userMessage error) { + return fmt.Errorf("panic: %v", err) + }) + + srv.SetErrorPresenter(func(ctx context.Context, err error) *gqlerror.Error { + return &gqlerror.Error{ + Message: "presented: " + err.Error(), + Path: graphql.GetFieldContext(ctx).Path(), + } + }) + + c := client.New(srv) t.Run("panics in marshallers will not kill server", func(t *testing.T) { var resp interface{} err := c.Post(`query { panics { fieldScalarMarshal } }`, &resp) - require.EqualError(t, err, "http 422: {\"errors\":[{\"message\":\"internal system error\"}],\"data\":null}") + require.EqualError(t, err, "http 422: {\"errors\":[{\"message\":\"presented: panic: BOOM\"}],\"data\":null}") }) t.Run("panics in unmarshalers will not kill server", func(t *testing.T) { var resp interface{} err := c.Post(`query { panics { argUnmarshal(u: ["aa", "bb"]) } }`, &resp) - require.EqualError(t, err, "[{\"message\":\"internal system error\",\"path\":[\"panics\",\"argUnmarshal\"]}]") + require.EqualError(t, err, "[{\"message\":\"presented: panic: BOOM\",\"path\":[\"panics\",\"argUnmarshal\"]}]") }) t.Run("panics in funcs unmarshal return errors", func(t *testing.T) { var resp interface{} err := c.Post(`query { panics { fieldFuncMarshal(u: ["aa", "bb"]) } }`, &resp) - require.EqualError(t, err, "[{\"message\":\"internal system error\",\"path\":[\"panics\",\"fieldFuncMarshal\"]}]") + require.EqualError(t, err, "[{\"message\":\"presented: panic: BOOM\",\"path\":[\"panics\",\"fieldFuncMarshal\"]}]") }) t.Run("panics in funcs marshal return errors", func(t *testing.T) { var resp interface{} err := c.Post(`query { panics { fieldFuncMarshal(u: []) } }`, &resp) - require.EqualError(t, err, "http 422: {\"errors\":[{\"message\":\"internal system error\"}],\"data\":null}") + require.EqualError(t, err, "http 422: {\"errors\":[{\"message\":\"presented: panic: BOOM\"}],\"data\":null}") }) } diff --git a/graphql/handler/executor.go b/graphql/handler/executor.go index 394d3f5619b..61593f7553c 100644 --- a/graphql/handler/executor.go +++ b/graphql/handler/executor.go @@ -119,7 +119,7 @@ func (e executor) DispatchOperation(ctx context.Context, rc *graphql.OperationCo func (e executor) CreateOperationContext(ctx context.Context, params *graphql.RawParams) (*graphql.OperationContext, gqlerror.List) { rc := &graphql.OperationContext{ DisableIntrospection: true, - Recover: graphql.DefaultRecover, + Recover: e.server.recoverFunc, ResolverMiddleware: e.fieldMiddleware, Stats: graphql.Stats{ Read: params.ReadTime, diff --git a/graphql/recovery.go b/graphql/recovery.go index 5c3c1027eb0..3aa032dc5aa 100644 --- a/graphql/recovery.go +++ b/graphql/recovery.go @@ -6,8 +6,6 @@ import ( "fmt" "os" "runtime/debug" - - "github.com/vektah/gqlparser/v2/gqlerror" ) type RecoverFunc func(ctx context.Context, err interface{}) (userMessage error) @@ -19,21 +17,3 @@ func DefaultRecover(ctx context.Context, err interface{}) error { return errors.New("internal system error") } - -var _ OperationContextMutator = RecoverFunc(nil) - -func (f RecoverFunc) ExtensionName() string { - return "RecoverFunc" -} - -func (f RecoverFunc) Validate(schema ExecutableSchema) error { - if f == nil { - return fmt.Errorf("RecoverFunc can not be nil") - } - return nil -} - -func (f RecoverFunc) MutateOperationContext(ctx context.Context, rc *OperationContext) *gqlerror.Error { - rc.Recover = f - return nil -}