Backport #2028 and #2029 (#2031)

* gateway: remove dependency on apollo-server-caching (#2029)

The point of apollo-server-caching is to provide an abstraction over
multiple cache backends. Gateway is not using that abstraction; it's
just using one particular implementation, which is a wrapper around an
old version of lru-cache.

As part of apollographql/apollo-server#6057
and apollographql/apollo-server#6719 we want
to remove dependencies on Apollo Server from Apollo Gateway. Technically
we don't really need to remove this dependency (since
apollo-server-caching doesn't depend on anything else in AS) but
apollo-server-caching won't be updated any more (in fact, even in AS3 it
has already been replaced by `@apollo/utils.keyvaluecache`), so let's do
it.

While we're at it, we make a few other improvements:
- Ever since #440, the queryPlanStore field is always set, so we can
  remove some conditionals around it.
- Instead of using the old lru-cache@6 wrapped by the
  apollo-server-caching package, we use the newer lru-cache@7 (which
  improves the algorithm internally and changes the names of methods a
  bit).
- The get and set methods on InMemoryLRUCache were only async because
  they implement the abstract KeyValueCache interface: the
  implementations didn't actually do anything async. So we no longer
  need to await them or include a giant comment about how we're not
  awaiting them.

* gateway RemoteGraphQLDataSource: throw GraphQLError, not ApolloError (#2028)

This is part of
apollographql/apollo-server#6057 (which is
itself part of
apollographql/apollo-server#6719). We are
trying to break the dependency of Gateway on Server so that (among other
things) it is easier to have a single version of Gateway that works with
both the current AS3 and the upcoming AS4.

In AS4, we are removing the ApolloError class and its subclasses.
Instead, we will just use GraphQLError directly. See:

https://www.apollographql.com/docs/apollo-server/v4/migration#apolloerror
https://www.apollographql.com/docs/apollo-server/v4/migration#built-in-error-classes
apollographql/apollo-server#6355
apollographql/apollo-server#6705

This commit changes RemoteGraphQLDataSource to throw GraphQLError
instead of ApolloError. The `code` extension will still be the same.
(The `name` field of the thrown Error will no longer be eg
`AuthenticationError`, though; this does not affect the error as
serialized in GraphQL.)

This is technically slightly backwards-incompatible (eg, the method
errorFromResponse is public and now returns GraphQLError instead of the
tighter ApolloError) but this doesn't seem likely to affect many users.
We can adjust based on feedback if necessary.

* Adjust #2028 for graphql@15.8 compatibility

gateway-js/CHANGELOG.md

@@ -5,7 +5,8 @@ This CHANGELOG pertains only to Apollo Federation packages in the `0.x` range. T
 ## vNEXT
 > 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)
 
-- Nothing yet! Stay tuned.
+- The method `RemoteGraphQLDataSource.errorFromResponse` now returns a `GraphQLError` (as defined by `graphql`) rather than an `ApolloError` (as defined by `apollo-server-errors`). [PR #2028](https://github.com/apollographql/federation/pull/2028)
+ - __BREAKING__: If you call `RemoteGraphQLDataSource.errorFromResponse` manually and expect its return value to be a particular subclass of `GraphQLError`, or if you expect the error received by `didEncounterError` to be a particular subclass of `GraphQLError`, then this change may affect you. We recommend checking `error.extensions.code` instead.
 
 ## v0.51.0
 

gateway-js/package.json

@@ -35,12 +35,11 @@
  "@opentelemetry/api": "^1.0.1",
  "@types/node-fetch": "2.6.2",
  "apollo-reporting-protobuf": "^0.8.0 || ^3.0.0",
- "apollo-server-caching": "^0.7.0 || ^3.0.0",
  "apollo-server-core": "^2.23.0 || ^3.0.0",
- "apollo-server-errors": "^2.5.0 || ^3.0.0",
  "apollo-server-types": "^0.9.0 || ^3.0.0",
  "async-retry": "^1.3.3",
  "loglevel": "^1.6.1",
+ "lru-cache": "^7.13.1",
  "make-fetch-happen": "^10.1.2",
  "pretty-format": "^28.0.0"
  },

gateway-js/src/datasources/RemoteGraphQLDataSource.ts

@@ -7,18 +7,14 @@ import {
  CacheScope,
  CachePolicy,
 } from 'apollo-server-types';
-import {
- ApolloError,
- AuthenticationError,
- ForbiddenError,
-} from 'apollo-server-errors';
 import { isObject } from '../utilities/predicates';
 import { GraphQLDataSource, GraphQLDataSourceProcessOptions, GraphQLDataSourceRequestKind } from './types';
 import { createHash } from '@apollo/utils.createhash';
 import { parseCacheControlHeader } from './parseCacheControlHeader';
 import fetcher from 'make-fetch-happen';
 import { Headers as NodeFetchHeaders, Request as NodeFetchRequest } from 'node-fetch';
 import { Fetcher, FetcherRequestInit, FetcherResponse } from '@apollo/utils.fetcher';
+import { GraphQLError, GraphQLErrorExtensions } from 'graphql';
 
 export class RemoteGraphQLDataSource<
  TContext extends Record<string, any> = Record<string, any>,
@@ -303,28 +299,36 @@ export class RemoteGraphQLDataSource<
  }
 
  public async errorFromResponse(response: FetcherResponse) {
- const message = `${response.status}: ${response.statusText}`;
-
- let error: ApolloError;
- if (response.status === 401) {
- error = new AuthenticationError(message);
- } else if (response.status === 403) {
- error = new ForbiddenError(message);
- } else {
- error = new ApolloError(message);
- }
-
  const body = await this.parseBody(response);
 
- Object.assign(error.extensions, {
+ const extensions: GraphQLErrorExtensions = {
  response: {
  url: response.url,
  status: response.status,
  statusText: response.statusText,
  body,
  },
- });
+ };
+
+ if (response.status === 401) {
+ extensions.code = 'UNAUTHENTICATED';
+ } else if (response.status === 403) {
+ extensions.code = 'FORBIDDEN';
+ }
 
- return error;
+ // Note: gateway 0.x still supports graphql-js v15.8, which does
+ // not have the options-based GraphQLError constructor. Note that
+ // the constructor used here is dropped in graphql-js v17, so this
+ // will have to be adjusted if we try to make gateway 0.x support
+ // graphql-js v17.
+ return new GraphQLError(
+ `${response.status}: ${response.statusText}`,
+ null,
+ null,
+ null,
+ null,
+ null,
+ extensions,
+ );
  }
 }

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

@@ -1,15 +1,10 @@
-import {
- ApolloError,
- AuthenticationError,
- ForbiddenError,
-} from 'apollo-server-errors';
-
 import { RemoteGraphQLDataSource } from '../RemoteGraphQLDataSource';
 import { Response, Headers } from 'node-fetch';
 import { GraphQLRequestContext } from 'apollo-server-types';
 import { GraphQLDataSourceRequestKind } from '../types';
 import { nockBeforeEach, nockAfterEach } from '../../__tests__/nockAssertions';
 import nock from 'nock';
+import { GraphQLError } from 'graphql';
 
 beforeEach(nockBeforeEach);
 afterEach(nockAfterEach);
@@ -461,15 +456,15 @@ describe('didEncounterError', () => {
  context,
  });
 
- await expect(result).rejects.toThrow(AuthenticationError);
+ await expect(result).rejects.toThrow(GraphQLError);
  expect(context).toMatchObject({
  timingData: [{ time: 1616446845234 }],
  });
  });
 });
 
 describe('error handling', () => {
- it('throws an AuthenticationError when the response status is 401', async () => {
+ it('throws error with code UNAUTHENTICATED when the response status is 401', async () => {
  const DataSource = new RemoteGraphQLDataSource({
  url: 'https://api.example.com/foo',
  });
@@ -480,7 +475,7 @@ describe('error handling', () => {
  ...defaultProcessOptions,
  request: { query: '{ me { name } }' },
  });
- await expect(result).rejects.toThrow(AuthenticationError);
+ await expect(result).rejects.toThrow(GraphQLError);
  await expect(result).rejects.toMatchObject({
  extensions: {
  code: 'UNAUTHENTICATED',
@@ -492,7 +487,7 @@ describe('error handling', () => {
  });
  });
 
- it('throws a ForbiddenError when the response status is 403', async () => {
+ it('throws an error with code FORBIDDEN when the response status is 403', async () => {
  const DataSource = new RemoteGraphQLDataSource({
  url: 'https://api.example.com/foo',
  });
@@ -503,7 +498,7 @@ describe('error handling', () => {
  ...defaultProcessOptions,
  request: { query: '{ me { name } }' },
  });
- await expect(result).rejects.toThrow(ForbiddenError);
+ await expect(result).rejects.toThrow(GraphQLError);
  await expect(result).rejects.toMatchObject({
  extensions: {
  code: 'FORBIDDEN',
@@ -515,7 +510,7 @@ describe('error handling', () => {
  });
  });
 
- it('throws an ApolloError when the response status is 500', async () => {
+ it('throws a GraphQLError when the response status is 500', async () => {
  const DataSource = new RemoteGraphQLDataSource({
  url: 'https://api.example.com/foo',
  });
@@ -526,7 +521,7 @@ describe('error handling', () => {
  ...defaultProcessOptions,
  request: { query: '{ me { name } }' },
  });
- await expect(result).rejects.toThrow(ApolloError);
+ await expect(result).rejects.toThrow(GraphQLError);
  await expect(result).rejects.toMatchObject({
  extensions: {
  response: {
@@ -560,7 +555,7 @@ describe('error handling', () => {
  ...defaultProcessOptions,
  request: { query: '{ me { name } }' },
  });
- await expect(result).rejects.toThrow(ApolloError);
+ await expect(result).rejects.toThrow(GraphQLError);
  await expect(result).rejects.toMatchObject({
  extensions: {
  response: {

gateway-js/src/index.ts

@@ -5,7 +5,7 @@ import {
  GraphQLRequestContextExecutionDidStart,
 } from 'apollo-server-types';
 import type { Logger } from '@apollo/utils.logger';
-import { InMemoryLRUCache } from 'apollo-server-caching';
+import LRUCache from 'lru-cache';
 import {
  isObjectType,
  isIntrospectionType,
@@ -128,7 +128,7 @@ export class ApolloGateway implements GraphQLService {
  private serviceMap: DataSourceMap = Object.create(null);
  private config: GatewayConfig;
  private logger: Logger;
- private queryPlanStore: InMemoryLRUCache<QueryPlan>;
+ private queryPlanStore: LRUCache<string, QueryPlan>;
  private apolloConfig?: ApolloConfigFromAS3;
  private onSchemaChangeListeners = new Set<(schema: GraphQLSchema) => void>();
  private onSchemaLoadOrUpdateListeners = new Set<
@@ -210,14 +210,14 @@ export class ApolloGateway implements GraphQLService {
  }
 
  private initQueryPlanStore(approximateQueryPlanStoreMiB?: number) {
- return new InMemoryLRUCache<QueryPlan>({
+ return new LRUCache<string, QueryPlan>({
  // Create ~about~ a 30MiB InMemoryLRUCache. This is less than precise
  // since the technique to calculate the size of a DocumentNode is
  // only using JSON.stringify on the DocumentNode (and thus doesn't account
  // for unicode characters, etc.), but it should do a reasonable job at
  // providing a caching document store for most operations.
  maxSize: Math.pow(2, 20) * (approximateQueryPlanStoreMiB || 30),
- sizeCalculator: approximateObjectSize,
+ sizeCalculation: approximateObjectSize,
  });
  }
 
@@ -607,7 +607,7 @@ export class ApolloGateway implements GraphQLService {
  // Once we remove the deprecated onSchemaChange() method, we can remove this.
  legacyDontNotifyOnSchemaChangeListeners: boolean = false,
  ): void {
- if (this.queryPlanStore) this.queryPlanStore.flush();
+ this.queryPlanStore.clear();
  this.schema = toAPISchema(coreSchema);
  this.queryPlanner = new QueryPlanner(coreSchema);
 
@@ -843,10 +843,7 @@ export class ApolloGateway implements GraphQLService {
  return { errors: validationErrors };
  }
 
- let queryPlan: QueryPlan | undefined;
- if (this.queryPlanStore) {
- queryPlan = await this.queryPlanStore.get(queryPlanStoreKey);
- }
+ let queryPlan = this.queryPlanStore.get(queryPlanStoreKey);
 
  if (!queryPlan) {
  queryPlan = tracer.startActiveSpan(
@@ -868,25 +865,11 @@ export class ApolloGateway implements GraphQLService {
  },
  );
 
- if (this.queryPlanStore) {
- // The underlying cache store behind the `documentStore` returns a
- // `Promise` which is resolved (or rejected), eventually, based on the
- // success or failure (respectively) of the cache save attempt. While
- // it's certainly possible to `await` this `Promise`, we don't care about
- // whether or not it's successful at this point. We'll instead proceed
- // to serve the rest of the request and just hope that this works out.
- // If it doesn't work, the next request will have another opportunity to
- // try again. Errors will surface as warnings, as appropriate.
- //
- // While it shouldn't normally be necessary to wrap this `Promise` in a
- // `Promise.resolve` invocation, it seems that the underlying cache store
- // is returning a non-native `Promise` (e.g. Bluebird, etc.).
- Promise.resolve(
- this.queryPlanStore.set(queryPlanStoreKey, queryPlan),
- ).catch((err) =>
- this.logger.warn(
- 'Could not store queryPlan' + ((err && err.message) || err),
- ),
+ try {
+ this.queryPlanStore.set(queryPlanStoreKey, queryPlan);
+ } catch (err) {
+ this.logger.warn(
+ 'Could not store queryPlan' + ((err && err.message) || err),
  );
  }
  }