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

Officially support throwing from didResolveOperation #7001

Merged
merged 1 commit into from
Oct 7, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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