From 10a9083622cf21d79725ca6fb0874c4057f3783f Mon Sep 17 00:00:00 2001 From: Ryan Johnson Date: Fri, 5 Apr 2019 12:47:31 -0400 Subject: [PATCH] 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 + .../src/__tests__/errors.test.ts | 36 ++++++++++++++++ .../src/__tests__/runHttpQuery.test.ts | 43 ++++++++++++++++++- .../apollo-server-core/src/runHttpQuery.ts | 16 +++++-- packages/apollo-server-errors/src/index.ts | 10 +++++ 5 files changed, 102 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b8ff24b0dda..f0443043793 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/packages/apollo-server-core/src/__tests__/errors.test.ts b/packages/apollo-server-core/src/__tests__/errors.test.ts index 37860a117d6..c922de1c3f7 100644 --- a/packages/apollo-server-core/src/__tests__/errors.test.ts +++ b/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); + }); + }); }); diff --git a/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts b/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts index 2f0a1565d0b..32198300d64 100644 --- a/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts +++ b/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' }); + } + }); + }); }); diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index 5159dc88154..d8f02820d82 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/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( +export function throwHttpGraphQLError( statusCode: number, errors: Array, options?: Pick, ): 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( : errors, }), true, - { - 'Content-Type': 'application/json', - }, + headers, ); } @@ -280,6 +287,7 @@ export async function processHTTPRequest( try { const requestContext = buildRequestContext(request); + const response = await processGraphQLRequest(options, requestContext); // This code is run on parse/validation errors and any other error that diff --git a/packages/apollo-server-errors/src/index.ts b/packages/apollo-server-errors/src/index.ts index 48aafde5154..f330df8a7fe 100644 --- a/packages/apollo-server-errors/src/index.ts +++ b/packages/apollo-server-errors/src/index.ts @@ -264,3 +264,13 @@ export function formatApolloErrors( } }) as Array; } + +export function hasPersistedQueryError(errors: Array): boolean { + return Array.isArray(errors) + ? errors.some( + error => + error instanceof PersistedQueryNotFoundError || + error instanceof PersistedQueryNotSupportedError, + ) + : false; +}