Skip to content

Add optional Type to FormattedError #238

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

Closed
wants to merge 2 commits into from

Conversation

maxsz
Copy link

@maxsz maxsz commented Aug 17, 2017

This is how github's API exposes machine-readable error strings. We need this in our API to better be able to handle specific errors in the client. This is just a proposal to start a discussion on how to best handle this requirement 😃

For example:

Query:

{
  repository(owner:"bla",name:"blubb") {
    id
  }
}

Response:

{
  "data": {
    "repository": null
  },
  "errors": [
    {
      "message": "Could not resolve to a Repository with the name 'blubb'.",
      "type": "NOT_FOUND",
      "path": [
        "repository"
      ],
      "locations": [
        {
          "line": 2,
          "column": 3
        }
      ]
    }
  ]
}

@coveralls
Copy link

coveralls commented Aug 17, 2017

Coverage Status

Coverage increased (+0.01%) to 82.078% when pulling 816804e on equinux:formatted-error-type into 105a6c2 on graphql-go:master.

@coveralls
Copy link

coveralls commented Aug 23, 2017

Coverage Status

Coverage decreased (-0.02%) to 82.054% when pulling 263d35c on equinux:formatted-error-type into ed4c4d8 on graphql-go:master.

@coveralls
Copy link

coveralls commented Sep 14, 2017

Coverage Status

Coverage increased (+0.01%) to 82.054% when pulling 77d686b on equinux:formatted-error-type into 4b17eb6 on graphql-go:master.

@coveralls
Copy link

coveralls commented Jan 19, 2018

Coverage Status

Coverage decreased (-0.1%) to 80.927% when pulling c2ae40a on equinux:formatted-error-type into 1c504cf on graphql-go:master.

@maxsz
Copy link
Author

maxsz commented Jan 19, 2018

What do you think @sogko, @chris-ramon?

@christianpv
Copy link
Contributor

@maxsz I believe we have to do something like this instead: graphql/graphql-js@0b2e81c#diff-f8e4bedf89ec2148ccb1602bfc6a7ae7

This is how github's API exposes machine-readable error strings.
@maxsz maxsz force-pushed the formatted-error-type branch from a05aae4 to 3b63e33 Compare January 22, 2018 09:55
@maxsz
Copy link
Author

maxsz commented Jan 22, 2018

@christianpv looks good! I've updated the PR 👍

@dvic
Copy link
Contributor

dvic commented Feb 14, 2018

I went with a different approach in #279 and chose just to store the original error with FormattedError and move the responsibility of rendering errors differently up the chain. This enables me easily to render my domain errors differently, since I now have access to the actual underlying error. Any thoughts on this approach?

@appleboy
Copy link
Contributor

Any update on this?

@chris-ramon
Copy link
Member

chris-ramon commented Jul 19, 2018

Thanks a lot @maxsz! — latest spec of GraphQL details how to expose additional information about errors via extensions:

{
  "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"
      }
    }
  ]
}

We have a PR for it, so closing this one in favor of: #363

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.

6 participants