Skip to content

Add helpful message for invalid API key #1413

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.

Sign up for GitHub

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

Merged
JakeDawkins merged 4 commits into from
Jul 19, 2019
Merged

Add helpful message for invalid API key #1413

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
185b952
Add check for api-key/serviceName match
JakeDawkins Jul 17, 2019
b530728
Update changelog
JakeDawkins Jul 18, 2019
1658d45
Addressed PR comments/types
JakeDawkins Jul 19, 2019
3441e83
update changelog
JakeDawkins Jul 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/apollo-language-server/src/engine/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,12 @@ export class ApolloEngineClient extends GraphQLDataSource {
}
});

if (!(data && data.service)) {
throw new Error();
if (!(data && data.service) || errors) {
throw new Error(
Copy link
Copy Markdown
Contributor Author

@JakeDawkins JakeDawkins Jul 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we can't find a service, we should at least give a recommendation for why

errors
? errors.map(error => error.message).join("\n")
: "No service returned. Make sure your service name and API key match"
);
}

const schemaTags: string[] = data.service.schemaTags.map(
Expand Down
25 changes: 15 additions & 10 deletions packages/apollo-language-server/src/project/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Copy Markdown
Contributor Author

@JakeDawkins JakeDawkins Jul 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)

Copy link
Copy Markdown
Contributor

@trevor-scheer trevor-scheer Jul 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Boolean(this.serviceID && (this.schema || this.serviceSchema))

lastFetch: this.lastLoadDate
};
}
Expand Down Expand Up @@ -271,15 +272,19 @@ export class GraphQLClientProject extends GraphQLProject {
await this.loadingHandler.handle(
`Loading Engine data for ${this.displayName}`,
(async () => {
const {
schemaTags,
fieldStats
} = await engineClient.loadSchemaTagsAndFieldStats(serviceID);
this._onSchemaTags && this._onSchemaTags([serviceID, schemaTags]);
this.fieldStats = fieldStats;
this.lastLoadDate = +new Date();

this.generateDecorations();
try {
const {
schemaTags,
fieldStats
} = await engineClient.loadSchemaTagsAndFieldStats(serviceID);
this._onSchemaTags && this._onSchemaTags([serviceID, schemaTags]);
this.fieldStats = fieldStats;
this.lastLoadDate = +new Date();

this.generateDecorations();
} catch (e) {
console.error(e);
Copy link
Copy Markdown
Contributor Author

@JakeDawkins JakeDawkins Jul 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just wrapped this in a try/catch so we're not letting any unhandled rejections through

Copy link
Copy Markdown
Contributor

@trevor-scheer trevor-scheer Jul 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

@JakeDawkins JakeDawkins Jul 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be here or else I get an unhandled rejection error 😬 I'd love to know how to properly fix this though

}
})()
);
}
Expand Down
10 changes: 10 additions & 0 deletions packages/apollo-language-server/src/providers/schema/engine.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import gql from "graphql-tag";
import { GraphQLSchema, buildClientSchema } from "graphql";
import { ApolloEngineClient, ClientIdentity } from "../../engine";
import { ClientConfig, parseServiceSpecifier } from "../../config";
import { getServiceFromKey } from "../../config/utils";
import {
GraphQLSchemaProvider,
SchemaChangeUnsubscribeHandler,
Expand Down Expand Up @@ -44,6 +45,15 @@ export class EngineSchemaProvider implements GraphQLSchemaProvider {
}

const [id, tag = "current"] = parseServiceSpecifier(client.service);

// make sure the API key is valid for the service we're requesting a schema of.
const keyServiceName = getServiceFromKey(engine.apiKey);
if (id !== keyServiceName) {
Copy link
Copy Markdown
Contributor

@trevor-scheer trevor-scheer Jul 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yesssss

throw new Error(
`API key service name (${keyServiceName}) does not match the service name in your config (${id}). Try changing the service name in your config to ${keyServiceName} or get a new key.`
);
}

const { data, errors } = await this.client.execute<GetSchemaByTag>({
query: SCHEMA_QUERY,
variables: {
Expand Down
⚡