Add helpful message for invalid API key #1413
Conversation
JakeDawkins
force-pushed
the
jake/api-key-error
branch
from
a4c2b50
to
7704643
Compare
JakeDawkins
force-pushed
the
jake/api-key-error
branch
from
d426e9f
to
185b952
Compare
| @@ -169,7 +169,8 @@ export class GraphQLClientProject extends GraphQLProject { | |||
| total: totalTypes | |||
| }, | |||
| tag: this.config.tag, | |||
| loaded: this.serviceID ? true : false, | |||
| loaded: | |||
| this.serviceID && (this.schema || this.serviceSchema) ? true : false, | |||
There was a problem hiding this comment.
This checks to make sure the project actually has a schema. Previously, this would pass if there are 0 types on both the client and service, and it would show users in the console that everything was fine. This is not true :)
There was a problem hiding this comment.
Nit: Boolean(this.serviceID && (this.schema || this.serviceSchema))
|
|
||
| this.generateDecorations(); | ||
| } catch (e) { | ||
| console.error(e); |
There was a problem hiding this comment.
Just wrapped this in a try/catch so we're not letting any unhandled rejections through
There was a problem hiding this comment.
Just making a note that I believe that errors here are meant to bubble up to the LoadingHandler. They're try/catch in the handler so it can communicate them to the extension. This may be a better UX or we may need to improve the messages we send across the connection.
There was a problem hiding this comment.
This has to be here or else I get an unhandled rejection error 😬 I'd love to know how to properly fix this though
| if (!(data && data.service)) { | ||
| throw new Error(); | ||
| if (!(data && data.service) || errors) { | ||
| throw new Error( |
There was a problem hiding this comment.
if we can't find a service, we should at least give a recommendation for why
| @@ -169,7 +169,8 @@ export class GraphQLClientProject extends GraphQLProject { | |||
| total: totalTypes | |||
| }, | |||
| tag: this.config.tag, | |||
| loaded: this.serviceID ? true : false, | |||
| loaded: | |||
| this.serviceID && (this.schema || this.serviceSchema) ? true : false, | |||
There was a problem hiding this comment.
Nit: Boolean(this.serviceID && (this.schema || this.serviceSchema))
|
|
||
| this.generateDecorations(); | ||
| } catch (e) { | ||
| console.error(e); |
There was a problem hiding this comment.
Just making a note that I believe that errors here are meant to bubble up to the LoadingHandler. They're try/catch in the handler so it can communicate them to the extension. This may be a better UX or we may need to improve the messages we send across the connection.
|
|
||
| // make sure the API key is valid for the service we're requesting a schema of. | ||
| const keyServiceName = getServiceFromKey(engine.apiKey); | ||
| if (id !== keyServiceName) { |
- Fixed unneeded ternary - Added ApolloConfig as an `apollo` type export
* Add check for api-key/serviceName match * Added ApolloConfig as an `apollo` type export
Right now, there's no error being printed when schema lookup fails.
This PR aims to fix that, and add at least a safeguard for people using incorrect API keys. This can be partially checked on the client by checking the
config.service.nameagainst the identifier in the API key.See the comments in the file changes for more explanation.
For the future, we really should standardize our error strategy to make sure they're being printed prettily in the editor. Right now, we're getting some pretty ugly stack traces.
TODO:
*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.