Skip to content

Commit

Permalink
Fix(GraphQL): Log query along with the panic (#7638)
Browse files Browse the repository at this point in the history
* Log query along with panic

* Log mutations using panic handler

* Address Abhimanyu's comments

(cherry picked from commit 071d5de)
  • Loading branch information
vmrajas committed Mar 24, 2021
1 parent 3ccc521 commit 8e3d025
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 10 deletions.
5 changes: 3 additions & 2 deletions graphql/api/panics.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@ import (
//
// If PanicHandler recovers from a panic, it logs a stack trace, creates an error
// and applies fn to the error.
func PanicHandler(fn func(error)) {
func PanicHandler(fn func(error), query string) {
if err := recover(); err != nil {
glog.Errorf("panic: %s.\n trace: %s", err, string(debug.Stack()))
// Log the panic along with query which caused it.
glog.Errorf("panic: %s.\n query: %s\n trace: %s", err, query, string(debug.Stack()))

fn(errors.Errorf("Internal Server Error - a panic was trapped. " +
"This indicates a bug in the GraphQL server. A stack trace was logged. " +
Expand Down
23 changes: 16 additions & 7 deletions graphql/resolve/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ func New(s schema.Schema, resolverFactory ResolverFactory) *RequestResolver {
// r.GqlReq should be set with a request before Resolve is called
// and a schema and backend Dgraph should have been added.
// Resolve records any errors in the response's error field.
func (r *RequestResolver) Resolve(ctx context.Context, gqlReq *schema.Request) *schema.Response {
func (r *RequestResolver) Resolve(ctx context.Context, gqlReq *schema.Request) (resp *schema.Response) {
span := otrace.FromContext(ctx)
stop := x.SpanTimer(span, methodResolve)
defer stop()
Expand All @@ -418,14 +418,22 @@ func (r *RequestResolver) Resolve(ctx context.Context, gqlReq *schema.Request) *
}

startTime := time.Now()
resp := &schema.Response{
resp = &schema.Response{
Extensions: &schema.Extensions{
Tracing: &schema.Trace{
Version: 1,
StartTime: startTime.Format(time.RFC3339Nano),
},
},
}
// Panic Handler for mutation. This ensures that the mutation which causes panic
// gets logged in Alpha logs. This panic handler overrides the default Panic Handler
// used in recoveryHandler in admin/http.go
defer api.PanicHandler(
func(err error) {
resp.Errors = schema.AsGQLErrors(schema.AppendGQLErrs(resp.Errors, err))
}, gqlReq.Query)

defer func() {
endTime := time.Now()
resp.Extensions.Tracing.EndTime = endTime.Format(time.RFC3339Nano)
Expand All @@ -435,7 +443,8 @@ func (r *RequestResolver) Resolve(ctx context.Context, gqlReq *schema.Request) *

op, err := r.schema.Operation(gqlReq)
if err != nil {
return schema.ErrorResponse(err)
resp.Errors = schema.AsGQLErrors(err)
return
}

if glog.V(3) {
Expand Down Expand Up @@ -472,7 +481,7 @@ func (r *RequestResolver) Resolve(ctx context.Context, gqlReq *schema.Request) *
Field: q,
Err: err,
}
})
}, gqlReq.Query)
allResolved[storeAt] = r.resolvers.queryResolverFor(q).Resolve(ctx, q)
}(q, i)
}
Expand Down Expand Up @@ -861,7 +870,7 @@ func resolveCustomField(ctx context.Context, f schema.Field, vals []interface{},
errCh chan error) {
defer api.PanicHandler(func(err error) {
errCh <- internalServerError(err, f)
})
}, "")

fconf, err := f.CustomHTTPConfig()
if err != nil {
Expand Down Expand Up @@ -999,7 +1008,7 @@ func resolveCustomField(ctx context.Context, f schema.Field, vals []interface{},
defer api.PanicHandler(
func(err error) {
errChan <- internalServerError(err, f)
})
}, "")

requestInput := input
if graphql {
Expand Down Expand Up @@ -1113,7 +1122,7 @@ func resolveNestedFields(ctx context.Context, f schema.Field, vals []interface{}
errCh chan error) {
defer api.PanicHandler(func(err error) {
errCh <- internalServerError(err, f)
})
}, "")

// If this field doesn't have custom directive and also doesn't have any children,
// then there is nothing to do and we can just continue.
Expand Down
2 changes: 1 addition & 1 deletion graphql/web/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ func recoveryHandler(next http.Handler) http.Handler {
func(err error) {
rr := schema.ErrorResponse(err)
write(w, rr, strings.Contains(r.Header.Get("Accept-Encoding"), "gzip"))
})
}, "")

next.ServeHTTP(w, r)
})
Expand Down

0 comments on commit 8e3d025

Please sign in to comment.