Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement June 2018 error spec #363

Merged
merged 2 commits into from
Jul 20, 2018

Conversation

ccbrown
Copy link
Contributor

@ccbrown ccbrown commented Jul 19, 2018

This implements section 7.1.2 of the June 2018 spec.

Paths are now added to errors:

  "errors": [
    {
      "message": "Name for character with ID 1002 could not be fetched.",
      "locations": [ { "line": 6, "column": 7 } ],
      "path": [ "hero", "heroFriends", 1, "name" ]
    }
  ],

If an error can be associated to a particular field in the GraphQL result, it must contain an entry with the key path that details the path of the response field which experienced the error.

And extensions are now present when resolve functions return implementations of gqlerrors.ExtendedError:

  "errors": [
    {
      "message": "Name for character with ID 1002 could not be fetched.",
      "locations": [ { "line": 6, "column": 7 } ],
      "path": [ "hero", "heroFriends", 1, "name" ],
      "extensions": {
        "code": "CAN_NOT_FETCH_BY_ID",
        "timestamp": "Fri Feb 9 14:33:09 UTC 2018"
      }
    }
  ]

GraphQL services may provide an additional entry to errors with key extensions. This entry, if set, must have a map as its value. This entry is reserved for implementors to add additional information to errors however they see fit, and there are no additional restrictions on its contents.

It's very much a transliteration of the JavaScript implementation. No creativity was involved here. I've gone through all the tests and painstakingly updated them. Those in addition to some new tests cover the changes extremely well.

This addresses #200, #259, #351, #109, and kinda sorta #312.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 91.765% when pulling 8752c8e on ccbrown:error-spec-catchup into 086b3bf on graphql-go:master.

@appleboy
Copy link
Contributor

LGTM. @chris-ramon When to merge this feature?

@@ -6,9 +6,16 @@ import (
"github.com/graphql-go/graphql/language/location"
)

type ExtendedError interface {
error
ErrorExtensions() map[string]interface{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be naming this function Extensions to avoid extra suffix.
It is addressed in a new PR: #364

@chris-ramon
Copy link
Member

Thanks a lot @ccbrown this looks very good to me! 👍 — Also thanks @appleboy for reviewing it as well. 🚢

@chris-ramon chris-ramon merged commit 5eec4f9 into graphql-go:master Jul 20, 2018
@ccbrown ccbrown deleted the error-spec-catchup branch July 20, 2018 04:43
@SebScoFr
Copy link

When do you plan to release this? We would love to have that change available 👍

@chris-ramon
Copy link
Member

When do you plan to release this? We would love to have that change available

Hi @SebScoFr, thanks a lot for using the lib!

I've just created a new release v0.7.6 which contains this PR along some other great improvements. 🚢

@jphuc96
Copy link

jphuc96 commented Aug 14, 2019

@ccbrown I am working on this "extension", may I have a simple example that implement this feature, as I find some difficult to implement the ExtendedError interface ? Thanks for the great PR

@ccbrown
Copy link
Contributor Author

ccbrown commented Aug 14, 2019

@jphuc96 here's an example:

type ErrorWithCode struct {
    Message string
    Code string
}

func (err *ErrorWithCode) Error() string {
    return err.Message
}

func (err *ErrorWithCode) Extensions() map[string]interface{} {
    return map[string]interface{}{
        "code": err.Code,
    }
}

// Not required but recommended:
// Makes sure at compile time that ErrorWithCode implements gqlerrors.ExtendedError
var _ gqlerrors.ExtendedError = (*ErrorWithCode)(nil)

Then in your resolver you can do something like this:

return nil, &ErrorWithCode{"Not authorized.", "not_authorized"}

@jphuc96
Copy link

jphuc96 commented Aug 14, 2019

@jphuc96 here's an example:

type ErrorWithCode struct {
    Message string
    Code string
}

func (err *ErrorWithCode) Error() string {
    return err.Message
}

func (err *ErrorWithCode) Extensions() map[string]interface{} {
    return map[string]interface{}{
        "code": err.Code,
    }
}

// Not required but recommended:
// Makes sure at compile time that ErrorWithCode implements gqlerrors.ExtendedError
var _ gqlerrors.ExtendedError = (*ErrorWithCode)(nil)

Then in your resolver you can do something like this:

return nil, &ErrorWithCode{"Not authorized.", "not_authorized"}

that really did the trick, thanks for saving me @ccbrown 👍 👍 👍

@appleboy
Copy link
Contributor

@jphuc96 You can ref the example code: https://github.com/go-ggz/ggz/blob/d3d22a347506cbe36ef24bdd229efb74a56c0541/pkg/errors/errors.go#L1

// Error applicational
type Error struct {
	Type    Type
	Message string
	cause   error
}

// Error message
func (e *Error) Error() string {
	return e.Message
}

// Cause of the original error
func (e *Error) Cause() string {
	if e.cause != nil {
		return e.cause.Error()
	}

	return ""
}

// Extensions for graphQL extension
func (e *Error) Extensions() map[string]interface{} {
	if e.cause != nil {
		log.Error().Err(e.cause).Msg("graphql error report")
	}

	return map[string]interface{}{
		"code": e.Type.Code(),
		"type": e.Type,
	}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants