Skip to content

Commit

Permalink
Updates (see PR description)
Browse files Browse the repository at this point in the history
  • Loading branch information
glasser committed Aug 4, 2022
1 parent 0282712 commit 407ef56
Show file tree
Hide file tree
Showing 7 changed files with 278 additions and 95 deletions.
87 changes: 84 additions & 3 deletions docs/source/migration.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -759,8 +759,6 @@ throw new ApolloError(message, 'YOUR_ERROR_CODE');
you should now pass your error code to the `extensions` option; see the above code snippet for an example.
In Apollo Server 3, the `code` and `stacktrace` fields on errors are found at `error.extensions.exception.code` and `error.extensions.exception.stacktrace`; `error.extensions.exception` also contains any enumerable properties on the originally thrown error. In Apollo Server 4, these fields are found directly at `error.extensions.code` and `error.extensions.stacktrace`; `error.extensions.exception` is reserved for the enumerable properties of the originally thrown error only.
### Built-in error classes
Apollo Server 3 exports several specific error classes. Apollo Server's code produces some of them (`SyntaxError`, `ValidationError`, and `UserInputError`). Others (`ForbiddenError` and `AuthenticationError`) are provided for users to use in their apps. All of these are subclasses of the `ApolloError` class.
Expand Down Expand Up @@ -1028,7 +1026,9 @@ expect(result.data?.hello).toBe('Hello world!'); // -> true
</MultiCodeBlock>
### `formatError` hook improvements
### Error formatting changes
#### `formatError` improvements
Apollo Server 3 supports the `formatError` hook with the following signature:
```
Expand Down Expand Up @@ -1066,6 +1066,87 @@ So now you can format errors as such:
},
```
#### `error.extensions.exception` is removed
When Apollo Server 3 formats an error, it may add an extension called `exception`. This extension is an object with a field for every *enumerable* property of the originally thrown error. (This does not apply if the originally thrown error was already a `GraphQLError`.) In addition, [when in dev/debug mode](#debug), `exception` contains an array of strings called `stacktrace`.
For example, if this code runs in a resolver:
```js
const e = new Error("hello");
e.extraProperty = "bye";
throw e;
```
then (in debug mode) Apollo Server 3 will format the error like this:
```js
{
"errors": [
{
"message": "hello",
"locations": [
{
"line": 2,
"column": 3
}
],
"path": ["x"],
"extensions": {
"code": "INTERNAL_SERVER_ERROR",
"exception": {
"extraProperty": "bye",
"stacktrace": [
"Error: hello",
" at Object.x (file:///private/tmp/as3-t/server.mjs:8:27)",
" at field.resolve (/private/tmp/as3-t/node_modules/apollo-server-core/dist/utils/schemaInstrumentation.js:56:26)",
// more lines elided
]
}
}
}
]
}
```
It was often hard to predict exactly which properties of which errors would be publicly exposed in this manner, which could lead to surprising information leaks.
In Apollo Server 4, there is no `exception` extension. The `stacktrace` is provided directly on `extensions`. If you'd like to copy some or all properties from the original error onto the formatted error, you can do that with the `formatError` hook.
If you'd like your errors to be formatted like they are in Apollo Server 3 (with the stack trace and the enumerable properties of the original error on the `exception` extension), you can provide this `formatError` implementation:
<MultiCodeBlock>
```ts
function formatError(
formattedError: GraphQLFormattedError,
error: unknown,
) {
const exception: Record<string, unknown> = {
...(typeof error === 'object' ? error : null),
};
delete exception.extensions;
if (formattedError.extensions?.stacktrace) {
exception.stacktrace = formattedError.extensions.stacktrace;
}
const extensions: Record<string, unknown> = {
...formattedError.extensions,
exception,
};
delete extensions.stacktrace;
return {
...formattedError,
extensions,
};
}
```
</MultiCodeBlock>
Apollo Server 3.5.0 and newer included a TypeScript `declare module` declaration that teaches TypeScript that `error.extensions.exception.stacktrace` is an array of strings on *all* `GraphQLError` objects. Apollo Server 4 does not provide a replacement for this: that is, we do not tell TypeScript the type of `error.extensions.code` or `error.extensions.stacktrace`. (The Apollo Server 3 `declare module` declaration also incorrectly teaches TypeScript that `error.extensions.exception.code` is a string, which should have been `error.extensions.code` instead.)
### HTTP error handling changes
Apollo Server 3 returns specific errors relating to GraphQL operations over HTTP/JSON as `text/plain` error messages.
Expand Down
14 changes: 3 additions & 11 deletions packages/integration-testsuite/src/apolloServerTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -862,14 +862,6 @@ export function defineIntegrationTestSuiteApolloServerTests(
locations: [{ line: 1, column: 2 }],
extensions: {
code: 'INTERNAL_SERVER_ERROR',
exception: {
name: 'ValidationError',
message: 'email must be a valid email',
type: undefined,
value: { email: 'invalid-email' },
errors: ['email must be a valid email'],
path: 'email',
},
},
});
expect(formatErrorArgs[1] instanceof Error).toBe(true);
Expand Down Expand Up @@ -1765,7 +1757,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
expect(result.errors).toBeDefined();
expect(result.errors.length).toEqual(1);
expect(result.errors[0].extensions.code).toEqual('SOME_CODE');
expect(result.errors[0].extensions.exception).toBeUndefined();
expect(result.errors[0].extensions).not.toHaveProperty('exception');
});

it('propagates error codes with null response in production', async () => {
Expand Down Expand Up @@ -1796,7 +1788,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
expect(result.errors).toBeDefined();
expect(result.errors.length).toEqual(1);
expect(result.errors[0].extensions.code).toEqual('SOME_CODE');
expect(result.errors[0].extensions.exception).toBeUndefined();
expect(result.errors[0].extensions).not.toHaveProperty('exception');
});

it('propagates error codes in dev mode', async () => {
Expand Down Expand Up @@ -1828,7 +1820,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
expect(result.errors).toBeDefined();
expect(result.errors.length).toEqual(1);
expect(result.errors[0].extensions.code).toEqual('SOME_CODE');
expect(result.errors[0].extensions.exception).toBeUndefined();
expect(result.errors[0].extensions).not.toHaveProperty('exception');
expect(result.errors[0].extensions.stacktrace).toBeDefined();
});

Expand Down
91 changes: 84 additions & 7 deletions packages/integration-testsuite/src/httpServerTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
} from '@jest/globals';
import type { Mock, SpyInstance } from 'jest-mock';
import { cacheControlFromInfo } from '@apollo/cache-control-types';
import { ApolloServerErrorCode } from '@apollo/server/errors';

const QueryRootType = new GraphQLObjectType({
name: 'QueryRoot',
Expand Down Expand Up @@ -174,12 +175,21 @@ const queryType = new GraphQLObjectType({
testError: {
type: GraphQLString,
resolve() {
throw new Error('Secret error message');
throw new MyError('Secret error message');
},
},
testGraphQLError: {
type: GraphQLString,
resolve() {
throw new MyGraphQLError('Secret error message');
},
},
},
});

class MyError extends Error {}
class MyGraphQLError extends GraphQLError {}

const mutationType = new GraphQLObjectType({
name: 'MutationType',
fields: {
Expand Down Expand Up @@ -1403,21 +1413,88 @@ export function defineIntegrationTestSuiteHttpServerTests(

it('formatError receives error that passes instanceof checks', async () => {
const expected = '--blank--';
let gotMyError = false;
const app = await createApp({
schema,
formatError: (_, error) => {
expect(error instanceof Error).toBe(true);
expect(error instanceof GraphQLError).toBe(true);
gotMyError = error instanceof MyError;
return { message: expected };
},
});
const req = request(app).post('/').send({
const res = await request(app).post('/').send({
query: 'query test{ testError }',
});
return req.then((res) => {
expect(res.status).toEqual(200);
expect(res.body.errors[0].message).toEqual(expected);
expect(res.status).toEqual(200);
expect(res.body).toMatchInlineSnapshot(`
Object {
"data": Object {
"testError": null,
},
"errors": Array [
Object {
"message": "--blank--",
},
],
}
`);
expect(gotMyError).toBe(true);
});

it('formatError receives error that passes instanceof checks when GraphQLError', async () => {
const expected = '--blank--';
let gotMyGraphQLError = false;
const app = await createApp({
schema,
formatError: (_, error) => {
gotMyGraphQLError = error instanceof MyGraphQLError;
return { message: expected };
},
});
const res = await request(app).post('/').send({
query: 'query test{ testGraphQLError }',
});
expect(res.status).toEqual(200);
expect(res.body).toMatchInlineSnapshot(`
Object {
"data": Object {
"testGraphQLError": null,
},
"errors": Array [
Object {
"message": "--blank--",
},
],
}
`);
expect(gotMyGraphQLError).toBe(true);
});

it('formatError receives correct error for parse failure', async () => {
const expected = '--blank--';
let gotCorrectCode = false;
const app = await createApp({
schema,
formatError: (_, error) => {
gotCorrectCode =
(error as any).extensions.code ===
ApolloServerErrorCode.GRAPHQL_PARSE_FAILED;
return { message: expected };
},
});
const res = await request(app).post('/').send({
query: '}',
});
expect(res.status).toEqual(400);
expect(res.body).toMatchInlineSnapshot(`
Object {
"errors": Array [
Object {
"message": "--blank--",
},
],
}
`);
expect(gotCorrectCode).toBe(true);
});

it('allows for custom error formatting to sanitize', async () => {
Expand Down
12 changes: 6 additions & 6 deletions packages/server/src/__tests__/ApolloServer.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ApolloServer } from '..';
import type { ApolloServerOptions, GatewayInterface } from '..';
import type { GraphQLSchema } from 'graphql';
import { GraphQLError, GraphQLSchema } from 'graphql';
import type { ApolloServerPlugin, BaseContext } from '../externalTypes';
import { ApolloServerPluginCacheControlDisabled } from '../plugin/disabled/index.js';
import { ApolloServerPluginUsageReporting } from '../plugin/usageReporting/index.js';
Expand All @@ -23,9 +23,9 @@ const resolvers = {
return 'world';
},
error() {
const myError = new Error('A test error');
(myError as any).someField = 'value';
throw myError;
throw new GraphQLError('A test error', {
extensions: { someField: 'value' },
});
},
contextFoo(_root: any, _args: any, context: any) {
return context.foo;
Expand Down Expand Up @@ -254,7 +254,7 @@ describe('ApolloServer executeOperation', () => {
expect(result.errors).toHaveLength(1);
expect(result.errors?.[0].extensions).toStrictEqual({
code: 'INTERNAL_SERVER_ERROR',
exception: { someField: 'value' },
someField: 'value',
});
await server.stop();
});
Expand All @@ -275,7 +275,7 @@ describe('ApolloServer executeOperation', () => {
const extensions = result.errors?.[0].extensions;
expect(extensions).toHaveProperty('code', 'INTERNAL_SERVER_ERROR');
expect(extensions).toHaveProperty('stacktrace');
expect(extensions).toHaveProperty('exception', { someField: 'value' });
expect(extensions).toHaveProperty('someField', 'value');
await server.stop();
});

Expand Down
Loading

0 comments on commit 407ef56

Please sign in to comment.