From bb23f607354db2c3d1443e29c65c2e360f04fa64 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Mon, 9 Nov 2020 23:17:21 +0200 Subject: [PATCH] Wrap HttpError into GraphQLError (#716) Fixes #699 --- .eslintrc.yml | 2 +- src/__tests__/http-test.ts | 37 ++++++++++++++++++++++++++++++++++--- src/index.ts | 35 +++++++++++++++++++++++++++-------- src/parseBody.ts | 2 +- 4 files changed, 63 insertions(+), 13 deletions(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index 3c65ee95..c5bdef89 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -474,7 +474,7 @@ overrides: '@typescript-eslint/no-extraneous-class': off # TODO consider '@typescript-eslint/no-floating-promises': error '@typescript-eslint/no-for-in-array': error - '@typescript-eslint/no-implicit-any-catch': off # TODO: Enable + '@typescript-eslint/no-implicit-any-catch': error '@typescript-eslint/no-implied-eval': error '@typescript-eslint/no-inferrable-types': [error, { ignoreParameters: true, ignoreProperties: true }] diff --git a/src/__tests__/http-test.ts b/src/__tests__/http-test.ts index 54fbcd0d..dfab1ebc 100644 --- a/src/__tests__/http-test.ts +++ b/src/__tests__/http-test.ts @@ -1199,7 +1199,7 @@ function runTests(server: Server) { }); }); - it('allows for custom error formatting to sanitize', async () => { + it('allows for custom error formatting to sanitize GraphQL errors', async () => { const app = server(); app.get( @@ -1207,7 +1207,10 @@ function runTests(server: Server) { graphqlHTTP({ schema: TestSchema, customFormatErrorFn(error) { - return { message: 'Custom error format: ' + error.message }; + return { + message: + `Custom ${error.constructor.name} format: ` + error.message, + }; }, }), ); @@ -1223,7 +1226,35 @@ function runTests(server: Server) { data: { thrower: null }, errors: [ { - message: 'Custom error format: Throws!', + message: 'Custom GraphQLError format: Throws!', + }, + ], + }); + }); + + it('allows for custom error formatting to sanitize HTTP errors', async () => { + const app = server(); + + app.get( + urlString(), + graphqlHTTP({ + schema: TestSchema, + customFormatErrorFn(error) { + return { + message: + `Custom ${error.constructor.name} format: ` + error.message, + }; + }, + }), + ); + + const response = await app.request().get(urlString()); + + expect(response.status).to.equal(400); + expect(JSON.parse(response.text)).to.deep.equal({ + errors: [ + { + message: 'Custom GraphQLError format: Must provide query string.', }, ], }); diff --git a/src/index.ts b/src/index.ts index a185ff99..6b891963 100644 --- a/src/index.ts +++ b/src/index.ts @@ -8,7 +8,6 @@ import type { ExecutionArgs, ExecutionResult, FormattedExecutionResult, - GraphQLError, GraphQLSchema, GraphQLFieldResolver, GraphQLTypeResolver, @@ -16,8 +15,10 @@ import type { } from 'graphql'; import accepts from 'accepts'; import httpError from 'http-errors'; +import type { HttpError } from 'http-errors'; import { Source, + GraphQLError, parse, validate, execute, @@ -206,7 +207,7 @@ export function graphqlHTTP(options: Options): Middleware { // Parse the Request to get GraphQL request parameters. try { params = await getGraphQLParams(request); - } catch (error) { + } catch (error: unknown) { // When we failed to parse the GraphQL parameters, we still need to get // the options object, so make an options call to resolve just that. const optionsData = await resolveOptions(); @@ -284,7 +285,7 @@ export function graphqlHTTP(options: Options): Middleware { let documentAST; try { documentAST = parseFn(new Source(query, 'GraphQL request')); - } catch (syntaxError) { + } catch (syntaxError: unknown) { // Return 400: Bad Request if any syntax errors errors exist. throw httpError(400, 'GraphQL syntax error.', { graphqlErrors: [syntaxError], @@ -337,7 +338,7 @@ export function graphqlHTTP(options: Options): Middleware { fieldResolver, typeResolver, }); - } catch (contextError) { + } catch (contextError: unknown) { // Return 400: Bad Request if any execution context errors exist. throw httpError(400, 'GraphQL execution context error.', { graphqlErrors: [contextError], @@ -359,9 +360,15 @@ export function graphqlHTTP(options: Options): Middleware { result = { ...result, extensions }; } } - } catch (error) { + } catch (rawError: unknown) { // If an error was caught, report the httpError status, or 500. - response.statusCode = error.status ?? 500; + const error: HttpError = httpError( + 500, + /* istanbul ignore next: Thrown by underlying library. */ + rawError instanceof Error ? rawError : String(rawError), + ); + + response.statusCode = error.status; const { headers } = error; if (headers != null) { @@ -370,7 +377,19 @@ export function graphqlHTTP(options: Options): Middleware { } } - result = { data: undefined, errors: error.graphqlErrors ?? [error] }; + if (error.graphqlErrors == null) { + const graphqlError = new GraphQLError( + error.message, + undefined, + undefined, + undefined, + undefined, + error, + ); + result = { data: undefined, errors: [graphqlError] }; + } else { + result = { data: undefined, errors: error.graphqlErrors }; + } } // If no data was included in the result, that indicates a runtime query @@ -482,7 +501,7 @@ export async function getGraphQLParams( if (typeof variables === 'string') { try { variables = JSON.parse(variables); - } catch (error) { + } catch { throw httpError(400, 'Variables are invalid JSON.'); } } else if (typeof variables !== 'object') { diff --git a/src/parseBody.ts b/src/parseBody.ts index 0fd3e4f4..c86e5e7a 100644 --- a/src/parseBody.ts +++ b/src/parseBody.ts @@ -51,7 +51,7 @@ export async function parseBody( if (jsonObjRegex.test(rawBody)) { try { return JSON.parse(rawBody); - } catch (error) { + } catch { // Do nothing } }