Merge branch 'release-2.12.0' into trevor/gateway-retries

CHANGELOG.md

@@ -9,6 +9,7 @@ The version headers in this history reflect the versions of Apollo Server itself
 
 - `apollo-server-core`: When operating in gateway mode using the `gateway` property of the Apollo Server constructor options, the failure to initialize a schema during initial start-up, e.g. connectivity problems, will no longer result in the federated executor from being assigned when the schema eventually becomes available.  This precludes a state where the gateway may never become available to serve federated requests, even when failure conditions are no longer present. [PR #3811](https://github.com/apollographql/apollo-server/pull/3811)
 - `apollo-server-core`: Prevent a condition which prefixed an error message on each request when the initial gateway initialization resulted in a Promise-rejection which was memoized and re-prepended with `Invalid options provided to ApolloServer:` on each request. [PR #3811](https://github.com/apollographql/apollo-server/pull/3811)
+- `apollo-server-express`: Disable the automatic inclusion of the `x-powered-by: express` header. [PR #3821](https://github.com/apollographql/apollo-server/pull/3821)
 
 ### v2.11.0
 

packages/apollo-gateway/CHANGELOG.md

@@ -5,6 +5,11 @@
 - Several previously unhandled Promise rejection errors stemming from, e.g. connectivity, failures when communicating with Apollo Graph Manager within asynchronous code are now handled. [PR #3811](https://github.com/apollographql/apollo-server/pull/3811)
 - Provide a more helpful error message when encountering expected errors. [PR #3811](https://github.com/apollographql/apollo-server/pull/3811)
 - General improvements and clarity to error messages and logging. [PR #3811](https://github.com/apollographql/apollo-server/pull/3811)
+- Warn of a possible misconfiguration when local service configuration is provided (via `serviceList` or `localServiceList`) and a remote Apollo Graph Manager configuration is subsequently found as well. [PR #3868](https://github.com/apollographql/apollo-server/pull/3868)
+
+## 0.14.0 (pre-release; `@next` tag)
+
+- During composition, the unavailability of a downstream service in unmanaged federation mode will no longer result in a partially composed schema which merely lacks the types provided by the downed service.  This prevents unexpected validation errors for clients querying a graph which lacks types which were merely unavailable during the initial composition but were intended to be part of the graph. [PR #3867](https://github.com/apollographql/apollo-server/pull/3867)
 
 ## 0.13.2
 

packages/apollo-gateway/src/__tests__/integration/networkRequests.test.ts

@@ -17,6 +17,8 @@ import {
   graphId,
 } from './nockMocks';
 
+import loadServicesFromStorage = require("../../loadServicesFromStorage");
+
 // This is a nice DX hack for GraphQL code highlighting and formatting within the file.
 // Anything wrapped within the gql tag within this file is just a string, not an AST.
 const gql = String.raw;
@@ -108,6 +110,59 @@ it('Extracts service definitions from remote storage', async () => {
   expect(gateway.schema!.getType('User')!.description).toBe('This is my User');
 });
 
+it.each([
+  ['warned', 'present'],
+  ['not warned', 'absent'],
+])('conflicting configurations are %s about when %s', async (_word, mode) => {
+  const isConflict = mode === 'present';
+  const spyConsoleWarn = jest.spyOn(console, 'warn');
+  let blockerResolve: () => void;
+  const blocker = new Promise(resolve => (blockerResolve = resolve));
+  const original = loadServicesFromStorage.getServiceDefinitionsFromStorage;
+  const spyGetServiceDefinitionsFromStorage = jest
+    .spyOn(loadServicesFromStorage, 'getServiceDefinitionsFromStorage')
+    .mockImplementationOnce(async (...args) => {
+      try {
+        return await original(...args);
+      } catch (e) {
+        throw e;
+      } finally {
+        setImmediate(blockerResolve);
+      }
+    });
+
+  mockStorageSecretSuccess();
+  if (isConflict) {
+    mockCompositionConfigLinkSuccess();
+    mockCompositionConfigsSuccess([service.implementingServicePath]);
+    mockImplementingServicesSuccess(service);
+    mockRawPartialSchemaSuccess(service);
+  } else {
+    mockCompositionConfigLink().reply(403);
+  }
+
+  mockLocalhostSDLQuery({ url: service.federatedServiceURL }).reply(200, {
+    data: { _service: { sdl: service.federatedServiceSchema } },
+  });
+
+  const gateway = new ApolloGateway({
+    serviceList: [
+      { name: 'accounts', url: `${service.federatedServiceURL}/graphql` },
+    ],
+  });
+
+  await gateway.load({ engine: { apiKeyHash, graphId } });
+  await blocker; // Wait for the definitions to be "fetched".
+
+  (isConflict
+    ? expect(spyConsoleWarn)
+    : expect(spyConsoleWarn).not
+  ).toHaveBeenCalledWith(expect.stringMatching(
+    /A local gateway service list is overriding an Apollo Graph Manager managed configuration/));
+  spyGetServiceDefinitionsFromStorage.mockRestore();
+  spyConsoleWarn.mockRestore();
+});
+
 it('Rollsback to a previous schema when triggered', async () => {
   // Init
   mockStorageSecretSuccess();

packages/apollo-gateway/src/__tests__/loadServicesFromRemoteEndpoint.test.ts

@@ -0,0 +1,35 @@
+import { getServiceDefinitionsFromRemoteEndpoint } from '../loadServicesFromRemoteEndpoint';
+import { mockLocalhostSDLQuery } from './integration/nockMocks';
+import { RemoteGraphQLDataSource } from '../datasources';
+import nock = require('nock');
+
+describe('getServiceDefinitionsFromRemoteEndpoint', () => {
+  it('errors when no URL was specified', async () => {
+    const serviceSdlCache = new Map<string, string>();
+    const dataSource = new RemoteGraphQLDataSource({ url: '' });
+    const serviceList = [{ name: 'test', dataSource }];
+    await expect(
+      getServiceDefinitionsFromRemoteEndpoint({
+        serviceList,
+        serviceSdlCache,
+      }),
+    ).rejects.toThrowError(
+      "Tried to load schema for 'test' but no 'url' was specified.",
+    );
+  });
+
+  it('throws when the downstream service returns errors', async () => {
+    const serviceSdlCache = new Map<string, string>();
+    const host = 'http://host-which-better-not-resolve';
+    const url = host + '/graphql';
+
+    const dataSource = new RemoteGraphQLDataSource({ url });
+    const serviceList = [{ name: 'test', url, dataSource }];
+    await expect(
+      getServiceDefinitionsFromRemoteEndpoint({
+        serviceList,
+        serviceSdlCache,
+      }),
+    ).rejects.toThrowError(/^Couldn't load service definitions for "test" at http:\/\/host-which-better-not-resolve\/graphql: request to http:\/\/host-which-better-not-resolve\/graphql failed, reason: getaddrinfo ENOTFOUND/);
+  });
+});

packages/apollo-gateway/src/index.ts

@@ -151,6 +151,15 @@ type RequestContext<TContext> = WithRequired<
   GraphQLRequestContext<TContext>,
   'document' | 'queryHash'
 >;
+
+// Local state to track whether particular UX-improving warning messages have
+// already been emitted.  This is particularly useful to prevent recurring
+// warnings of the same type in, e.g. repeating timers, which don't provide
+// additional value when they are repeated over and over during the life-time
+// of a server.
+type WarnedStates = {
+  remoteWithLocalConfig?: boolean;
+};
 
 export const GCS_RETRY_COUNT = 5;
 
@@ -186,6 +195,7 @@ export class ApolloGateway implements GraphQLService {
   private serviceDefinitions: ServiceDefinition[] = [];
   private compositionMetadata?: CompositionMetadata;
   private serviceSdlCache = new Map<string, string>();
+  private warnedStates: WarnedStates = Object.create(null);
 
   private fetcher: typeof fetch = getDefaultGcsFetcher();
 
@@ -479,6 +489,38 @@ export class ApolloGateway implements GraphQLService {
   protected async loadServiceDefinitions(
     config: GatewayConfig,
   ): ReturnType<Experimental_UpdateServiceDefinitions> {
+    // This helper avoids the repetition of options in the two cases this method
+    // is invoked below. It is a helper, rather than an options object, since it
+    // depends on the presence of `this.engineConfig`, which is guarded against
+    // further down in this method in two separate places.
+    const getRemoteConfig = (engineConfig: GraphQLServiceEngineConfig) => {
+      return getServiceDefinitionsFromStorage({
+        graphId: engineConfig.graphId,
+        apiKeyHash: engineConfig.apiKeyHash,
+        graphVariant: engineConfig.graphVariant,
+        federationVersion:
+          (config as ManagedGatewayConfig).federationVersion || 1,
+        fetcher: this.fetcher,
+      });
+    };
+
+    if (isLocalConfig(config) || isRemoteConfig(config)) {
+      if (this.engineConfig && !this.warnedStates.remoteWithLocalConfig) {
+        // Only display this warning once per start-up.
+        this.warnedStates.remoteWithLocalConfig = true;
+        // This error helps avoid common misconfiguration.
+        // We don't await this because a local configuration should assume
+        // remote is unavailable for one reason or another.
+        getRemoteConfig(this.engineConfig).then(() => {
+          this.logger.warn(
+            "A local gateway service list is overriding an Apollo Graph " +
+            "Manager managed configuration.  To use the managed " +
+            "configuration, do not specifiy a service list locally.",
+          );
+        }).catch(() => {}); // Don't mind errors if managed config is missing.
+      }
+    }
+
     if (isLocalConfig(config)) {
       return { isNewSchema: false };
     }
@@ -504,13 +546,7 @@ export class ApolloGateway implements GraphQLService {
       );
     }
 
-    return getServiceDefinitionsFromStorage({
-      graphId: this.engineConfig.graphId,
-      apiKeyHash: this.engineConfig.apiKeyHash,
-      graphVariant: this.engineConfig.graphVariant,
-      federationVersion: config.federationVersion || 1,
-      fetcher: this.fetcher
-    });
+    return getRemoteConfig(this.engineConfig);
   }
 
   // XXX Nothing guarantees that the only errors thrown or returned in

packages/apollo-gateway/src/loadServicesFromRemoteEndpoint.ts

@@ -26,58 +26,51 @@ export async function getServiceDefinitionsFromRemoteEndpoint({
 
   let isNewSchema = false;
   // for each service, fetch its introspection schema
-  const serviceDefinitions: ServiceDefinition[] = (await Promise.all(
-    serviceList.map(({ name, url, dataSource }) => {
-      if (!url) {
-        throw new Error(
-          `Tried to load schema for '${name}' but no 'url' was specified.`);
-      }
+  const promiseOfServiceList = serviceList.map(({ name, url, dataSource }) => {
+    if (!url) {
+      throw new Error(
+        `Tried to load schema for '${name}' but no 'url' was specified.`);
+    }
 
-      const request: GraphQLRequest = {
-        query: 'query GetServiceDefinition { _service { sdl } }',
-        http: {
-          url,
-          method: 'POST',
-          headers: new Headers(headers),
-        },
-      };
+    const request: GraphQLRequest = {
+      query: 'query GetServiceDefinition { _service { sdl } }',
+      http: {
+        url,
+        method: 'POST',
+        headers: new Headers(headers),
+      },
+    };
 
-      return dataSource
-        .process({ request, context: {} })
-        .then(({ data, errors }) => {
-          if (data && !errors) {
-            const typeDefs = data._service.sdl as string;
-            const previousDefinition = serviceSdlCache.get(name);
-            // this lets us know if any downstream service has changed
-            // and we need to recalculate the schema
-            if (previousDefinition !== typeDefs) {
-              isNewSchema = true;
-            }
-            serviceSdlCache.set(name, typeDefs);
-            return {
-              name,
-              url,
-              typeDefs: parse(typeDefs),
-            };
+    return dataSource
+      .process({ request, context: {} })
+      .then(({ data, errors }): ServiceDefinition => {
+        if (data && !errors) {
+          const typeDefs = data._service.sdl as string;
+          const previousDefinition = serviceSdlCache.get(name);
+          // this lets us know if any downstream service has changed
+          // and we need to recalculate the schema
+          if (previousDefinition !== typeDefs) {
+            isNewSchema = true;
           }
+          serviceSdlCache.set(name, typeDefs);
+          return {
+            name,
+            url,
+            typeDefs: parse(typeDefs),
+          };
+        }
 
-          // XXX handle local errors better for local development
-          if (errors) {
-            errors.forEach(console.error);
-          }
+        throw new Error(errors?.map(e => e.message).join("\n"));
+      })
+      .catch(err => {
+        const errorMessage =
+          `Couldn't load service definitions for "${name}" at ${url}` +
+          (err && err.message ? ": " + err.message || err : "");
 
-          return false;
-        })
-        .catch(error => {
-          console.warn(
-            `Encountered error when loading ${name} at ${url}: ${error.message}`,
-          );
-          return false;
-        });
-    }),
-  ).then(serviceDefinitions =>
-    serviceDefinitions.filter(Boolean),
-  )) as ServiceDefinition[];
+        throw new Error(errorMessage);
+      });
+  });
 
+  const serviceDefinitions = await Promise.all(promiseOfServiceList);
   return { serviceDefinitions, isNewSchema }
 }

packages/apollo-server/src/index.ts

@@ -97,6 +97,8 @@ export class ApolloServer extends ApolloServerBase {
     // object, so we have to create it.
     const app = express();
 
+    app.disable('x-powered-by');
+
     // provide generous values for the getting started experience
     super.applyMiddleware({
       app,