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

Provide errors response for error handling #41

Open
int128 opened this Issue Feb 21, 2019 · 3 comments

Comments

3 participants
@int128
Copy link

int128 commented Feb 21, 2019

GitHub GraphQL API returns errors response on error cases such as not found, for example:

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

Currently Query() does not provide any errors but it should do for error handling, for example:

err := client.Query(ctx, &q, vars)
if errors, ok := githubv4.Errors(err); ok {
  if len(errors) > 0 && errors[0].Type() == "NOT_FOUND" {
    // not found case
  }
}

Thank you for the great work!

@int128

This comment has been minimized.

Copy link
Author

int128 commented Feb 21, 2019

This is workaround, we can identify if errors is provided by checking pointer type as follows:

var q *struct{}
if err := client.Query(ctx, &q, vars); err != nil {
  if q != nil {
    return errors.Wrapf(err, "this is an error such as not found or invalid query")
  }
  return errors.Wrapf(err, "this is an error such as network or bad json")
}
@mweibel

This comment has been minimized.

Copy link

mweibel commented Mar 7, 2019

👍
I think actually shurcooL/graphql#31 or rather shurcooL/graphql#33 should fix this and this library here doesn't need to change much.
Is there anything we can do to help bring this forward?

@dmitshur

This comment has been minimized.

Copy link
Member

dmitshur commented Apr 11, 2019

I think actually shurcooL/graphql#31 or rather shurcooL/graphql#33 should fix this and this library here doesn't need to change much.

Agreed.

Is there anything we can do to help bring this forward?

Sorry, I won't have the ability to focus on this for the next few weeks, but I'd like to after that. This is an important API decision, and I'd like to take care to find a good solution rather than rushing.

I suggest iterating on this on forks as a workaround if you need a solution sooner. Reporting results here would be helpful. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.