From 462025b400e9b792a5afbe320cde4cc952f6b547 Mon Sep 17 00:00:00 2001 From: Steve Coffman Date: Tue, 13 Sep 2022 13:04:59 -0400 Subject: [PATCH] nil check error before type assertion follow-up from #2341 (#2368) * nil check error before cast from #2341 Signed-off-by: Steve Coffman * Improve errcode.Set safety Signed-off-by: Steve Coffman Signed-off-by: Steve Coffman --- graphql/errcode/codes.go | 12 +++++- graphql/executor/executor.go | 40 +++++++++++++------ .../filetemplate/out/schema.custom.go | 6 +-- .../followschema/out/schema.resolvers.go | 6 +-- 4 files changed, 44 insertions(+), 20 deletions(-) diff --git a/graphql/errcode/codes.go b/graphql/errcode/codes.go index 4333d87e4d0..854b206f4e8 100644 --- a/graphql/errcode/codes.go +++ b/graphql/errcode/codes.go @@ -23,14 +23,22 @@ var codeType = map[string]ErrorKind{ ParseFailed: KindProtocol, } -// RegisterErrorType should be called by extensions that want to customize the http status codes for errors they return +// RegisterErrorType should be called by extensions that want to customize the http status codes for +// errors they return func RegisterErrorType(code string, kind ErrorKind) { codeType[code] = kind } // Set the error code on a given graphql error extension func Set(err error, value string) { - gqlErr, _ := err.(*gqlerror.Error) + if err == nil { + return + } + gqlErr, ok := err.(*gqlerror.Error) + if !ok { + return + } + if gqlErr.Extensions == nil { gqlErr.Extensions = map[string]interface{}{} } diff --git a/graphql/executor/executor.go b/graphql/executor/executor.go index 6bfb698f5fd..c46a007b99e 100644 --- a/graphql/executor/executor.go +++ b/graphql/executor/executor.go @@ -37,7 +37,10 @@ func New(es graphql.ExecutableSchema) *Executor { return e } -func (e *Executor) CreateOperationContext(ctx context.Context, params *graphql.RawParams) (*graphql.OperationContext, gqlerror.List) { +func (e *Executor) CreateOperationContext( + ctx context.Context, + params *graphql.RawParams, +) (*graphql.OperationContext, gqlerror.List) { rc := &graphql.OperationContext{ DisableIntrospection: true, RecoverFunc: e.recoverFunc, @@ -75,10 +78,13 @@ func (e *Executor) CreateOperationContext(ctx context.Context, params *graphql.R var err error rc.Variables, err = validator.VariableValues(e.es.Schema(), rc.Operation, params.Variables) - gqlErr, _ := err.(*gqlerror.Error) - if gqlErr != nil { - errcode.Set(gqlErr, errcode.ValidationFailed) - return rc, gqlerror.List{gqlErr} + + if err != nil { + gqlErr, ok := err.(*gqlerror.Error) + if ok { + errcode.Set(gqlErr, errcode.ValidationFailed) + return rc, gqlerror.List{gqlErr} + } } rc.Stats.Validation.End = graphql.Now() @@ -91,7 +97,10 @@ func (e *Executor) CreateOperationContext(ctx context.Context, params *graphql.R return rc, nil } -func (e *Executor) DispatchOperation(ctx context.Context, rc *graphql.OperationContext) (graphql.ResponseHandler, context.Context) { +func (e *Executor) DispatchOperation( + ctx context.Context, + rc *graphql.OperationContext, +) (graphql.ResponseHandler, context.Context) { ctx = graphql.WithOperationContext(ctx, rc) var innerCtx context.Context @@ -161,9 +170,14 @@ func (e *Executor) SetRecoverFunc(f graphql.RecoverFunc) { // parseQuery decodes the incoming query and validates it, pulling from cache if present. // -// NOTE: This should NOT look at variables, they will change per request. It should only parse and validate +// NOTE: This should NOT look at variables, they will change per request. It should only parse and +// validate // the raw query string. -func (e *Executor) parseQuery(ctx context.Context, stats *graphql.Stats, query string) (*ast.QueryDocument, gqlerror.List) { +func (e *Executor) parseQuery( + ctx context.Context, + stats *graphql.Stats, + query string, +) (*ast.QueryDocument, gqlerror.List) { stats.Parsing.Start = graphql.Now() if doc, ok := e.queryCache.Get(ctx, query); ok { @@ -175,10 +189,12 @@ func (e *Executor) parseQuery(ctx context.Context, stats *graphql.Stats, query s } doc, err := parser.ParseQuery(&ast.Source{Input: query}) - gqlErr, _ := err.(*gqlerror.Error) - if gqlErr != nil { - errcode.Set(gqlErr, errcode.ParseFailed) - return nil, gqlerror.List{gqlErr} + if err != nil { + gqlErr, ok := err.(*gqlerror.Error) + if ok { + errcode.Set(gqlErr, errcode.ParseFailed) + return nil, gqlerror.List{gqlErr} + } } stats.Parsing.End = graphql.Now() diff --git a/plugin/resolvergen/testdata/filetemplate/out/schema.custom.go b/plugin/resolvergen/testdata/filetemplate/out/schema.custom.go index 43f420426b0..c1412cb9184 100644 --- a/plugin/resolvergen/testdata/filetemplate/out/schema.custom.go +++ b/plugin/resolvergen/testdata/filetemplate/out/schema.custom.go @@ -35,9 +35,9 @@ type resolverCustomResolverType struct{ *CustomResolverType } // !!! WARNING !!! // The code below was going to be deleted when updating resolvers. It has been copied here so you have // one last chance to move it out of harms way if you want. There are two reasons this happens: -// - When renaming or deleting a resolver the old code will be put in here. You can safely delete -// it when you're done. -// - You have helper methods in this file. Move them out to keep these resolver files clean. +// - When renaming or deleting a resolver the old code will be put in here. You can safely delete +// it when you're done. +// - You have helper methods in this file. Move them out to keep these resolver files clean. func AUserHelperFunction() { // AUserHelperFunction implementation } diff --git a/plugin/resolvergen/testdata/followschema/out/schema.resolvers.go b/plugin/resolvergen/testdata/followschema/out/schema.resolvers.go index 43f420426b0..c1412cb9184 100644 --- a/plugin/resolvergen/testdata/followschema/out/schema.resolvers.go +++ b/plugin/resolvergen/testdata/followschema/out/schema.resolvers.go @@ -35,9 +35,9 @@ type resolverCustomResolverType struct{ *CustomResolverType } // !!! WARNING !!! // The code below was going to be deleted when updating resolvers. It has been copied here so you have // one last chance to move it out of harms way if you want. There are two reasons this happens: -// - When renaming or deleting a resolver the old code will be put in here. You can safely delete -// it when you're done. -// - You have helper methods in this file. Move them out to keep these resolver files clean. +// - When renaming or deleting a resolver the old code will be put in here. You can safely delete +// it when you're done. +// - You have helper methods in this file. Move them out to keep these resolver files clean. func AUserHelperFunction() { // AUserHelperFunction implementation }