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

Some GraphQL servers return an empty error array, even though it's not spec compliant #156

Closed
WilliamHolmes opened this issue Apr 27, 2016 · 23 comments
Assignees
Labels
Milestone

Comments

@WilliamHolmes
Copy link

WilliamHolmes commented Apr 27, 2016

Our GraphQL schema QueryRoot contains a type Viewer which we use to query most of the data.
Apollo doesn't seem to like this causing the error
Can't find field viewer on object [object Object]
I have validated (and verified the response) with this simple query using GraphiQL
Query
{ viewer { me { displayName } } }
Response
{ "data": { "viewer": { "me": { "displayName": "William Holmes" } } }, "errors": [] }

REF: apollostack/apollo-client/src/networkInterface.ts#L91

I'm using React Integration with mapQueriesToProps and connect

export default connect({ mapQueriesToProps })(MyComponent);

@stubailo stubailo self-assigned this Apr 27, 2016
@stubailo
Copy link
Contributor

I'll take a look soon, thanks for reporting!

@WilliamHolmes
Copy link
Author

Thanks @stubailo
Let me know if you need more info.
I don't think I'm doing anything out of the ordinary, I've followed the docs and added the apollo reducer & middleware to the redux store (and I do see it in the redux dev tools)

I really do like the approach you guys are taking with this apollo-client, I'm looking forward to integrated it. 😄

@abhiaiyer91
Copy link
Contributor

Could we get a code sample of the react component too?

@jbaxleyiii
Copy link
Contributor

@WilliamHolmes what version of react-apollo are you using, this sounds like it may be an issue with the connect component

@WilliamHolmes
Copy link
Author

@jbaxleyiii

"apollo-client": "0.1.6",
"react": "0.14.3",
"react-apollo": "0.1.6",
"redux": "3.0.4",

hmm. I have a feeling your about to tell me to update my react components. 😄

@jbaxleyiii
Copy link
Contributor

jbaxleyiii commented Apr 29, 2016

@WilliamHolmes maybe! It depends on how you are accessing the data from apollo-react (if that is what is causing the error)

the props from your query:
v0.1.*

{
  loading: bool,
  result: {
    viewer: {}
  },
}

v0.2.*

{
  loading: bool,
  viewer: {}
}

@stubailo
Copy link
Contributor

stubailo commented May 2, 2016

@WilliamHolmes any updates? really curious to hear what is going on.

@stubailo stubailo removed their assignment May 2, 2016
@WilliamHolmes
Copy link
Author

@stubailo I was out for a few day, will try to update and test again.
@jbaxleyiii Right now I'm just logging out this.props and checking the console, so this shouldn't be an issue.
@abhiaiyer91 I'll trim back my React Comp and upload today for review. Thanks.

@WilliamHolmes
Copy link
Author

WilliamHolmes commented May 3, 2016

@stubailo I've tried to reduce the code down to the basic modules (still having the same issue)
My Git Repo
You will need to edit /src/index.js to point to your own GraphQL server.

NOTE: package.json : I've updated all the versions, but still the same error.

"apollo-client": "0.2.0",
"react-apollo": "0.2.0",
"react-dom": "15.0.2",
"react-redux": "4.4.5",
"react": "15.0.2",
"redux": "3.5.2"

@abhiaiyer91
Copy link
Contributor

Where's your schema and resolvers?

@WilliamHolmes
Copy link
Author

WilliamHolmes commented May 3, 2016

@abhiaiyer91 I can't include our schema, but you can change the query in App.js to suit whatever schema your GraphQL server is using.

I've already verified locally that the Posted query and server response is what is expected.

Updated:

I also tried other queries to eliminate the "viewer key" with similar issues.
Query
{"query":"{\n node(id: "urn:lsid:[REMOVED]") {\n ... on Conversation {\n updated\n }\n }\n}\n"}

Server Response (Headers include: Content-Type:application/json)
{"data":{"node":{"updated":"2016-04-26T09:18:09.023+0000"}},"errors":[]}

Error in Props
"Can't find field node({"id":"urn:lsid:[REMOVED]"}) on object [object Object]."
image

@WilliamHolmes
Copy link
Author

It's the errors:[] attribute.
{"data":{"node":{"updated":"2016-04-26T09:18:09.023+0000"}},"errors":[]}

If it's not present (in the root) then everything works as expected, otherwise when its present we see this issue.

I believe the errors attribute (even if it's an empty array) is a valid GraphQL response.

@stubailo
Copy link
Contributor

stubailo commented May 3, 2016

Ooooooooooh! Thank you for identifying this. Seems related to #51

@WilliamHolmes what GraphQL server are you using?

It's a bummer but GraphQL doesn't really have enough guidelines for errors to treat them in a sane way....

@WilliamHolmes
Copy link
Author

@stubailo It's graphql-java

@stubailo
Copy link
Contributor

stubailo commented May 3, 2016

@WilliamHolmes on a scale of 1 to 10, how sure are you that the empty error array is the underlying problem? As in, should I rename this issue, or open a new one?

@WilliamHolmes
Copy link
Author

I'll commit at 10.
I've tested this with (not working) and without (working) the errors attribute.
I did not, however, test it with something other than an array, but I'm going to assume that would never happen. 😄

@stubailo stubailo changed the title "Can't find field viewer on object [object Object]." Treat empty errors array as no error May 3, 2016
@stubailo stubailo changed the title Treat empty errors array as no error Some GraphQL servers return an empty error array, even though it's not spec compliant May 3, 2016
@stubailo
Copy link
Contributor

stubailo commented May 3, 2016

@WilliamHolmes thanks for the confidence! Should be a trivial fix.

@helfer
Copy link
Contributor

helfer commented May 3, 2016

@WilliamHolmes I think you should file an issue on graphql-java. The GraphQL spec says:

The errors entry in the response is a non‐empty list of errors, where each error is a map.

If no errors were encountered during the requested operation, the errors entry should not be present in the result.

@stubailo
Copy link
Contributor

stubailo commented May 3, 2016

#lawyered

@WilliamHolmes
Copy link
Author

@helfer @stubailo

  1. Excellent News. I'll get on to the GraphQL server guys.
  2. Thanks so much for investigating this.
    Unless you want to add some robustness, I'm happy for you to close this issue.

@mquandalle
Copy link
Contributor

Maybe this client could provide a more comprehensible error message?

@stubailo
Copy link
Contributor

stubailo commented May 3, 2016

Yeah this is definitely still a "bug" in my mind! But now we have a specific cause isolated.

@stubailo stubailo self-assigned this May 3, 2016
@stubailo stubailo added this to the 5/10 cycle milestone May 3, 2016
stubailo pushed a commit that referenced this issue May 4, 2016
Fix #156 by accepting non-spec-compliant empty error array
stubailo pushed a commit that referenced this issue May 4, 2016
@stubailo
Copy link
Contributor

stubailo commented May 4, 2016

Fixing some of the internal error messages would be nice though so that these are easier to identify...

jbaxleyiii pushed a commit that referenced this issue Oct 18, 2017
add a note regarding reducer for custom redux actions
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants