Ensure startup errors are redacted even the first time

This is a regression in #4981. If the server start process is begun implicitly
by the execution of an operation (ensureStarted inside graphQLServerOptions)
rather than by an explicit start call (or the implicit start calls in
`apollo-server` or the serverless integrations) and startup throws, the
log-and-redact logic wasn't being invoked.

Add some tests of this behavior and fix some TypeScript issues in the test file.

packages/apollo-server-core/src/ApolloServer.ts

@@ -623,7 +623,12 @@ export class ApolloServerBase {
       switch (this.state.phase) {
         case 'initialized with gateway':
         case 'initialized with schema':
-          await this._start();
+          try {
+            await this._start();
+          } catch {
+            // Any thrown error should transition us to 'failed to start', and
+            // we'll handle that on the next iteration of the while loop.
+          }
           // continue the while loop
           break;
         case 'starting':
@@ -632,9 +637,8 @@ export class ApolloServerBase {
           // continue the while loop
           break;
         case 'failed to start':
-          //  First
-          // we log the error that prevented startup (which means it will get logged
-          // once for every GraphQL operation).
+          // First we log the error that prevented startup (which means it will
+          // get logged once for every GraphQL operation).
           this.logStartupError(this.state.error);
           // Now make the operation itself fail.
           // We intentionally do not re-throw actual startup error as it may contain
@@ -1270,7 +1274,10 @@ export class ApolloServerBase {
    * `{req: express.Request, res: express.Response }` object) and to keep it
    * updated as you upgrade Apollo Server.
    */
-  public async executeOperation(request: GraphQLRequest, integrationContextArgument?: Record<string, any>) {
+  public async executeOperation(
+    request: GraphQLRequest,
+    integrationContextArgument?: Record<string, any>,
+  ) {
     const options = await this.graphQLServerOptions(integrationContextArgument);
 
     if (typeof options.context === 'function') {

packages/apollo-server-core/src/__tests__/ApolloServerBase.test.ts

@@ -2,6 +2,8 @@ import { ApolloServerBase } from '../ApolloServer';
 import { buildServiceDefinition } from '@apollographql/apollo-tools';
 import gql from 'graphql-tag';
 import { Logger } from 'apollo-server-types';
+import { ApolloServerPlugin } from 'apollo-server-plugin-base';
+import type { GraphQLSchema } from 'graphql';
 
 const typeDefs = gql`
   type Query {
@@ -36,7 +38,6 @@ describe('ApolloServerBase construction', () => {
   });
 
   it('succeeds when passed a graphVariant in construction', () => {
-    let serverBase;
     expect(() =>
       new ApolloServerBase({
         typeDefs,
@@ -84,7 +85,7 @@ describe('ApolloServerBase construction', () => {
   it('throws when a GraphQLSchema is not provided to the schema configuration option', () => {
     expect(() => {
       new ApolloServerBase({
-        schema: {},
+        schema: {} as GraphQLSchema,
       });
     }).toThrowErrorMatchingInlineSnapshot(
       `"Expected {} to be a GraphQL schema."`,
@@ -100,6 +101,58 @@ describe('ApolloServerBase construction', () => {
   });
 });
 
+describe('ApolloServerBase start', () => {
+  const failToStartPlugin: ApolloServerPlugin = {
+    async serverWillStart() {
+      throw Error('nope');
+    },
+  };
+  const redactedMessage =
+    'This data graph is missing a valid configuration. More details may be available in the server logs.';
+
+  it('start throws on startup error', async () => {
+    const server = new ApolloServerBase({
+      typeDefs,
+      resolvers,
+      plugins: [failToStartPlugin],
+    });
+    await expect(server.start()).rejects.toThrow('nope');
+  });
+
+  it('execute throws redacted message on implicit startup error', async () => {
+    const error = jest.fn();
+    const logger: Logger = {
+      debug: jest.fn(),
+      info: jest.fn(),
+      warn: jest.fn(),
+      error,
+    };
+
+    const server = new ApolloServerBase({
+      typeDefs,
+      resolvers,
+      plugins: [failToStartPlugin],
+      logger,
+    });
+    // Run the operation twice (the first will kick off the start process). We
+    // want to see the same error thrown and log message for the "kick it off"
+    // call as the subsequent call.
+    await expect(
+      server.executeOperation({ query: '{__typename}' }),
+    ).rejects.toThrow(redactedMessage);
+    await expect(
+      server.executeOperation({ query: '{__typename}' }),
+    ).rejects.toThrow(redactedMessage);
+    expect(error).toHaveBeenCalledTimes(2);
+    expect(error.mock.calls[0][0]).toMatch(
+      /Apollo Server was started implicitly.*nope/,
+    );
+    expect(error.mock.calls[1][0]).toMatch(
+      /Apollo Server was started implicitly.*nope/,
+    );
+  });
+});
+
 describe('ApolloServerBase executeOperation', () => {
   it('returns error information without details by default', async () => {
     const server = new ApolloServerBase({