Added Custom Error Formatter

[racerxdl]

I added support for a custom error formatter. Sometimes you come with custom error objects that might need special parsing to add correct extensions in a generic way by object type.

This implementation doesn't change anything to the flow if no Custom Handler is specified since it uses by default the FormatError method. However if you specify, it will run your custom function expecting a FormattedError to return.

Just a quick example:

func CustomFormat(err error) gqlerrors.FormattedError {
	switch err := err.(type) {
	case *gqlerrors.Error:
                // My custom error object returned in the mutation / query is encapsulated as OriginalError
		cf := CustomFormat(err.OriginalError)
		cf.Locations = err.Locations
		cf.Path = err.Path
		return cf
	case *QuantoError.ErrorObject:
                // Here I can call my custom ErrorObject ToFormattedError that will return an FormattedError with proper extensions set.
		return err.ToFormattedError()
	default:
		return gqlerrors.FormatError(err)
	}
}

Then you can use something like:

	// execute graphql query
	params := graphql.Params{
		Schema:         		*h.Schema,
		RequestString:  		opts.Query,
		VariableValues: 		opts.Variables,
		OperationName:  		opts.OperationName,
		Context:        		ctx,
		CustomErrorFomatter: 	CustomFormat,
	}
	result := graphql.Do(params)

And everything will work.

I'm also opening a PR in graphql-go/handler since it also benefits in this custom handler. I will put a example there as well.

[coveralls]

Coverage decreased (-0.02%) to 91.775% when pulling 517e86d on racerxdl:master into ef7caf8 on graphql-go:master.

[ccbrown]

I think there are a lot of ways you can already accomplish this without any library changes:

• Modify your error types to have the correct extensions.

• Modify your resolvers to return error types with the right extensions.

• Modify your schema to convert errors to types with the right extensions:

  func resolveWrapper(resolve graphql.FieldResolveFn) graphql.FieldResolveFn {
  	return func(p graphql.ResolveParams) (interface{}, error) {
  		v, err := resolve(p)
  		// modify v and err accordingly
  		return v, err
  	}
  }
  
  func WrapResolvers(schema *graphql.Schema) {
  	for typeName, t := range schema.TypeMap() {
  		if strings.HasPrefix(typeName, "__") {
  			continue
  		}
  		if obj, ok := t.(*graphql.Object); ok {
  			for _, f := range obj.Fields() {
  				if f.Resolve != nil {
  					f.Resolve = resolveWrapper(f.Resolve)
  				}
  			}
  		}
  	}
  }

[racerxdl]

Well, for sure. But the easier way is to make a function that can adapt the error as you want. That's also the default way for other graphql libraries in other languages (for example javascript).

[ccbrown]

That's also the default way for other graphql libraries in other languages (for example javascript).

Does the JavaScript implementation actually have this feature? If so, reference implementation parity would certainly be a good reason to add this.

[racerxdl]

Yes, it does. https://graphql.org/graphql-js/express-graphql/#graphqlhttp
The GraphQL HTTP Client has a formatError function that is called before sending the error back to the client.

I did basically in similar way, changing where nescessary to match go language (since javascript is untyped and dynamic, the formatError function is way more flexible)

[ccbrown]

Yes, it does. https://graphql.org/graphql-js/express-graphql/#graphqlhttp
The GraphQL HTTP Client has a formatError function that is called before sending the error back to the client.

That's not actually in the core JavaScript GraphQL implementation (graphql/graphql-js); it's part of a separate, higher level library (graphql/express-graphql).

In the same way that it's not part of the core implementation for JavaScript, my opinion is that it shouldn't be part of the core implementation here either. It would be more comparable to your example if the functionality went into one of the other higher level Go packages such as graphql-go/handler instead.

[racerxdl]

That's true. I would implement on handler only if it was possible, but it wasnt. Since it was not exported. That's why I added a customErrorFormater, which is optional.

[ccbrown]

Maybe a simpler change that would enable this would be if gqlerrors.FormattedError just stored and provided access to the original error? That might also help meet other error-related needs such as #312.

[racerxdl]

That might not solve the issue, since you have no access to that error before is sent back to the user, and adding original error directly to it might leak private information and / or break the GraphQLError format.

[ccbrown]

If you add an unexported originalError error field and an OriginalError() error method to FormattedError, it wouldn't effect the serialization. So it's easy to avoid that kind of leak or breakage.

If that were added, you could then just add something like this to graphql-go/handler:

if customFormatter := h.CustomErrorFormatter; customFormatter != nil {
    formatted := make([]gqlerrors.FormattedError, len(result.Errors))
    for i, err := result.Errors {
        formatted[i] = customFormatter(err.OriginalError())
    }
    result.Errors = formatted
}

[racerxdl]

Hmm, thats true. Let me try to do that.

[racerxdl]

Lol, now I need to check those errors. These weren't there XD

[zonr]

Your change to add an additional field for tracking back the underlying error can also be found in #279 (link). Those CI failures is because many tests use reflect.DeepEqual to compare the errors returning from GraphQL execution (which will have originalError or cause being set) against the one created by hand for assertion (which doesn't and can't set the field) (example). #279 addressed this issue by adding custom compare functions (link) for checking graphql.Result and gqlerrors.FormattedError in tests (link).

[limoli]

Any news about the possibility to custom the error output?
I think that is is very useful!
In my case, I need it in order to provide an error reference field besides the message.

[1046102779]

it may use upgrade handler, src

maybe try it!

demo:

[limoli]

Thank you @1046102779 !
Have you already sent a pull request about this?
It would be useful to have in the official version.

[limoli]

Any news?

[chris-ramon]

Thanks a lot for the great feedback and the initial work @racerxdl! — Closing this one in favor of: #423