Skip to content

Commit

Permalink
Preserve the extensions property when pre-execution errors oc… (#3394)
Browse files Browse the repository at this point in the history
Without this change, any `extensions` that are present on the `response` are
not returned to the client when an error occurs in pre-execution stages,
despite the fact that `extensions` are a perfectly acceptable property to be
present when pre-execution errors occur[1].  (Note that `data` is not!)

For example, if a user had added their own `extensions` property in the
`willSendResponse` hook of a request pipeline API, it would have been
removed by the previous implementation of `throwHttpGraphQLError` which did
not receive, or accept `extensions`, since it explicitly only passed the
`errors` property.

This will all be greatly improved when we abstract this transport out!

[1]: https://graphql.github.io/graphql-spec/June2018/#sec-Response-Format
  • Loading branch information
abernix authored Oct 10, 2019
1 parent 54c287b commit fdaebaa
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ The version headers in this history reflect the versions of Apollo Server itself
- `@apollo/gateway`, `@apollo/federation`, `apollo-engine-reporting`: Update `apollo-graphql` dependency to bring in [`apollo-tooling`'s #1551](https://github.com/apollographql/apollo-tooling/pull/1551) which resolve runtime errors when its source is minified. While this fixes a particular minification bug when Apollo Server packages are minified, we _do not_ recommend minification of server code in most cases. [PR #3387](https://github.com/apollographql/apollo-server/pull/3387) [Issue #3335](https://github.com/apollographql/apollo-server/issues/3335)
- `apollo-server-koa`: Correctly declare dependency on `koa-compose`. [PR #3356](https://github.com/apollographql/apollo-server/pull/3356)
- `apollo-server-core`: Preserve any `extensions` that have been placed on the response when pre-execution errors occur. [PR #3394](https://github.com/apollographql/apollo-server/pull/3394)

### v2.9.3

Expand Down
33 changes: 24 additions & 9 deletions packages/apollo-server-core/src/runHttpQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
resolveGraphqlOptions,
} from './graphqlOptions';
import {
ApolloError,
formatApolloErrors,
PersistedQueryNotSupportedError,
PersistedQueryNotFoundError,
Expand All @@ -18,7 +19,7 @@ import {
} from './requestPipeline';
import { CacheControlExtensionOptions } from 'apollo-cache-control';
import { ApolloServerPlugin } from 'apollo-server-plugin-base';
import { WithRequired } from 'apollo-server-types';
import { WithRequired, GraphQLExecutionResult } from 'apollo-server-types';

export interface HttpQueryRequest {
method: string;
Expand Down Expand Up @@ -75,6 +76,7 @@ export function throwHttpGraphQLError<E extends Error>(
statusCode: number,
errors: Array<E>,
options?: Pick<GraphQLOptions, 'debug' | 'formatError'>,
extensions?: GraphQLExecutionResult['extensions'],
): never {
const defaultHeaders = { 'Content-Type': 'application/json' };
// force no-cache on PersistedQuery errors
Expand All @@ -84,16 +86,27 @@ export function throwHttpGraphQLError<E extends Error>(
'Cache-Control': 'private, no-cache, must-revalidate',
}
: defaultHeaders;

type Result =
& Pick<GraphQLExecutionResult, 'extensions'>
& { errors: E[] | ApolloError[] }

const result: Result = {
errors: options
? formatApolloErrors(errors, {
debug: options.debug,
formatter: options.formatError,
})
: errors,
};

if (extensions) {
result.extensions = extensions;
}

throw new HttpQueryError(
statusCode,
prettyJSONStringify({
errors: options
? formatApolloErrors(errors, {
debug: options.debug,
formatter: options.formatError,
})
: errors,
}),
prettyJSONStringify(result),
true,
headers,
);
Expand Down Expand Up @@ -303,6 +316,8 @@ export async function processHTTPRequest<TContext>(
return throwHttpGraphQLError(
(response.http && response.http.status) || 400,
response.errors as any,
undefined,
response.extensions,
);
}

Expand Down
42 changes: 42 additions & 0 deletions packages/apollo-server-integration-testsuite/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,48 @@ export function testApolloServer<AS extends ApolloServerBase>(
expect(result.errors).toBeDefined();
expect(apolloFetchResponse.status).toEqual(403);
});

it('preserves user-added "extensions" in the response when parsing errors occur', async () => {
await setupApolloServerAndFetchPairForPlugins([
{
requestDidStart() {
return {
willSendResponse({ response }) {
response.extensions = { myExtension: true };
},
};
},
},
]);

const result =
await apolloFetch({ query: '{ 🦠' });
expect(result.errors).toBeDefined();
expect(result.extensions).toEqual(expect.objectContaining({
myExtension: true
}));
});

it('preserves user-added "extensions" in the response when validation errors occur', async () => {
await setupApolloServerAndFetchPairForPlugins([
{
requestDidStart() {
return {
willSendResponse({ response }) {
response.extensions = { myExtension: true };
},
};
},
},
]);

const result =
await apolloFetch({ query: '{ missingFieldWhichWillNotValidate }' });
expect(result.errors).toBeDefined();
expect(result.extensions).toEqual(expect.objectContaining({
myExtension: true
}));
});
});

describe('formatError', () => {
Expand Down

0 comments on commit fdaebaa

Please sign in to comment.