Skip to content

Commit

Permalink
Officially support throwing from didResolveOperation (#7001)
Browse files Browse the repository at this point in the history
(Also make an existing vaguely related test a bit more clear.)
  • Loading branch information
glasser authored Oct 7, 2022
1 parent e3bd95a commit 63d568d
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/seven-avocados-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/server-integration-testsuite': patch
---

Test the behavior of didResolveOperation hooks throwing.
2 changes: 2 additions & 0 deletions docs/source/integrations/plugins-event-reference.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,8 @@ This event is _not_ associated with your GraphQL server's _resolvers_. When this

> If the operation is anonymous (i.e., the operation is `query { ... }` instead of `query NamedQuery { ... }`), then `operationName` is `null`.
If a `didResolveOperation` hook throws an [`GraphQLError`](../data/errors#custom-errors), the error will be serialized and returned to the client, with an HTTP status code of 500 unless [the error specifies a different status code](../data/errors#setting-http-status-code-and-headers). Because this hook has access to the parsed and validated operation as well as request-specific context (such as the `contextValue`) that makes this plugin hook a good place to perform extra validation. (`didResolveOperation` hooks from multiple plugins are run in parallel and if more than one throws, only one error will be sent to the client.)

```ts
didResolveOperation?(
requestContext: WithRequired<
Expand Down
4 changes: 2 additions & 2 deletions packages/integration-testsuite/src/apolloServerTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1099,8 +1099,8 @@ export function defineIntegrationTestSuiteApolloServerTests(
const reports = await reportIngress.promiseOfReports;
expect(reports.length).toBe(1);

expect(Object.keys(reports[0].tracesPerQuery)[0]).not.toEqual(
'# -\n{ aliasedField: justAField }',
expect(Object.keys(reports[0].tracesPerQuery)[0]).toEqual(
'# -\n{justAField}',
);
});

Expand Down
68 changes: 68 additions & 0 deletions packages/integration-testsuite/src/httpServerTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,74 @@ export function defineIntegrationTestSuiteHttpServerTests(
`);
});

it('throwing in didResolveOperation results in error with default HTTP status code', async () => {
const app = await createApp({
schema,
plugins: [
{
async requestDidStart() {
return {
async didResolveOperation() {
throw new GraphQLError('error with default status');
},
};
},
},
],
});
const res = await request(app)
.post('/')
.send({ query: `{ testString }` });
expect(res.status).toEqual(500);
expect(res.body).toMatchInlineSnapshot(`
{
"errors": [
{
"extensions": {
"code": "INTERNAL_SERVER_ERROR",
},
"message": "error with default status",
},
],
}
`);
});

it('throwing in didResolveOperation results in error with specified HTTP status code', async () => {
const app = await createApp({
schema,
plugins: [
{
async requestDidStart() {
return {
async didResolveOperation() {
throw new GraphQLError('error with another status', {
extensions: { http: { status: 401 }, code: 'OH_NO' },
});
},
};
},
},
],
});
const res = await request(app)
.post('/')
.send({ query: `{ testString }` });
expect(res.status).toEqual(401);
expect(res.body).toMatchInlineSnapshot(`
{
"errors": [
{
"extensions": {
"code": "OH_NO",
},
"message": "error with another status",
},
],
}
`);
});

it('multiple operations with no `operationName` specified returns 400 and OPERATION_RESOLUTION_FAILURE', async () => {
const app = await createApp();
const res = await request(app)
Expand Down
4 changes: 4 additions & 0 deletions packages/server/src/requestPipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,10 @@ export async function processGraphQLRequest<TContext extends BaseContext>(
),
);
} catch (err: unknown) {
// Note that we explicitly document throwing `GraphQLError`s from
// `didResolveOperation` as a good way to do validation that depends on the
// validated operation and the request context. (It will have status 500 by
// default.)
return await sendErrorResponse([ensureGraphQLError(err)]);
}

Expand Down

0 comments on commit 63d568d

Please sign in to comment.