Handle user-specified schema more defensively. (#3462)

* 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
* Apply suggestions from code review
* Adjust test snapshots based on previous commit.
* Add CHANGELOG.md for #3462.

CHANGELOG.md

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

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 (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.");
     }
   }
 

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(
+      `"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"`,
+    );
+  });
+});