Skip to content

Commit

Permalink
Invalid non-nullable or required List Type variable should return Use…
Browse files Browse the repository at this point in the history 
…rInputError (#6066)

Co-authored-by: gchen02 <[email protected]>
Co-authored-by: David Glasser <[email protected]>
  • Loading branch information
@zeroshine @glasser
3 people committed Feb 23, 2022
1 parent ea4fe02 commit ee67234
Show file tree
Hide file tree
Showing 3 changed files with 141 additions and 69 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ The version headers in this history reflect the versions of Apollo Server itself

- `apollo-server-core`: The inline trace plugin will now include the full query plan and subgraph traces if manually installed in an Apollo Gateway. (Previously, you technically could install this plugin in a Gateway but it would not have any real trace data.) This is recommended for development use only and not in production servers. [PR #6017](https://github.com/apollographql/apollo-server/pull/6017)
- `apollo-server-core`: The default landing page plugins now take an `includeCookies` option which allows you to specify that Explorer should send cookies to your server. [PR #6014](https://github.com/apollographql/apollo-server/pull/6014)
- `apollo-server-core`: Apollo Server has a heuristic added in v2.23.0 and improved in v3.1.0 which tries to detect execution errors that come from the `graphql-js` variable value validation phase and report them with an `extensions.code` of `BAD_USER_INPUT` rather than `INTERNAL_SERVER_ERROR`. In this release, the heuristic is improved to include some cases including variables that are non-null lists. [PR #6066](https://github.com/apollographql/apollo-server/pull/6066)

## v3.6.2

Expand Down
175 changes: 123 additions & 52 deletions packages/server/src/__tests__/integration/ApolloServer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,72 +313,143 @@ export function testApolloServer<AS extends ApolloServerBase>(
});
});

it('variable coercion errors', async () => {
const { url: uri } = await createApolloServer({
typeDefs: gql`
type Query {
hello(x: String): String
}
`,
describe('appropriate error for bad user input', () => {
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; String cannot represent a non string value: 2/,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});

const apolloFetch = createApolloFetch({ uri });
it('catches required type variable error and returns UserInputError', async () => {
const { url: uri } = await createApolloServer({
typeDefs: gql`
type Query {
hello(x: String!): String
}
`,
});

const result = await apolloFetch({
query: `query ($x:String) {hello(x:$x)}`,
variables: { x: 2 },
const apolloFetch = createApolloFetch({ uri });

const result = await apolloFetch({
query: `query ($x:String!) {hello(x:$x)}`,
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toMatch(
`Variable "$x" of required type "String!" was not provided.`,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toMatch(
/got invalid value 2; String cannot represent a non string value: 2/,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});

it('catches required type variable error and returns UserInputError', async () => {
const { url: uri } = await createApolloServer({
typeDefs: gql`
type Query {
hello(x: String!): String
}
`,
it('catches required List type variable error and returns UserInputError', 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)}`,
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toMatch(
`Variable "$x" of required type "[String]!" was not provided.`,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});

const apolloFetch = createApolloFetch({ uri });
it('catches non-null type variable error and returns UserInputError', async () => {
const { url: uri } = await createApolloServer({
typeDefs: gql`
type Query {
hello(x: String!): String
}
`,
});

const result = await apolloFetch({
query: `query ($x:String!) {hello(x:$x)}`,
const apolloFetch = createApolloFetch({ uri });

const result = await apolloFetch({
query: `query ($x:String!) {hello(x:$x)}`,
variables: { x: null },
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toMatch(
`Variable "$x" of non-null type "String!" must not be null.`,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toMatch(
`Variable "$x" of required type "String!" was not provided.`,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});

it('catches non-null type variable error and returns UserInputError', async () => {
const { url: uri } = await createApolloServer({
typeDefs: gql`
type Query {
hello(x: String!): String
}
`,
it('catches non-null List type variable error and returns UserInputError', 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: null },
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toMatch(
`Variable "$x" of non-null type "[String]!" must not be null.`,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});

const apolloFetch = createApolloFetch({ uri });
it('catches List of non-null type variable error and returns UserInputError', async () => {
const { url: uri } = await createApolloServer({
typeDefs: gql`
type Query {
hello(x: [String!]!): String
}
`,
});

const result = await apolloFetch({
query: `query ($x:String!) {hello(x:$x)}`,
variables: { x: null },
const apolloFetch = createApolloFetch({ uri });

const result = await apolloFetch({
query: `query ($x:[String!]!) {hello(x:$x)}`,
variables: { x: [null] },
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toBe(
`Variable "$x" got invalid value null at "x[0]"; ` +
`Expected non-nullable type "String!" not to be null.`,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});
expect(result.data).toBeUndefined();
expect(result.errors).toBeDefined();
expect(result.errors[0].message).toMatch(
`Variable "$x" of non-null type "String!" must not be null.`,
);
expect(result.errors[0].extensions.code).toBe('BAD_USER_INPUT');
});

describe('schema creation', () => {
Expand Down
34 changes: 17 additions & 17 deletions packages/server/src/requestPipeline.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,22 @@ export interface GraphQLRequestPipelineConfig<TContext> {

type Mutable<T> = { -readonly [P in keyof T]: T[P] };

function isBadUserInputGraphQLError(error: GraphQLError): Boolean {
return (
error.nodes?.length === 1 &&
error.nodes[0].kind === Kind.VARIABLE_DEFINITION &&
(error.message.startsWith(
`Variable "$${error.nodes[0].variable.name.value}" got invalid value `,
) ||
error.message.startsWith(
`Variable "$${error.nodes[0].variable.name.value}" of required type `,
) ||
error.message.startsWith(
`Variable "$${error.nodes[0].variable.name.value}" of non-null type `,
))
);
}

export async function processGraphQLRequest<TContext>(
config: GraphQLRequestPipelineConfig<TContext>,
requestContext: Mutable<GraphQLRequestContext<TContext>>,
Expand Down Expand Up @@ -390,23 +406,7 @@ export async function processGraphQLRequest<TContext>(
// variable resolution from execution later; see
// https://github.com/graphql/graphql-js/issues/3169
const resultErrors = result.errors?.map((e) => {
if (
e.nodes?.length === 1 &&
e.nodes[0].kind === Kind.VARIABLE_DEFINITION &&
(e.message.startsWith(
`Variable "$${e.nodes[0].variable.name.value}" got invalid value `,
) ||
(e.nodes[0].type.kind === Kind.NON_NULL_TYPE &&
e.nodes[0].type.type.kind === Kind.NAMED_TYPE &&
(e.message.startsWith(
`Variable "$${e.nodes[0].variable.name.value}" of required ` +
`type "${e.nodes[0].type.type.name.value}!" was not provided.`,
) ||
e.message.startsWith(
`Variable "$${e.nodes[0].variable.name.value}" of non-null ` +
`type "${e.nodes[0].type.type.name.value}!" must not be null.`,
))))
) {
if (isBadUserInputGraphQLError(e)) {
return fromGraphQLError(e, {
errorClass: UserInputError,
});
Expand Down

0 comments on commit ee67234

Please sign in to comment.