-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Report errors in parallel when batching #3605
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
Conversation
|
Deploy preview for hasura-docs ready! Built with commit 534fe10 |
|
@thefliik If you are interested in trying out this change, there should be a test environment available soon once CI finishes. Thanks! |
|
Review app for commit 63a55fc deployed to Heroku: https://hge-ci-pull-3605.herokuapp.com |
| - id: '2' | ||
| query: | ||
| - query: | | ||
| mutation { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this is poor form (updating the test data set from a test like this), so I try to put back the original data at the end so that it doesn't affect any other tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the test setup/teardown actions run around each individual test, not around the whole test group, so I think this is unnecessary.
|
Review app for commit 534fe10 deployed to Heroku: https://hge-ci-pull-3605.herokuapp.com |
|
This seems to work well with const ApolloClient = require('apollo-client').default;
const BatchHttpLink = require('apollo-link-batch-http').BatchHttpLink;
const InMemoryCache = require('apollo-cache-inmemory').InMemoryCache;
const fetch = require('node-fetch');
const gql = require('graphql-tag').default;
const link = new BatchHttpLink({
uri: 'http://localhost:8080/v1/graphql',
fetch
});
const client = new ApolloClient({ link, cache: new InMemoryCache() });
client.query({
query: gql`query first { user { id } }`,
}).then(user => console.log(user))
.catch(err => console.error(err.message));
client.query({
query: gql`query second { role { id } }`,
}).then(role => console.log(role))
.catch(err => console.error(err.message));Using netcat, I can see the requests get batched, and then errors get handled independently on the client. |
|
Review app https://hge-ci-pull-3605.herokuapp.com is deleted |
@paf31 looks like I was too slow on the draw. I'm not actually using batching in any projects at the moment though, so I probably couldn't have provided any new feedback anyway. 👍 |
* Report errors in parallel when batching * Add some more test cases
Fixes #1812
Description
Reports errors in parallel, treating each operation independently, when receiving batched requests.
WIP, because I would still like to figure out a good way to integrate this into the test suite, but I'm putting this PR up now so that I can get a test environment to share.
Affected components
Related Issues
#1812
Solution and Design
Steps to test and verify
Here is a simple test case on the terminal:
[ { "errors": [ { "extensions": { "code": "validation-failed", "path": "$.selectionSet.user" }, "message": "field \"user\" not found in type: 'query_root'" } ] }, { "errors": [ { "extensions": { "code": "validation-failed", "path": "$.selectionSet.role" }, "message": "field \"role\" not found in type: 'query_root'" } ] } ]Limitations, known bugs & workarounds
Server checklist
Catalog upgrade
Does this PR change Hasura Catalog version?
Metadata
Does this PR add a new Metadata feature?
GraphQL
Breaking changes