Use error instead of *gqlerror.Error

[vektah]

Use error instead of *gqlerror.Error

[vektah]

you can add something like this at the end of the go.mod

replace github.com/vektah/gqlparser => /home/you/projects/gqlparser

and you're done!

One of the best go modules features

[vektah]

Yeah this is an disappointing gotcha in go. I wish it would get fixed.

Returning error is fine, but its really only returning one type of error and gqlgen is going to reach into it on the other side. This results in interface overhead and type assertion spaghetti.

All that said, would be open to a pr if you want to do the conversion in gqlgen too.

[robojones]

I've opened a PR for gqlparser but I'm having trouble adding the conversions you mentioned to gqlgen.
How do I make my gqlgen fork use my gqlparser fork?
If this doesn't work, how will I be able to test my changes?

[robojones]

Okay thanks 😄

[AndrewWPhillips]

I wasted a lot of time trying to work out why ParseQuery was always returning an error (it wasn't).

func Do(schema *ast.Schema, s string) (r string, err error) {
	var query *ast.QueryDocument
	query, err = parser.ParseQuery(&ast.Source{Name: "query", Input: s})
	if err != nil {
		return // err is not nil even when nil is returned for error from ParseQuery
	}
	...
}

Please just change all error return types to error.

[JohnStarich]

I hit the same issue when I first picked this up, too. It's definitely a surprising behavior, compared to most (if not all) Go libraries I've worked with over the years.

Returning error is fine, but its really only returning one type of error and gqlgen is going to reach into it on the other side. This results in interface overhead and type assertion spaghetti.

@vektah As an idea, I think most potential type assertions in gqlgen could be handled using the built-in errors package. For example:

err := gqlparser.LoadSchema(files...)
var gqlErr *gqlerror.Error
if !errors.As(err, &gqlErr) {
    return err
}
// do gqlparser-specific error handling here
gqlErr.Message

At the very least, the spaghetti could be cleaner spaghetti 😉

[fredzqm]

I have a counter-argument.

It looks like the parsing library never return errors other than *gqlerror.Error. It would be nice to capture this characteristics and avoid seemingly unsafe conversion from error back to *gqlerror.Error.

Can I propose a pure additive change?

• For all private methods, return *gqlerror.Error.
• For some public methods, add a version that returns *gqlerror.Error and make the current method wraps it and return error.