Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add no-cache headers to PersistedQuery error responses #2452

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
da24aa8
add no-cache headers to PersistedQuery error responses
ryandrewjohnson Mar 15, 2019
bf5cdd8
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 15, 2019
09d44aa
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 15, 2019
d75c0c9
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 18, 2019
6c56a47
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 20, 2019
357e909
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 24, 2019
acba97f
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 26, 2019
2771c64
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 26, 2019
8267617
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 27, 2019
109a292
fix defaultHeaders typo
ryandrewjohnson Mar 27, 2019
40a1c93
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 28, 2019
1964ea1
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 29, 2019
2618f9c
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Mar 30, 2019
d50afe9
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Apr 1, 2019
1ad16fb
Merge branch 'master' into add-nocache-headers-persistedquery-errors
ryandrewjohnson Apr 3, 2019
cebb486
Merge branch 'release-2.5.0' into add-nocache-headers-persistedquery-…
abernix Apr 4, 2019
8374d35
remove try/catch on hasPersistedQueryError replace with Array.isArray…
ryandrewjohnson Apr 4, 2019
9aa0901
Add CHANGELOG.md for #2446.
abernix Apr 5, 2019
8b7ca90
Merge branch 'release-2.5.0' into add-nocache-headers-persistedquery-…
abernix Apr 5, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

### vNEXT

- 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)

### v2.4.8

- No functional changes in this version. The patch version has been bumped to fix the `README.md` displayed on the [npm package for `apollo-server`](https://npm.im/apollo-server) as a result of a broken publish. Apologies for the additional noise!
Expand Down
36 changes: 36 additions & 0 deletions packages/apollo-server-core/src/__tests__/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import {
ValidationError,
UserInputError,
SyntaxError,
hasPersistedQueryError,
PersistedQueryNotFoundError,
PersistedQueryNotSupportedError,
} from 'apollo-server-errors';

describe('Errors', () => {
Expand Down Expand Up @@ -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);
});
});
});
43 changes: 42 additions & 1 deletion packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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' });
}
});
});
});
16 changes: 12 additions & 4 deletions packages/apollo-server-core/src/runHttpQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
formatApolloErrors,
PersistedQueryNotSupportedError,
PersistedQueryNotFoundError,
hasPersistedQueryError,
} from 'apollo-server-errors';
import {
processGraphQLRequest,
Expand Down Expand Up @@ -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 defualtHeaders = { 'Content-Type': 'application/json' };
ryandrewjohnson marked this conversation as resolved.
Show resolved Hide resolved
// force no-cache on PersistedQuery errors
const headers = hasPersistedQueryError(errors)
? {
...defualtHeaders,
'Cache-Control': 'private, no-cache, must-revalidate',
}
: defualtHeaders;
throw new HttpQueryError(
statusCode,
prettyJSONStringify({
Expand All @@ -86,9 +95,7 @@ function throwHttpGraphQLError<E extends Error>(
: errors,
}),
true,
{
'Content-Type': 'application/json',
},
headers,
);
}

Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions packages/apollo-server-errors/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,3 +264,15 @@ export function formatApolloErrors(
}
}) as Array<ApolloError>;
}

export function hasPersistedQueryError(errors: Array<Error>): boolean {
try {
ryandrewjohnson marked this conversation as resolved.
Show resolved Hide resolved
return errors.some(
error =>
error instanceof PersistedQueryNotFoundError ||
error instanceof PersistedQueryNotSupportedError,
);
} catch (err) {
return false;
}
}