From f20c5df0edff85acf83d42b468e0335ed0e3bfcc Mon Sep 17 00:00:00 2001 From: Trevor Scheer Date: Fri, 1 Nov 2019 11:27:38 -0700 Subject: [PATCH 1/4] Handle _schema types more carefully * Use the isSchema predicate instead of instanceof checks * Create a final `else` case for unexpected results of the _schema variable * Add tests around AS Base constructor to validate previous and new behavior --- .../apollo-server-core/src/ApolloServer.ts | 7 ++- .../src/__tests__/ApolloServerBase.test.ts | 50 +++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index 8a6dc3387e0..d5bd3a100e5 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -16,6 +16,7 @@ import { DocumentNode, isObjectType, isScalarType, + isSchema, } from 'graphql'; import { GraphQLExtension } from 'graphql-extensions'; import { @@ -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 (_schema.then) { this.schemaDerivedData = _schema.then(schema => this.generateSchemaDerivedData(schema), ); + } else { + throw new Error("ApolloServer encountered a programming error. For some reason, we were unable to resolve a valid GraphQLSchema. Please open an issue on the apollo-server repo with a reproduction of this error if possible."); } } diff --git a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts new file mode 100644 index 00000000000..362adfaceee --- /dev/null +++ b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts @@ -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( + `"ApolloServer encountered a programming error. For some reason, we were unable to resolve a valid GraphQLSchema. Please open an issue on the apollo-server repo 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"`, + ); + }); +}); From 6437e3d34fc3f8ed8fae14ac47a2135d7e4eebe6 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Wed, 13 Nov 2019 16:52:36 +0200 Subject: [PATCH 2/4] Apply suggestions from code review --- packages/apollo-server-core/src/ApolloServer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts index d5bd3a100e5..1d5ad694a0f 100644 --- a/packages/apollo-server-core/src/ApolloServer.ts +++ b/packages/apollo-server-core/src/ApolloServer.ts @@ -362,12 +362,12 @@ export class ApolloServerBase { const derivedData = this.generateSchemaDerivedData(_schema); this.schema = derivedData.schema; this.schemaDerivedData = Promise.resolve(derivedData); - } else if (_schema.then) { + } else if (typeof _schema.then === 'function') { this.schemaDerivedData = _schema.then(schema => this.generateSchemaDerivedData(schema), ); } else { - throw new Error("ApolloServer encountered a programming error. For some reason, we were unable to resolve a valid GraphQLSchema. Please open an issue on the apollo-server repo with a reproduction of this error if possible."); + throw new Error("Unexpected error: Unable to resolve a valid GraphQLSchema. Please file an issue with a reproduction of this error, if possible."); } } From 546bb04798be941a81ba2c7cb0ba312608776ab1 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Wed, 13 Nov 2019 16:57:02 +0200 Subject: [PATCH 3/4] Adjust test snapshots based on previous commit. --- .../apollo-server-core/src/__tests__/ApolloServerBase.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts index 362adfaceee..27af9c3347f 100644 --- a/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts +++ b/packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts @@ -36,7 +36,7 @@ describe('ApolloServerBase construction', () => { schema: {}, }); }).toThrowErrorMatchingInlineSnapshot( - `"ApolloServer encountered a programming error. For some reason, we were unable to resolve a valid GraphQLSchema. Please open an issue on the apollo-server repo with a reproduction of this error if possible."`, + `"Unexpected error: Unable to resolve a valid GraphQLSchema. Please file an issue with a reproduction of this error, if possible."`, ); }); From 0390f5e7c56dcb690d02e8212f72672faf4993f3 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Wed, 13 Nov 2019 17:01:14 +0200 Subject: [PATCH 4/4] Add CHANGELOG.md for #3462. --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fada8b81092..5820c5ac076 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,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