Rethrow human-readable error on schema introspection #50
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wawhal
suggested changes
Oct 15, 2019
There was a problem hiding this comment.
Functionality LGTM.
Added some code review comments.
Thanks for the PR @BlackDahila :)
src/ui.js
Outdated
| @@ -153,7 +153,23 @@ const executeQueryFromTerminalUI = async (queryOptions, successCb, errorCb) => | |||
| headers, | |||
| } = queryOptions; | |||
| cli.action.start('Introspecting schema'); | |||
| const schemaResponse = await query({endpoint: endpoint, query: introspectionQuery, headers: headers}); | |||
|
|
|||
| const schemaResponse = await query({endpoint: endpoint, query: introspectionQuery, headers: headers}) | |||
There was a problem hiding this comment.
A better readable way for catching errors in the async await pattern is:
let schemaResponse;
try {
schemaResponse = await query({...})
} catch (e) {
// throw
}| const queryOptions = { | ||
| query: queryString, | ||
| endpoint: endpoint, | ||
| endpoint, |
There was a problem hiding this comment.
Maybe you want to do this assignment after you check if queryString is null.
Assigning here and spreading later is just an extra step IMO.
added 2 commits
October 15, 2019 15:16
wawhal
suggested changes
Oct 16, 2019
| try { | ||
| schemaResponse = await query({endpoint: endpoint, query: introspectionQuery, headers: headers}); | ||
| } catch (err) { | ||
| if (err.message && err.message.startsWith('Network error: Unexpected token')) { |
There was a problem hiding this comment.
@BlackDahila you might want to check this condition. I tried running the command:
./bin/run http://localhost:random --introspectIt gave me this error:
Executing query... error
Error: ApolloError: Network error: request to http://localhost/:random failed, reason: connect ECONNREFUSED 127.0.0.1:80
at new ApolloError (/home/wawhal/oss/graphql/graphqurl/node_modules/apollo-client/bundle.umd.js:124:32)
at /home/wawhal/oss/graphql/graphqurl/node_modules/apollo-client/bundle.umd.js:1248:45
at /home/wawhal/oss/graphql/graphqurl/node_modules/apollo-client/bundle.umd.js:1680:21
at Array.forEach (<anonymous>)
at /home/wawhal/oss/graphql/graphqurl/node_modules/apollo-client/bundle.umd.js:1679:22
at Map.forEach (<anonymous>)
at QueryManager.broadcastQueries (/home/wawhal/oss/graphql/graphqurl/node_modules/apollo-client/bundle.umd.js:1672:26)
at /home/wawhal/oss/graphql/graphqurl/node_modules/apollo-client/bundle.umd.js:1175:35
at processTicksAndRejections (internal/process/task_queues.js:85:5) {
graphQLErrors: [],
networkError: FetchError: request to http://localhost/:random failed, reason: connect ECONNREFUSED 127.0.0.1:80
at ClientRequest.<anonymous> (/home/wawhal/oss/graphql/graphqurl/node_modules/node-fetch/lib/index.js:1345:11)
at ClientRequest.emit (events.js:203:13)
at Socket.socketErrorListener (_http_client.js:399:9)
at Socket.emit (events.js:203:13)
at emitErrorNT (internal/streams/destroy.js:91:8)
at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
at processTicksAndRejections (internal/process/task_queues.js:77:11) {
message: 'request to http://localhost/:random failed, reason: connect ECONNREFUSED 127.0.0.1:80',
type: 'system',
errno: 'ECONNREFUSED',
code: 'ECONNREFUSED'
},
message: 'Network error: request to http://localhost/:random failed, reason: connect ECONNREFUSED 127.0.0.1:80',
extraInfo: undefined
}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #18.
This PR shows a human-readable error when schema introspection fails on JSON parsing.
Before
After
Questions
I've rethrown the error in ui.js but I'm not 100% sure if it's the proper place.
My first attempt was to wrap lines 59 to 72 of command.js in a try-catch and doing it there. (That's why I moved
queryOptionsassignment a little bit higher. I've decided to leave it to make the code a little bit more DRY.)If there's anything that needs some more work just let me know :)