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

Fully replace 'ApolloError' with 'GraphQLError' #6355

Merged
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
18 changes: 9 additions & 9 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
"cors": "2.8.5",
"cspell": "5.21.2",
"express": "4.18.1",
"graphql": "16.3.0",
"graphql": "16.5.0",
"graphql-tag": "2.12.6",
"jest": "28.1.0",
"jest-config": "28.1.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"whatwg-mimetype": "^3.0.0"
},
"peerDependencies": {
"graphql": "^16.3.0"
"graphql": "^16.5.0"
},
"volta": {
"extends": "../../package.json"
Expand Down
10 changes: 7 additions & 3 deletions packages/server/src/ApolloServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,10 @@ export type ContextThunk<TContext extends BaseContext = BaseContext> =
// TODO(AS4): Move this to its own file or something. Also organize the fields.

export interface ApolloServerInternals<TContext extends BaseContext> {
formatError?: (error: GraphQLError) => GraphQLFormattedError;
formatError?: (
formattedError: GraphQLFormattedError,
error: unknown,
) => GraphQLFormattedError;
// TODO(AS4): Is there a way (with generics/codegen?) to make
// this "any" more specific? In AS3 there was technically a
// generic for it but it was used inconsistently.
Expand Down Expand Up @@ -980,8 +983,9 @@ export class ApolloServer<TContext extends BaseContext = BaseContext> {
headers: new HeaderMap([['content-type', 'application/json']]),
completeBody: prettyJSONStringify({
errors: formatApolloErrors([e as Error], {
debug: this.internals.includeStackTracesInErrorResponses,
formatter: this.internals.formatError,
includeStackTracesInErrorResponses:
this.internals.includeStackTracesInErrorResponses,
formatError: this.internals.formatError,
}),
}),
bodyChunks: null,
Expand Down
77 changes: 3 additions & 74 deletions packages/server/src/__tests__/ApolloError.test.ts
Original file line number Diff line number Diff line change
@@ -1,80 +1,9 @@
import { ApolloError, ForbiddenError, AuthenticationError } from '../errors';

describe('ApolloError', () => {
it("doesn't overwrite extensions when provided in the constructor", () => {
const error = new ApolloError('My message', 'A_CODE', {
arbitrary: 'user_data',
});

expect(error.extensions).toEqual({
code: 'A_CODE',
arbitrary: 'user_data',
});
});

it("a code property doesn't overwrite the code provided to the constructor", () => {
const error = new ApolloError('My message', 'A_CODE', {
code: 'CANT_OVERWRITE',
});

expect(error.extensions).toEqual({
code: 'A_CODE',
});
});

// This is a byproduct of how we currently assign properties from the 3rd constructor
// argument onto properties of the class itself. This is expected, but deprecated behavior
// and as such this test should be deleted in the future when we make that breaking change.
it("a message property doesn't overwrite the message provided to the constructor", () => {
const error = new ApolloError('My original message', 'A_CODE', {
message:
"This message can't overwrite the original message, but it does end up in extensions",
});

expect(error.message).toEqual('My original message');
expect(error.extensions.message).toEqual(
"This message can't overwrite the original message, but it does end up in extensions",
);
});

it('throws for users who use an extensions key in the third constructor argument', () => {
expect(
() =>
new ApolloError('My original message', 'A_CODE', {
extensions: {
arbitrary: 'user_data',
},
}),
).toThrow(/Pass extensions directly/);
});

it('provides toJSON method', () => {
const error = new ApolloError('My original message', 'A_CODE', {
arbitrary: 'user_data',
});

expect(error.toJSON()).toEqual({
message: 'My original message',
extensions: {
code: 'A_CODE',
arbitrary: 'user_data',
},
});
});

it('provides toString method', () => {
const error = new ApolloError('My original message', 'A_CODE', {
arbitrary: 'user_data',
});

expect(error.toString()).toEqual('My original message');
});
});
import { ForbiddenError, AuthenticationError } from '../errors';

describe('ForbiddenError', () => {
it('supports arbitrary data being passed', () => {
const error = new ForbiddenError('My message', {
arbitrary: 'user_data',
extensions: { arbitrary: 'user_data' },
});

expect(error.extensions).toEqual({
Expand All @@ -87,7 +16,7 @@ describe('ForbiddenError', () => {
describe('AuthenticationError', () => {
it('supports arbitrary data being passed', () => {
const error = new AuthenticationError('My message', {
arbitrary: 'user_data',
extensions: { arbitrary: 'user_data' },
});

expect(error.extensions).toEqual({
Expand Down
75 changes: 33 additions & 42 deletions packages/server/src/__tests__/errors.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { GraphQLError } from 'graphql';

import {
ApolloError,
formatApolloErrors,
AuthenticationError,
ForbiddenError,
Expand All @@ -11,24 +10,6 @@ import {
} from '../errors';

describe('Errors', () => {
describe('ApolloError', () => {
const message = 'message';
it('defaults code to INTERNAL_SERVER_ERROR', () => {
const error = new ApolloError(message);
expect(error.message).toEqual(message);
expect(error.extensions.code).toBeUndefined();
});
it('allows code setting and additional properties', () => {
const code = 'CODE';
const key = 'value';
const error = new ApolloError(message, code, { key });
expect(error.message).toEqual(message);
expect(error.key).toBeUndefined();
expect(error.extensions.code).toEqual(code);
expect(error.extensions.key).toEqual(key);
});
});

describe('formatApolloErrors', () => {
type CreateFormatError =
| ((
Expand All @@ -45,7 +26,9 @@ describe('Errors', () => {
errors?: Error[],
) => {
if (errors === undefined) {
const error = new ApolloError(message, code, { key });
const error = new GraphQLError(message, {
extensions: { code, key },
});
return formatApolloErrors(
[
new GraphQLError(
Expand All @@ -65,7 +48,9 @@ describe('Errors', () => {
};

it('exposes a stacktrace in debug mode', () => {
const error = createFormattedError({ debug: true });
const error = createFormattedError({
includeStackTracesInErrorResponses: true,
});
expect(error.message).toEqual(message);
expect(error.extensions.key).toEqual(key);
expect(error.extensions.exception.key).toBeUndefined();
Expand All @@ -87,10 +72,10 @@ describe('Errors', () => {
),
])[0];
expect(error.message).toEqual(message);
expect(error.extensions.code).toEqual('INTERNAL_SERVER_ERROR');
expect(error.extensions.exception).toHaveProperty('key', key);
expect(error.extensions?.code).toEqual('INTERNAL_SERVER_ERROR');
expect(error.extensions?.exception).toHaveProperty('key', key);
// stacktrace should exist under exception
expect(error.extensions.exception).not.toHaveProperty('stacktrace');
expect(error.extensions?.exception).not.toHaveProperty('stacktrace');
});
it('exposes fields on error under exception field and provides code', () => {
const error = createFormattedError();
Expand All @@ -99,19 +84,23 @@ describe('Errors', () => {
expect(error.extensions.exception).toBeUndefined();
expect(error.extensions.code).toEqual(code);
});
it('calls formatter after exposing the code and stacktrace', () => {
const error = new ApolloError(message, code, { key });
const formatter = jest.fn();
it('calls formatError after exposing the code and stacktrace', () => {
const error = new GraphQLError(message, {
extensions: { code, key },
});
const formatError = jest.fn();
formatApolloErrors([error], {
formatter,
debug: true,
formatError,
includeStackTracesInErrorResponses: true,
});
expect(error.message).toEqual(message);
expect(error.extensions.key).toEqual(key);
expect(error.key).toBeUndefined();
expect(error.extensions.code).toEqual(code);
expect(error instanceof ApolloError).toBe(true);
expect(formatter).toHaveBeenCalledTimes(1);

expect(formatError).toHaveBeenCalledTimes(1);

const formatErrorArgs = formatError.mock.calls[0];
expect(formatErrorArgs[0].message).toEqual(message);
expect(formatErrorArgs[0].extensions.key).toEqual(key);
expect(formatErrorArgs[0].extensions.code).toEqual(code);
expect(formatErrorArgs[1]).toEqual(error);
});
it('Formats native Errors in a JSON-compatible way', () => {
const error = new Error('Hello');
Expand All @@ -122,7 +111,7 @@ describe('Errors', () => {
describe('Named Errors', () => {
const message = 'message';
function verifyError(
error: ApolloError,
error: GraphQLError,
{
code,
errorClass,
Expand All @@ -132,7 +121,7 @@ describe('Errors', () => {
expect(error.message).toEqual(message);
expect(error.extensions.code).toEqual(code);
expect(error.name).toEqual(name);
expect(error instanceof ApolloError).toBe(true);
expect(error instanceof GraphQLError).toBe(true);
expect(error instanceof errorClass).toBe(true);
}

Expand Down Expand Up @@ -166,8 +155,10 @@ describe('Errors', () => {
});
it('provides a user input error', () => {
const error = new UserInputError(message, {
field1: 'property1',
field2: 'property2',
extensions: {
field1: 'property1',
field2: 'property2',
},
});
verifyError(error, {
code: 'BAD_USER_INPUT',
Expand All @@ -186,9 +177,9 @@ describe('Errors', () => {
),
])[0];

expect(formattedError.extensions.field1).toEqual('property1');
expect(formattedError.extensions.field2).toEqual('property2');
expect(formattedError.extensions.exception).toBeUndefined();
expect(formattedError.extensions?.field1).toEqual('property1');
expect(formattedError.extensions?.field2).toEqual('property2');
expect(formattedError.extensions?.exception).toBeUndefined();
});
});
});
Loading