fix(gateway): Resolve gateway retry issues (UplinkFetcher failed to update...) (version-0.x)

docs/source/api/apollo-gateway.mdx

@@ -267,7 +267,7 @@ Provide this field **only** if you are using managed federation _and_ your use c
 
 The maximum number of times the gateway retries a failed poll request to Apollo Uplink, cycling through its list of [`uplinkEndpoints`](#uplinkendpoints).
 
-The default value is three times your number of `uplinkEndpoints` (i.e., `6` in almost all cases).
+The default value is to try each of your uplinkEndpoints three times (i.e., 5 retries with the default list of two endpoints).
 
 Provide this field **only** if you are using managed federation.
 

gateway-js/CHANGELOG.md

@@ -7,6 +7,7 @@ This CHANGELOG pertains only to Apollo Federation packages in the `0.x` range. T
 > 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 appropriate changes within that release will be moved into the new section.
 
 - Use specific error classes when throwing errors due Apollo Uplink being unreacheable or returning an invalid response [PR #1473](https://github.com/apollographql/federation/pull/1473)
+- __FIX__ Correct retry logic while fetching the supergraph schema from Uplink [PR #1503](https://github.com/apollographql/federation/pull/1503)
 
 ## v0.48.0
 

gateway-js/package.json

@@ -38,6 +38,7 @@
     "apollo-server-errors": "^2.5.0 || ^3.0.0",
     "apollo-server-types": "^0.9.0 || ^3.0.0",
     "apollo-utilities": "^1.3.0",
+    "async-retry": "^1.3.3",
     "loglevel": "^1.6.1",
     "make-fetch-happen": "^8.0.0",
     "pretty-format": "^27.4.6",

gateway-js/src/__tests__/integration/nockMocks.ts

@@ -121,9 +121,10 @@ export function mockSupergraphSdlRequestSuccessIfAfter(
 }
 
 export function mockSupergraphSdlRequestIfAfterUnchanged(
-    ifAfter: string | null = null,
+  ifAfter: string | null = null,
+  url: string = mockCloudConfigUrl1,
 ) {
-  return mockSupergraphSdlRequestIfAfter(ifAfter).reply(
+  return mockSupergraphSdlRequestIfAfter(ifAfter, url).reply(
       200,
       JSON.stringify({
         data: {

gateway-js/src/index.ts

@@ -400,7 +400,7 @@ export class ApolloGateway implements GraphQLService {
           apiKey: this.apolloConfig!.key!,
           uplinkEndpoints,
           maxRetries:
-            this.config.uplinkMaxRetries ?? uplinkEndpoints.length * 3,
+            this.config.uplinkMaxRetries ?? uplinkEndpoints.length * 3 - 1, // -1 for the initial request
           subgraphHealthCheck: this.config.serviceHealthCheck,
           fetcher: this.fetcher,
           logger: this.logger,

gateway-js/src/supergraphManagers/LegacyFetcher/index.ts

@@ -222,7 +222,7 @@ export class LegacyFetcher implements SupergraphManager {
 
   private logUpdateFailure(e: any) {
     this.config.logger?.error(
-      'UplinkFetcher failed to update supergraph with the following error: ' +
+      'LegacyFetcher failed to update supergraph with the following error: ' +
         (e.message ?? e),
     );
   }

gateway-js/src/supergraphManagers/LocalCompose/index.ts

@@ -76,7 +76,7 @@ export class LocalCompose implements SupergraphManager {
 
   private logUpdateFailure(e: any) {
     this.config.logger?.error(
-      'UplinkFetcher failed to update supergraph with the following error: ' +
+      'LocalCompose failed to update supergraph with the following error: ' +
         (e.message ?? e),
     );
   }

gateway-js/src/supergraphManagers/UplinkFetcher/__tests__/loadSupergraphSdlFromStorage.test.ts

@@ -63,7 +63,8 @@ describe('loadSupergraphSdlFromStorage', () => {
       errorReportingEndpoint: undefined,
       fetcher,
       compositionId: "originalId-1234",
-      maxRetries: 1
+      maxRetries: 1,
+      roundRobinSeed: 0,
     });
 
     expect(result).toMatchObject({
@@ -85,7 +86,8 @@ describe('loadSupergraphSdlFromStorage', () => {
         errorReportingEndpoint: undefined,
         fetcher,
         compositionId: "originalId-1234",
-        maxRetries: 1
+        maxRetries: 1,
+        roundRobinSeed: 0,
       }),
     ).rejects.toThrowError(
       new UplinkFetcherError(
@@ -368,3 +370,28 @@ describe('loadSupergraphSdlFromStorage', () => {
   });
 });
 
+
+describe("loadSupergraphSdlFromUplinks", () => {
+  beforeEach(nockBeforeEach);
+  afterEach(nockAfterEach);
+
+  it("doesn't retry in the unchanged / null case", async () => {
+    mockSupergraphSdlRequestIfAfterUnchanged("id-1234", mockCloudConfigUrl1);
+
+    const fetcher = jest.fn(getDefaultFetcher());
+    const result = await loadSupergraphSdlFromUplinks({
+      graphRef,
+      apiKey,
+      endpoints: [mockCloudConfigUrl1, mockCloudConfigUrl2],
+      errorReportingEndpoint: mockOutOfBandReporterUrl,
+      fetcher: fetcher as any,
+      compositionId: "id-1234",
+      maxRetries: 5,
+      roundRobinSeed: 0,
+    });
+
+    expect(result).toBeNull();
+    expect(fetcher).toHaveBeenCalledTimes(1);
+  });
+});
+

gateway-js/src/supergraphManagers/UplinkFetcher/index.ts

@@ -30,6 +30,7 @@ export class UplinkFetcher implements SupergraphManager {
   private errorReportingEndpoint: string | undefined =
     process.env.APOLLO_OUT_OF_BAND_REPORTER_ENDPOINT ?? undefined;
   private compositionId?: string;
+  private fetchCount: number = 0;
 
   constructor(options: UplinkFetcherOptions) {
     this.config = options;
@@ -81,6 +82,7 @@ export class UplinkFetcher implements SupergraphManager {
       fetcher: this.config.fetcher,
       compositionId: this.compositionId ?? null,
       maxRetries: this.config.maxRetries,
+      roundRobinSeed: this.fetchCount++,
     });
 
     if (!result) {

gateway-js/src/supergraphManagers/UplinkFetcher/loadSupergraphSdlFromStorage.ts

@@ -1,5 +1,6 @@
 import { fetch, Response, Request } from 'apollo-server-env';
 import { GraphQLError } from 'graphql';
+import retry from 'async-retry';
 import { SupergraphSdlUpdate } from '../../config';
 import { submitOutOfBandReportIfConfigured } from './outOfBandReporter';
 import { SupergraphSdlQuery } from '../../__generated__/graphqlTypes';
@@ -39,8 +40,6 @@ const { name, version } = require('../../../package.json');
 
 const fetchErrorMsg = "An error occurred while fetching your schema from Apollo: ";
 
-let fetchCounter = 0;
-
 export class UplinkFetcherError extends Error {
   constructor(message: string) {
     super(message);
@@ -56,36 +55,35 @@ export async function loadSupergraphSdlFromUplinks({
   fetcher,
   compositionId,
   maxRetries,
+  roundRobinSeed,
 }: {
   graphRef: string;
   apiKey: string;
   endpoints: string[];
   errorReportingEndpoint: string | undefined,
   fetcher: typeof fetch;
   compositionId: string | null;
-  maxRetries: number
+  maxRetries: number,
+  roundRobinSeed: number,
 }) : Promise<SupergraphSdlUpdate | null> {
-  let retries = 0;
-  let lastException = null;
-  let result: SupergraphSdlUpdate | null = null;
-  while (retries++ <= maxRetries && result == null) {
-    try {
-      result = await loadSupergraphSdlFromStorage({
+  // This Promise resolves with either an updated supergraph or null if no change.
+  // This Promise can reject in the case that none of the retries are successful,
+  // in which case it will reject with the most frequently encountered error.
+  return retry(
+    () =>
+      loadSupergraphSdlFromStorage({
         graphRef,
         apiKey,
-        endpoint: endpoints[fetchCounter++ % endpoints.length],
+        endpoint: endpoints[roundRobinSeed++ % endpoints.length],
         errorReportingEndpoint,
         fetcher,
-        compositionId
-      });
-    } catch (e) {
-      lastException = e;
-    }
-  }
-  if (result === null && lastException !== null) {
-    throw lastException;
-  }
-  return result;
+        compositionId,
+      }),
+    {
+      retries: maxRetries,
+    },
+  );
+
 }
 
 export async function loadSupergraphSdlFromStorage({

package.json

@@ -49,6 +49,7 @@
     "@rollup/plugin-commonjs": "21.0.1",
     "@rollup/plugin-json": "4.1.0",
     "@rollup/plugin-node-resolve": "13.1.3",
+    "@types/async-retry": "^1.4.3",
     "@types/bunyan": "1.8.8",
     "@types/deep-equal": "1.0.1",
     "@types/jest": "27.4.0",