Add several error types

[mspiegel]

This is a first attempt at implementing #31. Adds the types JSONMarshalError, JSONUnmarshalError, HTTPRequestError, HTTPResponseError, and UnmarshalError. These errors represent the different steps in the GraphQL request where errors can occur. Change the Errors struct to be exported by the package. Add 'Path' and 'Type' fields to the Errors struct. The 'path' field is in the GraphQL specification. The 'type' field is not in the GraphQL specification. 'type' is returned by the GitHub V4 API.

The implementation is a mixed success. The patch is providing me with some value. I can also see the argument that it has limited use. Here's how I use this new implementation in a GitHub service. For comparison I've also included the error handling logic for the GitHub V3 API:

func createErrorV3(resp *github.Response, err error) error {
	status := http.StatusInternalServerError
	if resp != nil {
		status = resp.StatusCode
	}
	// do stuff with the new http response code
}

func createErrorV4(err error) error {
	status := http.StatusInternalServerError
	switch e := err.(type) {
	case graphql.HTTPResponseError:
		status = e.Status
	case graphql.Errors:
		if e[0].Type == "NOT_FOUND" {
			status = http.StatusNotFound
		}
	}
	// do stuff with the new http response code
}

The HTTPResponseError type might be useful except I haven't validated yet that the GitHub API returns anything except for a 200 response code. I haven't tried making a V4 API request with the wrong authentication token. That request may return a 400-something response.

When the GitHub V4 API returns a GraphQL error, currently it populates the non-standard field "type" to define the error type. The error types are not documented. Maybe they will eventually be documented: https://platform.github.community/t/document-error-handling/5387 ?

[dmitshur]

Thanks for the PR. I'm going to need some time to review this and think about it.

One challenge with making progress on this is that it locks in HTTP-specific types in this package, potentially making it harder to resolve #5. This may not be a big problem, as it's likely that would require a breaking API change anyway.

Please feel free to ping me if I don't get back to this within a week.

[tgwizard]

Bump on this. shurcooL/graphql will presumably support HTTP transports even with pluggable transports, so exposing a graphql.HTTPResponseError would likely be forwards compatible. (You could move it to something like graphl/http and name it TransportResponseError or something).

Currently you need to inspect the err.Error() string in the returned error to see if the error you got is due to a 403, which isn't that nice.

[Napas]

Will this ever be done?

Current errors handling is just plain unusable.

[KenjiTakahashi]

Hey, Any chance of moving this forward?
We have a case where we have to perform some additional check on 401 response and it's currently not reasonably doable with github4 it seems. BTW: Yes, GitHub returns 4xx responses on auth errors.