Instantiate one CachedFetcher per gateway instance

packages/apollo-gateway/CHANGELOG.md

@@ -5,6 +5,7 @@
 > The changes noted within this `vNEXT` section have not been released yet.  New PRs and commits which introduce changes should include an entry in this `vNEXT` section as part of their development.  When a release is being prepared, a new header will be (manually) created below and the the appropriate changes within that release will be moved into the new section.
 
 * Reduce interface expansion for types contained to a single service [#3582](https://github.com/apollographql/apollo-server/pull/3582)
+* Instantiate one `CachedFetcher` per gateway instance.  This resolves a condition where multiple federated gateways would utilize the same cache store could result in an `Expected undefined to be a GraphQLSchema` error. [#3704](https://github.com/apollographql/apollo-server/pull/3704)
 
 # v0.11.6
 

packages/apollo-gateway/src/index.ts

@@ -40,6 +40,7 @@ import { GraphQLDataSource } from './datasources/types';
 import { RemoteGraphQLDataSource } from './datasources/RemoteGraphQLDataSource';
 import { HeadersInit } from 'node-fetch';
 import { getVariableValues } from 'graphql/execution/values';
+import { CachedFetcher } from './cachedFetcher';
 
 export type ServiceEndpointDefinition = Pick<ServiceDefinition, 'name' | 'url'>;
 
@@ -158,6 +159,7 @@ export class ApolloGateway implements GraphQLService {
   private serviceDefinitions: ServiceDefinition[] = [];
   private compositionMetadata?: CompositionMetadata;
   private serviceSdlCache = new Map<string, string>();
+  private fetcher = new CachedFetcher();
 
   // Observe query plan, service info, and operation info prior to execution.
   // The information made available here will give insight into the resulting
@@ -467,6 +469,7 @@ export class ApolloGateway implements GraphQLService {
       apiKeyHash: this.engineConfig.apiKeyHash,
       graphVariant: this.engineConfig.graphVariant,
       federationVersion: config.federationVersion || 1,
+      fetcher: this.fetcher
     });
   }
 

packages/apollo-gateway/src/loadServicesFromStorage.ts

@@ -1,5 +1,4 @@
 import { CachedFetcher } from './cachedFetcher';
-import { ServiceDefinition } from '@apollo/federation';
 import { parse } from 'graphql';
 import { Experimental_UpdateServiceDefinitions } from '.';
 
@@ -47,33 +46,27 @@ const urlStorageSecretBase: string = urlFromEnvOrDefault(
   'https://storage.googleapis.com/engine-partial-schema-prod/',
 );
 
-const fetcher = new CachedFetcher();
-
 function getStorageSecretUrl(graphId: string, apiKeyHash: string): string {
   return `${urlStorageSecretBase}/${graphId}/storage-secret/${apiKeyHash}.json`;
 }
 
-async function fetchStorageSecret(
-  graphId: string,
-  apiKeyHash: string,
-): Promise<string> {
-  const storageSecretUrl = getStorageSecretUrl(graphId, apiKeyHash);
-  const response = await fetcher.fetch(storageSecretUrl);
-  return JSON.parse(response.result);
-}
-
 export async function getServiceDefinitionsFromStorage({
   graphId,
   apiKeyHash,
   graphVariant,
   federationVersion,
+  fetcher,
 }: {
   graphId: string;
   apiKeyHash: string;
   graphVariant?: string;
   federationVersion: number;
+  fetcher: CachedFetcher;
 }): ReturnType<Experimental_UpdateServiceDefinitions> {
-  const secret = await fetchStorageSecret(graphId, apiKeyHash);
+  // fetch the storage secret
+  const storageSecretUrl = getStorageSecretUrl(graphId, apiKeyHash);
+  const response = await fetcher.fetch(storageSecretUrl);
+  const secret = JSON.parse(response.result);
 
   if (!graphVariant) {
     graphVariant = 'current';
@@ -84,7 +77,7 @@ export async function getServiceDefinitionsFromStorage({
   const {
     isCacheHit: linkFileCacheHit,
     result: linkFileResult,
-  } = await fetchLinkFile(baseUrl);
+  } = await fetcher.fetch(`${baseUrl}/composition-config-link`);
 
   // If the link file is a cache hit, no further work is needed
   if (linkFileCacheHit) return { isNewSchema: false };
@@ -99,8 +92,25 @@ export async function getServiceDefinitionsFromStorage({
     configFileResult,
   ) as CompositionMetadata;
 
-  const serviceDefinitions = await fetchServiceDefinitions(
-    compositionMetadata.implementingServiceLocations,
+  // It's important to maintain the original order here
+  const serviceDefinitions = await Promise.all(
+    compositionMetadata.implementingServiceLocations.map(
+      async ({ name, path }) => {
+        const serviceLocation = await fetcher.fetch(
+          `${urlPartialSchemaBase}/${path}`,
+        );
+
+        const { url, partialSchemaPath } = JSON.parse(
+          serviceLocation.result,
+        ) as ImplementingService;
+
+        const { result } = await fetcher.fetch(
+          `${urlPartialSchemaBase}/${partialSchemaPath}`,
+        );
+
+        return { name, url, typeDefs: parse(result) };
+      },
+    ),
   );
 
   // explicity return that this is a new schema, as the link file has changed.
@@ -113,33 +123,3 @@ export async function getServiceDefinitionsFromStorage({
     isNewSchema: true,
   };
 }
-
-async function fetchLinkFile(baseUrl: string) {
-  return fetcher.fetch(`${baseUrl}/composition-config-link`);
-}
-
-// The order of implementingServices is IMPORTANT
-async function fetchServiceDefinitions(
-  implementingServices: ImplementingServiceLocation[],
-): Promise<ServiceDefinition[]> {
-  const serviceDefinitionPromises = implementingServices.map(
-    async ({ name, path }) => {
-      const serviceLocation = await fetcher.fetch(
-        `${urlPartialSchemaBase}/${path}`,
-      );
-
-      const { url, partialSchemaPath } = JSON.parse(
-        serviceLocation.result,
-      ) as ImplementingService;
-
-      const { result } = await fetcher.fetch(
-        `${urlPartialSchemaBase}/${partialSchemaPath}`,
-      );
-
-      return { name, url, typeDefs: parse(result) };
-    },
-  );
-
-  // Respect the order here
-  return Promise.all(serviceDefinitionPromises);
-}