nil check error before type assertion follow-up from #2341 (#2368)

* nil check error before cast from #2341

Signed-off-by: Steve Coffman <steve@khanacademy.org>

* Improve errcode.Set safety

Signed-off-by: Steve Coffman <steve@khanacademy.org>

Signed-off-by: Steve Coffman <steve@khanacademy.org>

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{}{}
 	}

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()
 

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
 }

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
 }