Add no-cache headers to PersistedQuery error responses (#2452)

* add no-cache headers to PersistedQuery error responses

* fix defaultHeaders typo

* remove try/catch on hasPersistedQueryError replace with Array.isArray check

* Add CHANGELOG.md for #2446.

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ### v2.5.0
 
+- Add cache-control: no-cache header to both PersistedQueryNotSupportedError and PersistedQueryNotFoundError responses as these should never be cached. [PR #2452](https://github.com/apollographql/apollo-server/pull/2452)
+- `apollo-datasource-rest`: Don't attempt to parse "204 No Content" responses as JSON. [PR #2446](https://github.com/apollographql/apollo-server/pull/2446)
 - `apollo-server-express`: Fix Playground URL when Apollo Server is mounted inside of another Express app by utilizing `req.originalUrl`. [PR #2451](https://github.com/apollographql/apollo-server/pull/2451)
 - New plugin package `apollo-server-plugin-response-cache` implementing a full query response cache based on `apollo-cache-control` hints. The implementation added a few hooks and context fields; see the PR for details. There is a slight change to `cacheControl` object: previously, `cacheControl.stripFormattedExtensions` defaulted to false if you did not provide a `cacheControl` option object, but defaulted to true if you provided (eg) `cacheControl: {defaultMaxAge: 10}`. Now `stripFormattedExtensions` defaults to false unless explicitly provided as `true`, or if you use the legacy boolean `cacheControl: true`. [PR #2437](https://github.com/apollographql/apollo-server/pull/2437)
 - Allow `GraphQLRequestListener` callbacks in plugins to depend on `this`. [PR #2470](https://github.com/apollographql/apollo-server/pull/2470)

packages/apollo-server-core/src/__tests__/errors.test.ts

@@ -9,6 +9,9 @@ import {
   ValidationError,
   UserInputError,
   SyntaxError,
+  hasPersistedQueryError,
+  PersistedQueryNotFoundError,
+  PersistedQueryNotSupportedError,
 } from 'apollo-server-errors';
 
 describe('Errors', () => {
@@ -184,4 +187,37 @@ describe('Errors', () => {
       expect(formattedError.extensions.exception.field2).toEqual('property2');
     });
   });
+  describe('hasPersistedQueryError', () => {
+    it('should return true if errors contain error of type PersistedQueryNotFoundError', () => {
+      const errors = [
+        new PersistedQueryNotFoundError(),
+        new AuthenticationError('401'),
+      ];
+      const result = hasPersistedQueryError(errors);
+      expect(result).toBe(true);
+    });
+
+    it('should return true if errors contain error of type PersistedQueryNotSupportedError', () => {
+      const errors = [
+        new PersistedQueryNotSupportedError(),
+        new AuthenticationError('401'),
+      ];
+      const result = hasPersistedQueryError(errors);
+      expect(result).toBe(true);
+    });
+
+    it('should return false if errors does not contain PersistedQuery error', () => {
+      const errors = [
+        new ForbiddenError('401'),
+        new AuthenticationError('401'),
+      ];
+      const result = hasPersistedQueryError(errors);
+      expect(result).toBe(false);
+    });
+
+    it('should return false if an error is thrown', () => {
+      const result = hasPersistedQueryError({});
+      expect(result).toBe(false);
+    });
+  });
 });

packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts

@@ -3,7 +3,16 @@ import MockReq = require('mock-req');
 
 import { GraphQLSchema, GraphQLObjectType, GraphQLString } from 'graphql';
 
-import { runHttpQuery, HttpQueryError } from '../runHttpQuery';
+import {
+  runHttpQuery,
+  HttpQueryError,
+  throwHttpGraphQLError,
+} from '../runHttpQuery';
+import {
+  PersistedQueryNotFoundError,
+  PersistedQueryNotSupportedError,
+  ForbiddenError,
+} from 'apollo-server-errors';
 
 const queryType = new GraphQLObjectType({
   name: 'QueryType',
@@ -46,4 +55,36 @@ describe('runHttpQuery', () => {
       });
     });
   });
+
+  describe('throwHttpGraphQLError', () => {
+    it('should add no-cache headers if error is of type PersistedQueryNotSupportedError', () => {
+      try {
+        throwHttpGraphQLError(200, [new PersistedQueryNotSupportedError()]);
+      } catch (err) {
+        expect(err.headers).toEqual({
+          'Content-Type': 'application/json',
+          'Cache-Control': 'private, no-cache, must-revalidate',
+        });
+      }
+    });
+
+    it('should add no-cache headers if error is of type PersistedQueryNotFoundError', () => {
+      try {
+        throwHttpGraphQLError(200, [new PersistedQueryNotFoundError()]);
+      } catch (err) {
+        expect(err.headers).toEqual({
+          'Content-Type': 'application/json',
+          'Cache-Control': 'private, no-cache, must-revalidate',
+        });
+      }
+    });
+
+    it('should not add no-cache headers if error is not a PersitedQuery error', () => {
+      try {
+        throwHttpGraphQLError(200, [new ForbiddenError('401')]);
+      } catch (err) {
+        expect(err.headers).toEqual({ 'Content-Type': 'application/json' });
+      }
+    });
+  });
 });

packages/apollo-server-core/src/runHttpQuery.ts

@@ -7,6 +7,7 @@ import {
   formatApolloErrors,
   PersistedQueryNotSupportedError,
   PersistedQueryNotFoundError,
+  hasPersistedQueryError,
 } from 'apollo-server-errors';
 import {
   processGraphQLRequest,
@@ -70,11 +71,19 @@ export class HttpQueryError extends Error {
 /**
  * If options is specified, then the errors array will be formatted
  */
-function throwHttpGraphQLError<E extends Error>(
+export function throwHttpGraphQLError<E extends Error>(
   statusCode: number,
   errors: Array<E>,
   options?: Pick<GraphQLOptions, 'debug' | 'formatError'>,
 ): never {
+  const defaultHeaders = { 'Content-Type': 'application/json' };
+  // force no-cache on PersistedQuery errors
+  const headers = hasPersistedQueryError(errors)
+    ? {
+        ...defaultHeaders,
+        'Cache-Control': 'private, no-cache, must-revalidate',
+      }
+    : defaultHeaders;
   throw new HttpQueryError(
     statusCode,
     prettyJSONStringify({
@@ -86,9 +95,7 @@ function throwHttpGraphQLError<E extends Error>(
         : errors,
     }),
     true,
-    {
-      'Content-Type': 'application/json',
-    },
+    headers,
   );
 }
 
@@ -280,6 +287,7 @@ export async function processHTTPRequest<TContext>(
 
       try {
         const requestContext = buildRequestContext(request);
+
         const response = await processGraphQLRequest(options, requestContext);
 
         // This code is run on parse/validation errors and any other error that

packages/apollo-server-errors/src/index.ts

@@ -264,3 +264,13 @@ export function formatApolloErrors(
     }
   }) as Array<ApolloError>;
 }
+
+export function hasPersistedQueryError(errors: Array<Error>): boolean {
+  return Array.isArray(errors)
+    ? errors.some(
+        error =>
+          error instanceof PersistedQueryNotFoundError ||
+          error instanceof PersistedQueryNotSupportedError,
+      )
+    : false;
+}