Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
- `apollo-graphql`
- <First `apollo-graphql` related entry goes here>
- `apollo-language-server`
- <First `apollo-language-server` related entry goes here>
- Add error message for service lookup failure [#1413](https://github.com/apollographql/apollo-tooling/pull/1413)
- `apollo-tools`
- <First `apollo-tools` related entry goes here>
- `vscode-apollo`
- Only activate extension on apollo.config.js/ts [#1411](https://github.com/apollographql/apollo-tooling/pull/1411)
- Changed the status bar title to be "Apollo" to save space.
- Changed the status bar title to be "Apollo" to save space. [#1415](https://github.com/apollographql/apollo-tooling/pull/1415)

## `apollo@2.16.0`, `apollo-codegen-swift@0.34.0`, `apollo-language-server@1.13.0`, `apollo-tools@0.4.0`, `vscode-apollo@1.8.0`

Expand Down
8 changes: 6 additions & 2 deletions packages/apollo-language-server/src/engine/index.ts
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

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

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

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

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

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

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

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