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

Error extensions improvements #6759

Merged
merged 1 commit into from
Aug 5, 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
14 changes: 14 additions & 0 deletions .changeset/small-suits-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
"@apollo/server-integration-testsuite": patch
"@apollo/server": patch
---

Refactor error formatting.

Remove `error.extensions.exception`; you can add it back yourself with `formatError`. `error.extensions.exception.stacktrace` is now available on `error.extensions.stacktrace`.

Provide `unwrapResolverError` function in `@apollo/server/errors`; useful for your `formatError` hook.

No more TS `declare module` describing the `exception` extension (partially incorrectly).

Rename the (new in v4) constructor option `includeStackTracesInErrorResponses` to `includeStacktraceInErrorResponses`.
109 changes: 101 additions & 8 deletions docs/source/migration.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,10 @@ The `startStandaloneServer` function accepts two arguments; the first is the ins
<td>

An optional `listen` configuration option. The `listen` option accepts an object with the same properties as the [`net.Server.listen` options object](https://nodejs.org/api/net.html#serverlistenoptions-callback).

For example, in Apollo Server 3, if you used `server.listen(4321)`, you'll now pass `listen: { port: 4321 }` to the `startStandaloneServer` function in Apollo Server 4. If you didn't pass any arguments to Apollo Server 3's `server.listen()` method; you don't need to specify this `listen` option.
</td>

</tr>

</tbody>
Expand Down Expand Up @@ -654,15 +654,15 @@ In Apollo Server 3, the `debug` constructor option (which defaults to `true` unl
- Apollo Server 3 rarely sends messages at the `DEBUG` level, so this primarily affects plugins that use the provided `logger` to send `DEBUG` messages.
- The `debug` flag is also available to plugins on `GraphQLRequestContext` to use as they wish.

Apollo Server 4 removes the `debug` constructor option. In its place is a new `includeStackTracesInErrorResponses` option which controls its namesake feature. Like `debug`, this option defaults to `true` unless the `NODE_ENV` environment variable is either `production` or `test`.
Apollo Server 4 removes the `debug` constructor option. In its place is a new `includeStacktraceInErrorResponses` option which controls its namesake feature. Like `debug`, this option defaults to `true` unless the `NODE_ENV` environment variable is either `production` or `test`.

If you use `debug` in Apollo Server 3, you can use `includeStackTracesInErrorResponses` with the same value in Apollo Server 4:
If you use `debug` in Apollo Server 3, you can use `includeStacktraceInErrorResponses` with the same value in Apollo Server 4:

```ts
const apolloServerInstance = new ApolloServer<MyContext>({
typeDefs,
resolvers,
includeStackTracesInErrorResponses: true,
includeStacktraceInErrorResponses: true,
});
```

Expand All @@ -679,6 +679,8 @@ const server = new ApolloServer({
});
```

(Note that the stack traces themselves have moved from `extensions.exception.stacktrace` to `extensions.stacktrace`.)

### `formatResponse` hook

Apollo Server 3 provides the `formatResponse` hook as a top-level constructor argument. The `formatResponse` hook is called after an operation successfully gets to the "execution" stage, enabling you to transform the structure of GraphQL response objects before sending them to a client.
Expand Down Expand Up @@ -1082,7 +1084,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, which has the following signature:

Expand All @@ -1100,12 +1104,17 @@ In Apollo Server 4, this becomes:

Above, `formattedError` is the default JSON object sent in the response according to the [GraphQL specification](https://spec.graphql.org/draft/#sec-Errors), and `error` is the originally thrown error. If you need a field from the original error that isn't in `GraphQLFormattedError`, you can access that value from the `error` argument.

One caveat: if the error was thrown inside a resolver, `error` is not the error your resolver code threw; it is a `GraphQLError` wrapping it and adding helpful context such as the `path` in the operation to the resolver's field. If you want the exact error you threw, Apollo Server 4 provides the `unwrapResolverError` function in `@apollo/server/errors`, which removes this outer layer if you pass it a resolver error (and returns its argument otherwise).

So, you can format errors like this:

```ts
import { unwrapResolverError } from '@apollo/server/errors';

new ApolloServer({
formatError: (formattedError, error) => {
// Don't give the specific errors to the client.
if (error instanceof CustomDBError) {
if (unwrapResolverError(error) instanceof CustomDBError) {
return { message: 'Internal server error' };
}

Expand All @@ -1122,8 +1131,92 @@ So, you can format errors like this:
// be manipulated in other ways, as long as it's returned.
return formattedError;
},
// ...
});
```

#### `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 originalError = unwrapResolverError(error);
const exception: Record<string, unknown> = {
...(typeof originalError === 'object' ? originalError : 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 Expand Up @@ -1390,7 +1483,7 @@ The `@apollo/utils.fetcher` package has a more precise name and only supports ar

### `@apollo/cache-control-types`

In Apollo Server 3, you could import the `CacheScope`, `CacheHint`, `CacheAnnotation`, `CachePolicy`, and `ResolveInfoCacheControl` types from your chosen Apollo Server package.
In Apollo Server 3, you could import the `CacheScope`, `CacheHint`, `CacheAnnotation`, `CachePolicy`, and `ResolveInfoCacheControl` types from your chosen Apollo Server package.

In Apollo Server 4, the new `@apollo/cache-control-types` package exports the [`CacheScope`](#cachescope-type), `CacheHint`, `CacheAnnotation`, `CachePolicy`, and `ResolveInfoCacheControl` types. This enables libraries that work with multiple GraphQL servers (such as `@apollo/subgraph`) to refer to these types without depending on `@apollo/server`.

Expand Down
120 changes: 8 additions & 112 deletions packages/integration-testsuite/src/apolloServerTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -775,110 +775,6 @@ export function defineIntegrationTestSuiteApolloServerTests(
);
}
});

it('works with errors similar to GraphQL errors, such as yup', async () => {
// https://npm.im/yup is a package that produces a particular type of
// error that we test compatibility with. This test was first brought
// with https://github.com/apollographql/apollo-server/pull/1288. We
// used to use the actual `yup` package to generate the error, but we
// don't need to actually bundle that dependency just to test
// compatibility with that particular error shape. To be honest, it's
// not clear from the original PR which attribute of this error need be
// mocked, but for the sake not not breaking anything, all of yup's
// error properties have been reproduced here.
const throwError = jest.fn(async () => {
// Intentionally `any` because this is a custom Error class with
// various custom properties (like `value` and `params`).
const yuppieError: any = new Error('email must be a valid email');
yuppieError.name = 'ValidationError';

// Set `message` to enumerable, which `yup` does and `Error` doesn't.
Object.defineProperty(yuppieError, 'message', {
enumerable: true,
});

// Set other properties which `yup` sets.
yuppieError.path = 'email';
yuppieError.type = undefined;
yuppieError.value = { email: 'invalid-email' };
yuppieError.errors = ['email must be a valid email'];
yuppieError.inner = [];
yuppieError.params = {
path: 'email',
value: 'invalid-email',
originalValue: 'invalid-email',
label: undefined,
regex: /@/,
};

// This stack is fake, but roughly what `yup` generates!
yuppieError.stack = [
'ValidationError: email must be a valid email',
' at createError (yup/lib/util/createValidation.js:64:35)',
' at yup/lib/util/createValidation.js:113:108',
' at process._tickCallback (internal/process/next_tick.js:68:7)',
].join('\n');

throw yuppieError;
});

const formatError = jest.fn(() => {
return {
message: 'User Input Error',
extensions: { code: 'BAD_USER_INPUT' },
};
});

const uri = await createServerAndGetUrl({
typeDefs: gql`
type Query {
fieldWhichWillError: String
}
`,
resolvers: {
Query: {
fieldWhichWillError: () => {
return throwError();
},
},
},
introspection: true,
includeStackTracesInErrorResponses: true,
formatError,
});

const apolloFetch = createApolloFetch({ uri });

const result = await apolloFetch({
query: '{fieldWhichWillError}',
});

expect(throwError).toHaveBeenCalledTimes(1);
expect(formatError).toHaveBeenCalledTimes(1);
const formatErrorArgs: any = formatError.mock.calls[0];
expect(formatErrorArgs[0]).toMatchObject({
message: 'email must be a valid email',
path: ['fieldWhichWillError'],
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);

expect(result.data).toEqual({ fieldWhichWillError: null });
expect(result.errors).toBeDefined();
expect(result.errors[0].extensions.code).toEqual('BAD_USER_INPUT');
expect(result.errors[0].message).toEqual('User Input Error');
});
});

describe('lifecycle', () => {
Expand Down Expand Up @@ -1020,7 +916,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
}),
...plugins,
],
includeStackTracesInErrorResponses: true,
includeStacktraceInErrorResponses: true,
stopOnTerminationSignals: false,
nodeEnv: '',
...constructorOptions,
Expand Down Expand Up @@ -1459,7 +1355,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
},
],
formatError,
includeStackTracesInErrorResponses: true,
includeStacktraceInErrorResponses: true,
});
const apolloFetch = createApolloFetch({ uri });
const result = await apolloFetch({
Expand Down Expand Up @@ -1691,7 +1587,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
expect(e.message).toMatch('valid result');
expect(e.extensions).toBeDefined();
expect(e.extensions.code).toEqual('SOME_CODE');
expect(e.extensions.exception.stacktrace).toBeDefined();
expect(e.extensions.stacktrace).toBeDefined();

expect(contextCreationDidFail.mock.calls).toMatchInlineSnapshot(`
Array [
Expand Down Expand Up @@ -1765,7 +1661,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 +1692,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,8 +1724,8 @@ 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).toBeDefined();
expect(result.errors[0].extensions.exception.stacktrace).toBeDefined();
expect(result.errors[0].extensions).not.toHaveProperty('exception');
expect(result.errors[0].extensions.stacktrace).toBeDefined();
});

it('shows error extensions in extensions (only!)', async () => {
Expand All @@ -1850,7 +1746,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
},
stopOnTerminationSignals: false,
nodeEnv: 'development',
includeStackTracesInErrorResponses: false,
includeStacktraceInErrorResponses: false,
});

const apolloFetch = createApolloFetch({ uri });
Expand Down
Loading