Skip to content

fix(handler): Response maker handles errors correctly #45

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

Merged
merged 2 commits into from
Jan 20, 2023
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
9 changes: 9 additions & 0 deletions src/__tests__/fixtures/simple.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
GraphQLString,
GraphQLNonNull,
GraphQLSchemaConfig,
GraphQLInt,
} from 'graphql';

export const schemaConfig: GraphQLSchemaConfig = {
Expand All @@ -14,6 +15,14 @@ export const schemaConfig: GraphQLSchemaConfig = {
type: new GraphQLNonNull(GraphQLString),
resolve: () => 'world',
},
num: {
type: GraphQLInt,
args: {
num: {
type: GraphQLInt,
},
},
},
},
}),
mutation: new GraphQLObjectType({
Expand Down
23 changes: 23 additions & 0 deletions src/__tests__/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,3 +97,26 @@ it('should respond with error if execution result is iterable', async () => {
],
});
});

it('should correctly serialise execution result errors', async () => {
const server = startTServer();
const url = new URL(server.url);
url.searchParams.set('query', 'query ($num: Int) { num(num: $num) }');
url.searchParams.set('variables', JSON.stringify({ num: 'foo' }));
const result = await fetch(url.toString());
expect(result.json()).resolves.toMatchInlineSnapshot(`
{
"errors": [
{
"locations": [
{
"column": 8,
"line": 1,
},
],
"message": "Variable "$num" got invalid value "foo"; Int cannot represent non-integer value: "foo"",
},
],
}
`);
});
40 changes: 20 additions & 20 deletions src/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
areGraphQLErrors,
isAsyncIterable,
isExecutionResult,
isGraphQLError,
isObject,
} from './utils';

Expand Down Expand Up @@ -681,12 +682,24 @@ export function makeResponse(
| Readonly<GraphQLError>,
acceptedMediaType: AcceptableMediaType,
): Response {
if (isExecutionResult(resultOrErrors)) {
const errors = isGraphQLError(resultOrErrors)
? [resultOrErrors]
: areGraphQLErrors(resultOrErrors)
? resultOrErrors
: null;
if (errors) {
return [
JSON.stringify(resultOrErrors),
JSON.stringify({ errors }),
{
status: 200,
statusText: 'OK',
...(acceptedMediaType === 'application/json'
? {
status: 200,
statusText: 'OK',
}
: {
status: 400,
statusText: 'Bad Request',
}),
headers: {
'content-type':
acceptedMediaType === 'application/json'
Expand All @@ -698,23 +711,10 @@ export function makeResponse(
}

return [
JSON.stringify({
errors: Array.isArray(resultOrErrors)
? isObject(resultOrErrors)
? resultOrErrors
: new GraphQLError(String(resultOrErrors))
: [resultOrErrors],
}),
JSON.stringify(resultOrErrors),
{
...(acceptedMediaType === 'application/json'
? {
status: 200,
statusText: 'OK',
}
: {
status: 400,
statusText: 'Bad Request',
}),
status: 200,
statusText: 'OK',
headers: {
'content-type':
acceptedMediaType === 'application/json'
Expand Down
13 changes: 9 additions & 4 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
*
*/

import type { ExecutionResult, GraphQLError } from 'graphql';
import type { ExecutionResult } from 'graphql';
import { GraphQLError } from 'graphql';

/** @private */
export function extendedTypeof(
Expand Down Expand Up @@ -42,13 +43,17 @@ export function isObject(val: unknown): val is Record<
export function areGraphQLErrors(obj: unknown): obj is readonly GraphQLError[] {
return (
Array.isArray(obj) &&
// must be at least one error
obj.length > 0 &&
// error has at least a message
obj.every((ob) => 'message' in ob)
// if one item in the array is a GraphQLError, we're good
obj.some(isGraphQLError)
);
}

/** @private */
export function isGraphQLError(obj: unknown): obj is GraphQLError {
return obj instanceof GraphQLError;
}

/** @private */
export function isExecutionResult(val: unknown): val is ExecutionResult {
return (
Expand Down