feat(gateway): Provide context to didEncounterError for RGDS (#600)

This commit introduces a fourth (optional) argument context to the
RemoteGraphQLDataSource's didEncounterError hook. This is a fairly small,
non-breaking change to the API and it seems fairly reasonable to want
access to context from within this hook.

gateway-js/CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Enable gateway to fetch CSDL / cloud config in managed mode. This feature is currently opt-in only and should have no effect for existing use cases. After some testing, this behavior will become the default (and should be nearly transparent / non-breaking for users when we do so). It's unintended for users to start using this feature for now unless instructed by Apollo to do so. [PR #458](https://github.com/apollographql/federation/pull/458)
 - __FIX__: followup to #458. Fix typings of `getDefaultFetcher` - `make-fetch-happen` types were not being included in the gateway's compiled `index.d.ts` file. [PR #585](https://github.com/apollographql/federation/pull/585)
+- Provide `context` as a fourth, optional argument to `RemoteGraphQLDataSource.didEncounterError`. This is a non-breaking change which allows implementors to read and modify the `context` object which is already similarly available in other hooks. [PR #600](https://github.com/apollographql/federation/pull/600)
 
 ## v0.24.4
 

gateway-js/src/datasources/RemoteGraphQLDataSource.ts

@@ -170,7 +170,7 @@ export class RemoteGraphQLDataSource<TContext extends Record<string, any> = Reco
         http: fetchResponse,
       };
     } catch (error) {
-      this.didEncounterError(error, fetchRequest, fetchResponse);
+      this.didEncounterError(error, fetchRequest, fetchResponse, context);
       throw error;
     }
   }
@@ -192,7 +192,8 @@ export class RemoteGraphQLDataSource<TContext extends Record<string, any> = Reco
   public didEncounterError(
     error: Error,
     _fetchRequest: Request,
-    _fetchResponse?: Response
+    _fetchResponse?: Response,
+    _context?: TContext,
   ) {
     throw error;
   }

gateway-js/src/datasources/__tests__/RemoteGraphQLDataSource.test.ts

@@ -1,5 +1,5 @@
 import { fetch } from '../../__mocks__/apollo-server-env';
-import { makeFetchHappenFetcher} from '../../__mocks__/make-fetch-happen-fetcher';
+import { makeFetchHappenFetcher } from '../../__mocks__/make-fetch-happen-fetcher';
 
 import {
   ApolloError,
@@ -33,7 +33,7 @@ describe('constructing requests', () => {
       expect(data).toEqual({ me: 'james' });
       expect(fetch).toBeCalledTimes(1);
       expect(fetch).toHaveFetched('https://api.example.com/foo', {
-        body: { query: '{ me { name } }' }
+        body: { query: '{ me { name } }' },
       });
     });
 
@@ -67,21 +67,23 @@ describe('constructing requests', () => {
 
     // This is a SHA-256 hash of `query` above.
     const sha256Hash =
-      "b8d9506e34c83b0e53c2aa463624fcea354713bc38f95276e6f0bd893ffb5b88";
+      'b8d9506e34c83b0e53c2aa463624fcea354713bc38f95276e6f0bd893ffb5b88';
 
     describe('miss', () => {
       const apqNotFoundResponse = {
-        "errors": [
+        errors: [
           {
-            "message": "PersistedQueryNotFound",
-            "extensions": {
-              "code": "PERSISTED_QUERY_NOT_FOUND",
-              "exception": {
-                "stacktrace": ["PersistedQueryNotFoundError: PersistedQueryNotFound"]
-              }
-            }
-          }
-        ]
+            message: 'PersistedQueryNotFound',
+            extensions: {
+              code: 'PERSISTED_QUERY_NOT_FOUND',
+              exception: {
+                stacktrace: [
+                  'PersistedQueryNotFoundError: PersistedQueryNotFound',
+                ],
+              },
+            },
+          },
+        ],
       };
 
       it('stringifies a request with a query', async () => {
@@ -106,8 +108,8 @@ describe('constructing requests', () => {
               persistedQuery: {
                 version: 1,
                 sha256Hash,
-              }
-            }
+              },
+            },
           },
         });
         expect(fetch).toHaveFetchedNth(2, 'https://api.example.com/foo', {
@@ -117,8 +119,8 @@ describe('constructing requests', () => {
               persistedQuery: {
                 version: 1,
                 sha256Hash,
-              }
-            }
+              },
+            },
           },
         });
       });
@@ -149,8 +151,8 @@ describe('constructing requests', () => {
               persistedQuery: {
                 version: 1,
                 sha256Hash,
-              }
-            }
+              },
+            },
           },
         });
         expect(fetch).toHaveFetchedNth(2, 'https://api.example.com/foo', {
@@ -161,8 +163,8 @@ describe('constructing requests', () => {
               persistedQuery: {
                 version: 1,
                 sha256Hash,
-              }
-            }
+              },
+            },
           },
         });
       });
@@ -185,13 +187,13 @@ describe('constructing requests', () => {
         expect(data).toEqual({ me: 'james' });
         expect(fetch).toBeCalledTimes(1);
         expect(fetch).toHaveFetched('https://api.example.com/foo', {
-            body: {
+          body: {
             extensions: {
               persistedQuery: {
                 version: 1,
                 sha256Hash,
-              }
-            }
+              },
+            },
           },
         });
       });
@@ -221,8 +223,8 @@ describe('constructing requests', () => {
               persistedQuery: {
                 version: 1,
                 sha256Hash,
-              }
-            }
+              },
+            },
           },
         });
       });
@@ -232,7 +234,9 @@ describe('constructing requests', () => {
 
 describe('fetcher', () => {
   it('uses a custom provided `fetcher`', async () => {
-    const injectedFetch = fetch.mockJSONResponseOnce({ data: { injected: true } });
+    const injectedFetch = fetch.mockJSONResponseOnce({
+      data: { injected: true },
+    });
     const DataSource = new RemoteGraphQLDataSource({
       url: 'https://api.example.com/foo',
       fetcher: injectedFetch,
@@ -247,13 +251,13 @@ describe('fetcher', () => {
     });
 
     expect(injectedFetch).toHaveBeenCalled();
-    expect(data).toEqual({injected: true});
-
+    expect(data).toEqual({ injected: true });
   });
 
   it('supports a custom fetcher, like `make-fetch-happen`', async () => {
-    const injectedFetch =
-      makeFetchHappenFetcher.mockJSONResponseOnce({ data: { me: 'james' } });
+    const injectedFetch = makeFetchHappenFetcher.mockJSONResponseOnce({
+      data: { me: 'james' },
+    });
     const DataSource = new RemoteGraphQLDataSource({
       url: 'https://api.example.com/foo',
       fetcher: injectedFetch,
@@ -269,7 +273,7 @@ describe('fetcher', () => {
 
     expect(injectedFetch).toHaveBeenCalled();
     expect(data).toEqual({ me: 'james' });
-  })
+  });
 });
 
 describe('willSendRequest', () => {
@@ -343,10 +347,12 @@ describe('didReceiveResponse', () => {
       didReceiveResponse<MyContext>({
         request,
         response,
-      }: Required<Pick<
-        GraphQLRequestContext<MyContext>,
+      }: Required<
+        Pick<
+          GraphQLRequestContext<MyContext>,
           'request' | 'response' | 'context'
-      >>) {
+        >
+      >) {
         const surrogateKeys =
           request.http && request.http.headers.get('surrogate-keys');
         if (surrogateKeys) {
@@ -383,17 +389,18 @@ describe('didReceiveResponse', () => {
 
       didReceiveResponse<MyContext>({
         response,
-      }: Required<Pick<
-        GraphQLRequestContext<MyContext>,
+      }: Required<
+        Pick<
+          GraphQLRequestContext<MyContext>,
           'request' | 'response' | 'context'
-      >>) {
+        >
+      >) {
         return response;
       }
     }
 
     const DataSource = new MyDataSource();
-    const spyDidReceiveResponse =
-      jest.spyOn(DataSource, 'didReceiveResponse');
+    const spyDidReceiveResponse = jest.spyOn(DataSource, 'didReceiveResponse');
 
     fetch.mockJSONResponseOnce({ data: { me: 'james' } });
 
@@ -406,7 +413,6 @@ describe('didReceiveResponse', () => {
     });
 
     expect(spyDidReceiveResponse).toHaveBeenCalledTimes(1);
-
   });
 
   // APQ makes two requests, so make sure only one calls the response hook.
@@ -417,10 +423,12 @@ describe('didReceiveResponse', () => {
 
       didReceiveResponse<MyContext>({
         response,
-      }: Required<Pick<
-        GraphQLRequestContext<MyContext>,
+      }: Required<
+        Pick<
+          GraphQLRequestContext<MyContext>,
           'request' | 'response' | 'context'
-      >>) {
+        >
+      >) {
         return response;
       }
     }
@@ -439,7 +447,45 @@ describe('didReceiveResponse', () => {
     });
 
     expect(spyDidReceiveResponse).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe('didEncounterError', () => {
+  it('can accept and modify context', async () => {
+    interface MyContext {
+      timingData: { time: number }[];
+    }
 
+    class MyDataSource extends RemoteGraphQLDataSource<MyContext> {
+      url = 'https://api.example.com/foo';
+
+      didEncounterError(
+        _error: Error,
+        _fetchRequest: Request,
+        _fetchResponse?: Response,
+        _context?: MyContext,
+      ) {
+        // a timestamp a la `Date.now()`
+        context.timingData.push({ time: 1616446845234 });
+      }
+    }
+
+    const DataSource = new MyDataSource();
+
+    fetch.mockResponseOnce('Invalid token', undefined, 401);
+
+    const context: MyContext = { timingData: [] };
+    const result = DataSource.process({
+      request: {
+        query: '{ me { name } }',
+      },
+      context,
+    });
+
+    await expect(result).rejects.toThrow(AuthenticationError);
+    expect(context).toMatchObject({
+      timingData: [{ time: 1616446845234 }]
+    });
   });
 });