Skip to content

Commit

Permalink
apollo-server-core: use UserInputError for variable coercion errors
Browse files Browse the repository at this point in the history
This particular error can be trivially triggered by clients, so it doesn't make
sense to use `INTERNAL_SERVER_ERROR` for it. It seems like a good fit for
`BAD_USER_INPUT`, which previously was only used if you explicitly throw a
`UserInputError` in your app.

Fixes #3498.
  • Loading branch information
glasser committed Apr 6, 2021
1 parent 01dab00 commit 551a5dd
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 5 deletions.
29 changes: 25 additions & 4 deletions packages/apollo-server-core/src/requestPipeline.ts
Expand Up @@ -31,6 +31,7 @@ import {
PersistedQueryNotSupportedError,
PersistedQueryNotFoundError,
formatApolloErrors,
UserInputError,
} from 'apollo-server-errors';
import {
GraphQLRequest,
Expand Down Expand Up @@ -451,16 +452,36 @@ export async function processGraphQLRequest<TContext>(
requestContext as GraphQLRequestContextExecutionDidStart<TContext>,
);

if (result.errors) {
await didEncounterErrors(result.errors);
// The first thing that execution does is coerce the request's variables
// to the types declared in the operation, which can lead to errors if
// they are of the wrong type. We change any such errors into
// UserInputError so that their code doesn't end up being
// INTERNAL_SERVER_ERROR, since these are client errors.
const resultErrors = result.errors?.map((e) => {
if (
e.nodes?.length === 1 &&
e.nodes[0].kind === 'VariableDefinition' &&
e.message.startsWith(
`Variable "$${e.nodes[0].variable.name.value}" got invalid value `,
)
) {
return fromGraphQLError(e, {
errorConstructor: (message) => new UserInputError(message),
});
}
return e;
});

if (resultErrors) {
await didEncounterErrors(resultErrors);
}

response = {
...result,
errors: result.errors ? formatErrors(result.errors) : undefined,
errors: resultErrors ? formatErrors(resultErrors) : undefined,
};

executionDispatcher.reverseInvokeHookSync("executionDidEnd");
executionDispatcher.reverseInvokeHookSync('executionDidEnd');
} catch (executionError) {
executionDispatcher.reverseInvokeHookSync("executionDidEnd", executionError);
return await sendErrorResponse(executionError);
Expand Down
6 changes: 5 additions & 1 deletion packages/apollo-server-errors/src/index.ts
Expand Up @@ -134,12 +134,16 @@ export function toApolloError(

export interface ErrorOptions {
code?: string;
// Sometimes it's more convenient to pass a function than a class name.
errorConstructor?: (message: string) => ApolloError;
errorClass?: typeof ApolloError;
}

export function fromGraphQLError(error: GraphQLError, options?: ErrorOptions) {
const copy: ApolloError =
options && options.errorClass
options && options.errorConstructor
? options.errorConstructor(error.message)
: options && options.errorClass
? new options.errorClass(error.message)
: new ApolloError(error.message);

Expand Down
21 changes: 21 additions & 0 deletions packages/apollo-server-integration-testsuite/src/ApolloServer.ts
Expand Up @@ -348,6 +348,27 @@ export function testApolloServer<AS extends ApolloServerBase>(
});
});

it('variable coercion errors', async () => {
const { url: uri } = await createApolloServer({
typeDefs: gql`
type Query {
hello(x: String): String
}
`,
});

const apolloFetch = createApolloFetch({ uri });

const result = await apolloFetch({
query: `query ($x:String) {hello(x:$x)}`,
variables: { x: 2 },
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toMatch(/got invalid value 2; Expected type String/);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});

describe('schema creation', () => {
it('accepts typeDefs and resolvers', async () => {
const typeDefs = gql`
Expand Down

0 comments on commit 551a5dd

Please sign in to comment.