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.

Sign up for GitHub

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

Jump to bottom

Add error code to parse/validate errors passed to 'didEncounterErrors' #6572

Merged
merged 2 commits into from
Jun 30, 2022
Merged

Add error code to parse/validate errors passed to 'didEncounterErrors' #6572

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3b2955
Add error code to parse/validate errors passed to 'didEncounterErrors'
IvanGoncharov Jun 14, 2022
2b1851d
Address review comments
IvanGoncharov Jun 30, 2022
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
8 changes: 6 additions & 2 deletions packages/server/src/ApolloServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ import Negotiator from 'negotiator';
import * as uuid from 'uuid';
import { newCachePolicy } from './cachePolicy.js';
import { determineApolloConfig } from './determineApolloConfig.js';
import { BadRequestError, ensureError, formatApolloErrors } from './errors.js';
import {
BadRequestError,
ensureError,
normalizeAndFormatErrors,
} from './errors.js';
import type {
ApolloServerPlugin,
BaseContext,
Expand Down Expand Up @@ -1064,7 +1068,7 @@ export class ApolloServer<TContext extends BaseContext = BaseContext> {
['content-type', 'application/json'],
]),
completeBody: prettyJSONStringify({
errors: formatApolloErrors([error], {
errors: normalizeAndFormatErrors([error], {
includeStackTracesInErrorResponses:
this.internals.includeStackTracesInErrorResponses,
formatError: this.internals.formatError,
Expand Down
31 changes: 29 additions & 2 deletions packages/server/src/__tests__/ApolloServer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,8 +315,35 @@ describe('ApolloServer executeOperation', () => {
await server.start();

const { result } = await server.executeOperation({ query: '{' });
expect(result.errors).toHaveLength(1);
expect(result.errors?.[0].extensions?.code).toBe('GRAPHQL_PARSE_FAILED');
expect(result.errors).toEqual([
{
message: 'Syntax Error: Expected Name, found <EOF>.',
locations: [{ line: 1, column: 2 }],
extensions: {
code: 'GRAPHQL_PARSE_FAILED',
},
},
]);
await server.stop();
});

it('validation errors', async () => {
const server = new ApolloServer({
typeDefs,
resolvers,
});
await server.start();

const { result } = await server.executeOperation({ query: '{ unknown }' });
expect(result.errors).toEqual([
{
message: 'Cannot query field "unknown" on type "Query".',
locations: [{ line: 1, column: 3 }],
extensions: {
code: 'GRAPHQL_VALIDATION_FAILED',
},
},
]);
await server.stop();
});

Expand Down
20 changes: 10 additions & 10 deletions packages/server/src/__tests__/errors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ import {
UserInputError,
SyntaxError,
} from '..';
import { formatApolloErrors } from '../errors.js';
import { normalizeAndFormatErrors } from '../errors.js';

describe('Errors', () => {
describe('formatApolloErrors', () => {
describe('normalizeAndFormatErrors', () => {
type CreateFormatError =
| ((
options: Record<string, any>,
Expand All @@ -29,7 +29,7 @@ describe('Errors', () => {
const error = new GraphQLError(message, {
extensions: { code, key },
});
return formatApolloErrors(
return normalizeAndFormatErrors(
[
new GraphQLError(
error.message,
Expand All @@ -43,7 +43,7 @@ describe('Errors', () => {
options,
)[0];
} else {
return formatApolloErrors(errors, options);
return normalizeAndFormatErrors(errors, options);
}
};

Expand All @@ -61,7 +61,7 @@ describe('Errors', () => {
it('hides stacktrace by default', () => {
const thrown = new Error(message);
(thrown as any).key = key;
const error = formatApolloErrors([
const error = normalizeAndFormatErrors([
new GraphQLError(
thrown.message,
undefined,
Expand Down Expand Up @@ -89,7 +89,7 @@ describe('Errors', () => {
extensions: { code, key },
});
const formatError = jest.fn();
formatApolloErrors([error], {
normalizeAndFormatErrors([error], {
formatError,
includeStackTracesInErrorResponses: true,
});
Expand All @@ -104,7 +104,7 @@ describe('Errors', () => {
});
it('Formats native Errors in a JSON-compatible way', () => {
const error = new Error('Hello');
const [formattedError] = formatApolloErrors([error]);
const [formattedError] = normalizeAndFormatErrors([error]);
expect(JSON.parse(JSON.stringify(formattedError)).message).toBe('Hello');
});
});
Expand Down Expand Up @@ -140,14 +140,14 @@ describe('Errors', () => {
});
});
it('provides a syntax error', () => {
verifyError(new SyntaxError(message), {
verifyError(new SyntaxError(new GraphQLError(message)), {
code: 'GRAPHQL_PARSE_FAILED',
errorClass: SyntaxError,
name: 'SyntaxError',
});
});
it('provides a validation error', () => {
verifyError(new ValidationError(message), {
verifyError(new ValidationError(new GraphQLError(message)), {
code: 'GRAPHQL_VALIDATION_FAILED',
errorClass: ValidationError,
name: 'ValidationError',
Expand All @@ -166,7 +166,7 @@ describe('Errors', () => {
name: 'UserInputError',
});

const formattedError = formatApolloErrors([
const formattedError = normalizeAndFormatErrors([
new GraphQLError(
error.message,
undefined,
Expand Down
31 changes: 29 additions & 2 deletions packages/server/src/__tests__/runQuery.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -777,7 +777,7 @@ describe('request pipeline life-cycle hooks', () => {
},
];

it('called when an error occurs', async () => {
it('called when an parsing error occurs', async () => {
await runQuery(
{
schema,
Expand All @@ -788,7 +788,34 @@ describe('request pipeline life-cycle hooks', () => {

expect(didEncounterErrors).toBeCalledWith(
expect.objectContaining({
errors: expect.arrayContaining([expect.any(Error)]),
errors: expect.arrayContaining([
expect.objectContaining({
message: 'Syntax Error: Expected Name, found "}".',
extensions: { code: 'GRAPHQL_PARSE_FAILED' },
}),
]),
}),
);
});

it('called when a validation error occurs', async () => {
await runQuery(
{
schema,
plugins,
},
{ query: '{ testStringWithParseError }' },
);

expect(didEncounterErrors).toBeCalledWith(
expect.objectContaining({
errors: expect.arrayContaining([
expect.objectContaining({
message:
'Cannot query field "testStringWithParseError" on type "QueryType".',
extensions: { code: 'GRAPHQL_VALIDATION_FAILED' },
}),
]),
}),
);
});
Expand Down
57 changes: 25 additions & 32 deletions packages/server/src/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,31 @@ declare module 'graphql' {
}

export class SyntaxError extends GraphQLError {
constructor(message: string) {
super(message, { extensions: { code: 'GRAPHQL_PARSE_FAILED' } });
constructor(graphqlError: GraphQLError) {
super(graphqlError.message, {
source: graphqlError.source,
positions: graphqlError.positions,
extensions: {
...graphqlError.extensions,
code: 'GRAPHQL_PARSE_FAILED',
},
originalError: graphqlError,
});

this.name = 'SyntaxError';
}
}

export class ValidationError extends GraphQLError {
constructor(message: string) {
super(message, { extensions: { code: 'GRAPHQL_VALIDATION_FAILED' } });
constructor(graphqlError: GraphQLError) {
super(graphqlError.message, {
nodes: graphqlError.nodes,
extensions: {
...graphqlError.extensions,
code: 'GRAPHQL_VALIDATION_FAILED',
},
originalError: graphqlError.originalError ?? graphqlError,
});

this.name = 'ValidationError';
}
Expand Down Expand Up @@ -102,46 +117,25 @@ export class BadRequestError extends GraphQLError {
}
}

// This function accepts any value that were thrown and convert it to GraphQLFormatterError.
// It also add default extensions.code and copy stack trace onto an extension if requested.
// This function should not throw.
export function formatApolloErrors(
export function normalizeAndFormatErrors(
errors: ReadonlyArray<unknown>,
options: {
formatError?: (
formattedError: GraphQLFormattedError,
error: unknown,
) => GraphQLFormattedError;
includeStackTracesInErrorResponses?: boolean;
errorCode?: string;
} = {},
): Array<GraphQLFormattedError> {
// Errors that occur in graphql-tools can contain an errors array that contains the errors thrown in a merged schema
// https://github.com/apollographql/graphql-tools/blob/3d53986ca/src/stitching/errors.ts#L104-L107
//
// They are are wrapped in an extra GraphQL error
// https://github.com/apollographql/graphql-tools/blob/3d53986ca/src/stitching/errors.ts#L109-L113
// which calls:
// https://github.com/graphql/graphql-js/blob/0a30b62964/src/error/locatedError.js#L18-L37
// Some processing for these nested errors could be done here:
//
// if (Array.isArray((error as any).errors)) {
// (error as any).errors.forEach(e => flattenedErrors.push(e));
// } else if (
// (error as any).originalError &&
// Array.isArray((error as any).originalError.errors)
// ) {
// (error as any).originalError.errors.forEach(e => flattenedErrors.push(e));
// } else {
// flattenedErrors.push(error);
// }

const { includeStackTracesInErrorResponses, errorCode } = options;

const formatError = options.formatError ?? ((error) => error);
return errors.map((error) => {
try {
return formatError(enrichError(error), error);
} catch (formattingError) {
if (includeStackTracesInErrorResponses) {
if (options.includeStackTracesInErrorResponses) {
// includeStackTracesInErrorResponses is used in development
// so it will be helpful to show errors thrown by formatError hooks in that mode
return enrichError(formattingError);
Expand All @@ -165,8 +159,7 @@ export function formatApolloErrors(

const extensions: GraphQLErrorExtensions = {
...graphqlError.extensions,
code:
graphqlError.extensions.code ?? errorCode ?? 'INTERNAL_SERVER_ERROR',
code: graphqlError.extensions.code ?? 'INTERNAL_SERVER_ERROR',
};

const { originalError } = graphqlError;
Expand All @@ -183,7 +176,7 @@ export function formatApolloErrors(
}
}

if (includeStackTracesInErrorResponses) {
if (options.includeStackTracesInErrorResponses) {
extensions.exception = {
...extensions.exception,
stacktrace: graphqlError.stack?.split('\n'),
Expand Down
Loading