chore(op-reg): Switch from node-fetch to make-fetch-happen.

packages/apollo-server-plugin-operation-registry/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Change Log
 
+### vNEXT
+
+- Switch from using our own HTTP _retries_ and _conditional-request_ implementation built on top of [`node-fetch`](https://npm.im/node-fetch) to a third-party "Fetcher" (i.e., a [Fetch API](https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API)-compliant implementation) called [`make-fetch-happen`](https://npm.im/make-fetch-happen). [PR #TODO]()
+
 ### 0.4.1
 
 - __BREAKING__: Use a content delivery network, fetch storage secrets and operation manifests from different domains: https://storage-secrets.api.apollographql.com and https://operations.api.apollographql.com. Please mind any firewall for outgoing traffic.

packages/apollo-server-plugin-operation-registry/package.json

@@ -18,12 +18,13 @@
   "dependencies": {
     "apollo-graphql": "0.4.5",
     "apollo-server-caching": "file:../apollo-server-caching",
+    "apollo-server-env": "file:../apollo-server-env",
     "apollo-server-errors": "file:../apollo-server-errors",
     "apollo-server-plugin-base": "file:../apollo-server-plugin-base",
     "fast-json-stable-stringify": "^2.0.0",
     "loglevel": "^1.6.1",
     "loglevel-debug": "^0.0.1",
-    "node-fetch": "^2.3.0"
+    "make-fetch-happen": "^8.0.7"
   },
   "peerDependencies": {
     "graphql": "^0.12.0 || ^0.13.0 || ^14.0.0"

packages/apollo-server-plugin-operation-registry/src/ApolloServerPluginOperationRegistry.ts

@@ -23,6 +23,7 @@ import { GraphQLSchema } from 'graphql/type';
 import { InMemoryLRUCache } from 'apollo-server-caching';
 import loglevel from 'loglevel';
 import loglevelDebug from 'loglevel-debug';
+import { fetch } from "apollo-server-env";
 
 type ForbidUnregisteredOperationsPredicate = (
   requestContext: GraphQLRequestContext,
@@ -45,6 +46,7 @@ export interface OperationManifest {
 
 export interface Options {
   debug?: boolean;
+  fetcher?: typeof fetch;
   forbidUnregisteredOperations?:
     | boolean
     | ForbidUnregisteredOperationsPredicate;
@@ -136,6 +138,7 @@ for observability purposes, but all operations will be permitted.`,
         engine,
         store,
         logger,
+        fetcher: options.fetcher,
       });
 
       await agent.start();

packages/apollo-server-plugin-operation-registry/src/__tests__/agent.test.ts

@@ -283,60 +283,6 @@ describe('Agent', () => {
         expect(storeDeleteSpy).toBeCalledTimes(0);
       });
 
-      it('continues polling even after initial failure', async () => {
-        nockStorageSecret(genericServiceID, genericApiKeyHash);
-        nockStorageSecretOperationManifest(
-          genericServiceID,
-          genericStorageSecret,
-          500,
-        );
-        const store = defaultStore();
-        const storeSetSpy = jest.spyOn(store, 'set');
-        const storeDeleteSpy = jest.spyOn(store, 'delete');
-        const agent = createAgent({ store });
-        jest.useFakeTimers();
-        await agent.start();
-
-        expect(storeSetSpy).toBeCalledTimes(0);
-        expect(storeDeleteSpy).toBeCalledTimes(0);
-
-        // Only the initial start-up check should have happened by now.
-        expect(agent._timesChecked).toBe(1);
-
-        // If it's one millisecond short of our next poll interval, nothing
-        // should have changed yet.
-        jest.advanceTimersByTime(defaultTestAgentPollSeconds * 1000 - 1);
-
-        // Still only one check.
-        expect(agent._timesChecked).toBe(1);
-        expect(storeSetSpy).toBeCalledTimes(0);
-
-        // Now, we'll expect another GOOD request to fulfill, so we'll nock it.
-        nockStorageSecret(genericServiceID, genericApiKeyHash);
-        nockGoodManifestsUnderStorageSecret(
-          genericServiceID,
-          genericStorageSecret,
-          [
-            sampleManifestRecords.a,
-            sampleManifestRecords.b,
-            sampleManifestRecords.c,
-          ],
-        );
-
-        // If we move forward the last remaining millisecond, we should trigger
-        // and end up with a successful sync.
-        jest.advanceTimersByTime(1);
-
-        // While that timer will fire, it will do work async, and we need to
-        // wait on that work itself.
-        await agent.requestPending();
-
-        // Now the times checked should have gone up.
-        expect(agent._timesChecked).toBe(2);
-        // And store should have been called with operations ABC
-        expect(storeSetSpy).toBeCalledTimes(3);
-      });
-
       it('purges operations which are removed from the manifest', async () => {
         const store = defaultStore();
         const storeSetSpy = jest.spyOn(store, 'set');

packages/apollo-server-plugin-operation-registry/src/agent.ts

@@ -8,17 +8,19 @@ import {
 } from './common';
 
 import loglevel from 'loglevel';
+import fetcher from 'make-fetch-happen';
+import { HttpRequestCache } from './cache';
 
-import { Response } from 'node-fetch';
 import { InMemoryLRUCache } from 'apollo-server-caching';
-import { fetchIfNoneMatch } from './fetchIfNoneMatch';
 import { OperationManifest } from "./ApolloServerPluginOperationRegistry";
+import { Response, RequestInit, fetch } from "apollo-server-env";
 
 const DEFAULT_POLL_SECONDS: number = 30;
 const SYNC_WARN_TIME_SECONDS: number = 60;
 
 export interface AgentOptions {
   logger?: loglevel.Logger;
+  fetcher?: typeof fetch;
   pollSeconds?: number;
   schemaHash: string;
   engine: any;
@@ -31,6 +33,7 @@ type SignatureStore = Set<string>;
 const callToAction = `Ensure this server's schema has been published with 'apollo service:push' and that operations have been registered with 'apollo client:push'.`;
 
 export default class Agent {
+  private fetcher: typeof fetch;
   private timer?: NodeJS.Timer;
   private logger: loglevel.Logger;
   private hashedServiceId?: string;
@@ -49,6 +52,8 @@ export default class Agent {
 
     this.logger = this.options.logger || loglevel.getLogger(pluginName);
 
+    this.fetcher = this.options.fetcher || getDefaultGcsFetcher();
+
     if (!this.options.schemaHash) {
       throw new Error('`schemaHash` must be passed to the Agent.');
     }
@@ -145,11 +150,7 @@ export default class Agent {
       this.options.engine.apiKeyHash,
     );
 
-    const response = await fetchIfNoneMatch(storageSecretUrl, {
-      method: 'GET',
-      // More than three times our polling interval should be long enough to wait.
-      timeout: this.pollSeconds() * 3 /* times */ * 1000 /* ms */,
-    });
+    const response = await this.fetcher(storageSecretUrl, this.fetchOptions);
 
     if (response.status === 304) {
       this.logger.debug(
@@ -169,12 +170,7 @@ export default class Agent {
     return this.storageSecret;
   }
 
-  private fetchOptions = {
-    // GET is what we request, but keep in mind that, when we include and get
-    // a match on the `If-None-Match` header we'll get an early return with a
-    // status code 304.
-    method: 'GET',
-
+  private fetchOptions: RequestInit = {
     // More than three times our polling interval should be long enough to wait.
     timeout: this.pollSeconds() * 3 /* times */ * 1000 /* ms */,
   };
@@ -191,7 +187,7 @@ export default class Agent {
       this.options.schemaHash,
     );
     this.logger.debug(`Checking for manifest changes at ${legacyManifestUrl}`);
-    return fetchIfNoneMatch(legacyManifestUrl, this.fetchOptions);
+    return this.fetcher(legacyManifestUrl, this.fetchOptions);
   }
 
   private async fetchManifest(): Promise<Response> {
@@ -212,10 +208,8 @@ export default class Agent {
     this.logger.debug(
       `Checking for manifest changes at ${storageSecretManifestUrl}`,
     );
-    const response = await fetchIfNoneMatch(
-      storageSecretManifestUrl,
-      this.fetchOptions,
-    );
+    const response =
+      await this.fetcher(storageSecretManifestUrl, this.fetchOptions);
 
     if (response.status === 404 || response.status === 403) {
       this.logger.warn(
@@ -340,3 +334,28 @@ export default class Agent {
     this.lastOperationSignatures = replacementSignatures;
   }
 }
+
+const GCS_RETRY_COUNT = 5;
+
+function getDefaultGcsFetcher() {
+  return fetcher.defaults({
+    cacheManager: new HttpRequestCache(),
+    // All headers should be lower-cased here, as `make-fetch-happen`
+    // treats differently cased headers as unique (unlike the `Headers` object).
+    // @see: https://git.io/JvRUa
+    headers: {
+      'user-agent': [
+        require('../package.json').name,
+        require('../package.json').version
+      ].join('/'),
+    },
+    retry: {
+      retries: GCS_RETRY_COUNT,
+      // The default factor: expected attempts at 0, 1, 3, 7, 15, and 31 seconds elapsed
+      factor: 2,
+      // 1 second
+      minTimeout: 1000,
+      randomize: true,
+    },
+  });
+}

packages/apollo-server-plugin-operation-registry/src/cache.ts

@@ -0,0 +1,66 @@
+import { CacheManager } from 'make-fetch-happen';
+import { Request, Response, Headers } from 'apollo-server-env';
+import { InMemoryLRUCache } from 'apollo-server-caching';
+
+const MAX_SIZE = 5 * 1024 * 1024; // 5MB
+
+function cacheKey(request: Request) {
+  return `op-reg:request-cache:${request.method}:${request.url}`;
+}
+
+interface CachedRequest {
+    body: string;
+    status: number;
+    statusText: string;
+    headers: Headers;
+}
+
+export class HttpRequestCache implements CacheManager {
+  constructor(
+    public cache: InMemoryLRUCache<CachedRequest> = new InMemoryLRUCache({
+      maxSize: MAX_SIZE,
+    }),
+  ) {}
+
+  // Return true if entry exists, else false
+  async delete(request: Request) {
+    const key = cacheKey(request);
+    const entry = await this.cache.get(key);
+    await this.cache.delete(key);
+    return Boolean(entry);
+  }
+
+  async put(request: Request, response: Response) {
+    // A `HEAD` request has no body to cache and a 304 response could have
+    // only been negotiated by using a cached body that was still valid.
+    // Therefore, we do NOT write to the cache in either of these cases.
+    // Without avoiding this, we will invalidate the cache, thus causing
+    // subsequent conditional requests (e.g., `If-None-Match: "MD%") to be
+    // lacking content to conditionally request against and necessitating
+    // a full request/response.
+    if (request.method === "HEAD" || response.status === 304) {
+      return response;
+    }
+
+    const body = await response.text();
+
+    this.cache.set(cacheKey(request), {
+      body,
+      status: response.status,
+      statusText: response.statusText,
+      headers: response.headers,
+    });
+
+    return new Response(body, response);
+  }
+
+  async match(request: Request) {
+    return this.cache.get(cacheKey(request)).then(response => {
+      if (response) {
+        const { body, ...requestInit } = response;
+        return new Response(body, requestInit);
+      }
+      return;
+    });
+  }
+}

packages/apollo-gateway/src/make-fetch-happen.d.ts → types/make-fetch-happen/index.d.ts

@@ -1,3 +1,9 @@
+/**
+ * We are attempting to get types included natively in this package, but it
+ * has not happened, yet!
+ *
+ * See https://github.com/npm/make-fetch-happen/issues/20
+ */
 declare module 'make-fetch-happen' {
   import {
     Response,