Skip to content
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

Handle user-specified schema more defensively. #3462

Merged
merged 5 commits into from
Nov 13, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ The version headers in this history reflect the versions of Apollo Server itself

- `apollo-server-core`: Provide accurate type for `formatResponse` rather than generic `Function` type. [PR #3431](https://github.com/apollographql/apollo-server/pull/3431)
- `apollo-server-core`: Pass complete request context to `formatResponse`, rather than just `context`. [PR #3431](https://github.com/apollographql/apollo-server/pull/3431)
- `apollo-server-core`: Use `graphql`'s `isSchema` to more defensively check the user-specified schema's type at runtime and prevent unexpected errors. [PR #3462](https://github.com/apollographql/apollo-server/pull/3462)

### v2.9.7

Expand Down
7 changes: 5 additions & 2 deletions packages/apollo-server-core/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
DocumentNode,
isObjectType,
isScalarType,
isSchema,
} from 'graphql';
import { GraphQLExtension } from 'graphql-extensions';
import {
Expand Down Expand Up @@ -357,14 +358,16 @@ export class ApolloServerBase {
// TODO: This is a bit nasty because the subscription server needs this.schema synchronously, for reasons of backwards compatibility.
const _schema = this.initSchema();

if (_schema instanceof GraphQLSchema) {
if (isSchema(_schema)) {
const derivedData = this.generateSchemaDerivedData(_schema);
this.schema = derivedData.schema;
this.schemaDerivedData = Promise.resolve(derivedData);
} else {
} else if (typeof _schema.then === 'function') {
this.schemaDerivedData = _schema.then(schema =>
this.generateSchemaDerivedData(schema),
);
} else {
throw new Error("Unexpected error: Unable to resolve a valid GraphQLSchema. Please file an issue with a reproduction of this error, if possible.");
}
}

Expand Down
50 changes: 50 additions & 0 deletions packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { ApolloServerBase } from '../ApolloServer';
import { buildServiceDefinition } from '@apollographql/apollo-tools';
import gql from 'graphql-tag';

const typeDefs = gql`
type Query {
hello: String
}
`;

const resolvers = {
Query: {
hello() {
return 'world';
},
},
};

describe('ApolloServerBase construction', () => {
it('succeeds when a valid configuration options are provided to typeDefs and resolvers', () => {
expect(() => new ApolloServerBase({ typeDefs, resolvers })).not.toThrow();
});

it('succeeds when a valid GraphQLSchema is provided to the schema configuration option', () => {
expect(
() =>
new ApolloServerBase({
schema: buildServiceDefinition([{ typeDefs, resolvers }]).schema,
}),
).not.toThrow();
});

it('throws when a GraphQLSchema is not provided to the schema configuration option', () => {
expect(() => {
new ApolloServerBase({
schema: {},
});
}).toThrowErrorMatchingInlineSnapshot(
`"Unexpected error: Unable to resolve a valid GraphQLSchema. Please file an issue with a reproduction of this error, if possible."`,
);
});

it('throws when the no schema configuration option is provided', () => {
expect(() => {
new ApolloServerBase({});
}).toThrowErrorMatchingInlineSnapshot(
`"Apollo Server requires either an existing schema, modules or typeDefs"`,
);
});
});