diff --git a/resources/benchmark.ts b/resources/benchmark.ts index 6d92af02f4..891c9af32d 100644 --- a/resources/benchmark.ts +++ b/resources/benchmark.ts @@ -286,7 +286,9 @@ function runBenchmark( results.push(computeStats(revision, samples)); process.stdout.write(' ' + cyan(i + 1) + ' tests completed.\u000D'); } catch (error) { - console.log(' ' + revision + ': ' + red(error.message)); + const errorMessage = + error instanceof Error ? error.message : String(error); + console.log(' ' + revision + ': ' + red(errorMessage)); } } console.log('\n'); diff --git a/src/__testUtils__/__tests__/expectPromise-test.ts b/src/__testUtils__/__tests__/expectPromise-test.ts index cd25624567..35b7c60aa4 100644 --- a/src/__testUtils__/__tests__/expectPromise-test.ts +++ b/src/__testUtils__/__tests__/expectPromise-test.ts @@ -22,7 +22,8 @@ describe('expectPromise', () => { 'foo', ); /* c8 ignore start */ } /* c8 ignore stop */ catch (err) { - expect(err.message).to.equal( + const errorMessage = err instanceof Error ? err.message : String(err); + expect(errorMessage).to.equal( "Promise should have rejected with message 'foo', but resolved as '{}'", ); } @@ -34,7 +35,8 @@ describe('expectPromise', () => { 'bar', ); /* c8 ignore start */ } /* c8 ignore stop */ catch (err) { - expect(err.message).to.equal( + const errorMessage = err instanceof Error ? err.message : String(err); + expect(errorMessage).to.equal( "expected Error: foo to have property 'message' of 'bar', but got 'foo'", ); } diff --git a/src/__testUtils__/expectPromise.ts b/src/__testUtils__/expectPromise.ts index b374faea3c..7c18f4a39b 100644 --- a/src/__testUtils__/expectPromise.ts +++ b/src/__testUtils__/expectPromise.ts @@ -14,7 +14,7 @@ export function expectPromise(maybePromise: unknown) { return maybePromise; }, async toRejectWith(message: string) { - let caughtError: Error | undefined; + let caughtError: unknown; let resolved; let rejected = false; try { diff --git a/src/error/__tests__/ensureGraphQLError-test.ts b/src/error/__tests__/ensureGraphQLError-test.ts new file mode 100644 index 0000000000..3a1233fba4 --- /dev/null +++ b/src/error/__tests__/ensureGraphQLError-test.ts @@ -0,0 +1,32 @@ +import { expect } from 'chai'; +import { describe, it } from 'mocha'; + +import { ensureGraphQLError } from '../ensureGraphQLError.js'; +import { GraphQLError } from '../GraphQLError.js'; + +describe('ensureGraphQLError', () => { + it('passes GraphQLError through', () => { + const error = new GraphQLError('boom'); + expect(ensureGraphQLError(error)).to.equal(error); + }); + + it('wraps Error values as GraphQLError', () => { + const originalError = new Error('boom'); + const error = ensureGraphQLError(originalError); + + expect(error).to.be.instanceOf(GraphQLError); + expect(error.message).to.equal('boom'); + expect(error.originalError).to.equal(originalError); + }); + + it('wraps non-error thrown values', () => { + const error = ensureGraphQLError('boom'); + + expect(error).to.be.instanceOf(GraphQLError); + expect(error.message).to.equal('Unexpected error value: "boom"'); + expect(error.originalError).to.include({ + name: 'NonErrorThrown', + thrownValue: 'boom', + }); + }); +}); diff --git a/src/error/ensureGraphQLError.ts b/src/error/ensureGraphQLError.ts new file mode 100644 index 0000000000..54f9d46112 --- /dev/null +++ b/src/error/ensureGraphQLError.ts @@ -0,0 +1,15 @@ +import { toError } from '../jsutils/toError.js'; + +import { GraphQLError } from './GraphQLError.js'; + +/** + * Ensure an unknown thrown value is represented as a GraphQLError. + */ +export function ensureGraphQLError(rawError: unknown): GraphQLError { + if (rawError instanceof GraphQLError) { + return rawError; + } + + const originalError = toError(rawError); + return new GraphQLError(originalError.message, { originalError }); +} diff --git a/src/execution/Executor.ts b/src/execution/Executor.ts index 7d16f63d3d..546662e0e7 100644 --- a/src/execution/Executor.ts +++ b/src/execution/Executor.ts @@ -12,6 +12,7 @@ import { promiseForObject } from '../jsutils/promiseForObject.js'; import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js'; import { promiseReduce } from '../jsutils/promiseReduce.js'; +import { ensureGraphQLError } from '../error/ensureGraphQLError.js'; import type { GraphQLFormattedError } from '../error/GraphQLError.js'; import { GraphQLError } from '../error/GraphQLError.js'; import { locatedError } from '../error/locatedError.js'; @@ -302,7 +303,7 @@ export class Executor< }, (error: unknown) => { onFinish(); - this.collectedErrors.add(error as GraphQLError, undefined); + this.collectedErrors.add(ensureGraphQLError(error), undefined); return this.buildResponse(null); }, ); @@ -312,7 +313,7 @@ export class Executor< return this.buildResponse(result); } catch (error) { onFinish(); - this.collectedErrors.add(error as GraphQLError, undefined); + this.collectedErrors.add(ensureGraphQLError(error), undefined); return this.buildResponse(null); } } diff --git a/src/execution/execute.ts b/src/execution/execute.ts index d15ffef673..10b15aa7dc 100644 --- a/src/execution/execute.ts +++ b/src/execution/execute.ts @@ -8,6 +8,7 @@ import type { Path } from '../jsutils/Path.js'; import { addPath, pathToArray } from '../jsutils/Path.js'; import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js'; +import { ensureGraphQLError } from '../error/ensureGraphQLError.js'; import { GraphQLError } from '../error/GraphQLError.js'; import { locatedError } from '../error/locatedError.js'; @@ -529,13 +530,13 @@ function createSourceEventStreamImpl( const eventStream = executeSubscription(validatedExecutionArgs); if (isPromise(eventStream)) { return eventStream.then(undefined, (error: unknown) => ({ - errors: [error as GraphQLError], + errors: [ensureGraphQLError(error)], })); } return eventStream; } catch (error) { - return { errors: [error] }; + return { errors: [ensureGraphQLError(error)] }; } } diff --git a/src/execution/incremental/IncrementalPublisher.ts b/src/execution/incremental/IncrementalPublisher.ts index 3d52bab194..52f1bf6db9 100644 --- a/src/execution/incremental/IncrementalPublisher.ts +++ b/src/execution/incremental/IncrementalPublisher.ts @@ -1,6 +1,7 @@ import type { ObjMap } from '../../jsutils/ObjMap.js'; import { pathToArray } from '../../jsutils/Path.js'; +import { ensureGraphQLError } from '../../error/ensureGraphQLError.js'; import type { GraphQLError } from '../../error/GraphQLError.js'; import { mapAsyncIterable } from '../mapAsyncIterable.js'; @@ -211,7 +212,7 @@ export class IncrementalPublisher { const id = this._ensureId(group); context.completed.push({ id, - errors: [error as GraphQLError], + errors: [ensureGraphQLError(error)], }); this._ids.delete(group); break; @@ -250,7 +251,7 @@ export class IncrementalPublisher { const stream = event.stream; context.completed.push({ id: this._ensureId(stream), - errors: [event.error as GraphQLError], + errors: [ensureGraphQLError(event.error)], }); this._ids.delete(stream); break; diff --git a/src/execution/legacyIncremental/BranchingIncrementalPublisher.ts b/src/execution/legacyIncremental/BranchingIncrementalPublisher.ts index e8fc4f926d..83c5690100 100644 --- a/src/execution/legacyIncremental/BranchingIncrementalPublisher.ts +++ b/src/execution/legacyIncremental/BranchingIncrementalPublisher.ts @@ -1,6 +1,7 @@ import type { ObjMap } from '../../jsutils/ObjMap.js'; import { addPath, pathToArray } from '../../jsutils/Path.js'; +import { ensureGraphQLError } from '../../error/ensureGraphQLError.js'; import type { GraphQLError } from '../../error/GraphQLError.js'; import type { @@ -157,7 +158,7 @@ export class BranchingIncrementalPublisher { path: pathToArray(group.path), }, group.label, - [event.error as GraphQLError], + [ensureGraphQLError(event.error)], ), ); break; @@ -205,7 +206,7 @@ export class BranchingIncrementalPublisher { path: pathToArray(stream.path), }, stream.label, - [event.error as GraphQLError], + [ensureGraphQLError(event.error)], ), ); break; diff --git a/src/execution/values.ts b/src/execution/values.ts index d531d99cf3..5b2a1ceee2 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -3,6 +3,7 @@ import type { Maybe } from '../jsutils/Maybe.js'; import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap.js'; import { printPathArray } from '../jsutils/printPathArray.js'; +import { ensureGraphQLError } from '../error/ensureGraphQLError.js'; import { GraphQLError } from '../error/GraphQLError.js'; import type { @@ -89,7 +90,7 @@ export function getVariableValues( return { variableValues }; } } catch (error) { - errors.push(error); + errors.push(ensureGraphQLError(error)); } return { errors }; diff --git a/src/graphql.ts b/src/graphql.ts index 5608d17478..8653da489d 100644 --- a/src/graphql.ts +++ b/src/graphql.ts @@ -1,6 +1,7 @@ import { isPromise } from './jsutils/isPromise.js'; import type { PromiseOrValue } from './jsutils/PromiseOrValue.js'; +import { ensureGraphQLError } from './error/ensureGraphQLError.js'; import type { GraphQLError } from './error/GraphQLError.js'; import type { DocumentNode } from './language/ast.js'; @@ -104,14 +105,14 @@ function graphqlImpl(args: GraphQLArgs): PromiseOrValue { try { document = harness.parse(source, args); } catch (syntaxError) { - return { errors: [syntaxError] }; + return { errors: [ensureGraphQLError(syntaxError)] }; } if (isPromise(document)) { return document.then( (resolvedDocument) => validateAndExecute(harness, args, schema, resolvedDocument), - (syntaxError: unknown) => ({ errors: [syntaxError as GraphQLError] }), + (syntaxError: unknown) => ({ errors: [ensureGraphQLError(syntaxError)] }), ); } diff --git a/src/utilities/validateInputValue.ts b/src/utilities/validateInputValue.ts index 1c6709b035..bb7cad1460 100644 --- a/src/utilities/validateInputValue.ts +++ b/src/utilities/validateInputValue.ts @@ -8,6 +8,7 @@ import type { Path } from '../jsutils/Path.js'; import { addPath, pathToArray } from '../jsutils/Path.js'; import { suggestionList } from '../jsutils/suggestionList.js'; +import { ensureGraphQLError } from '../error/ensureGraphQLError.js'; import { GraphQLError } from '../error/GraphQLError.js'; import type { ASTNode, ValueNode, VariableNode } from '../language/ast.js'; @@ -183,7 +184,7 @@ function validateInputValueImpl( assertLeafType(type); let result; - let caughtError; + let caughtError: unknown; try { result = type.coerceInputValue(inputValue, hideSuggestions); @@ -200,11 +201,11 @@ function validateInputValueImpl( onError, `Expected value of type "${type}"${ caughtError != null - ? `, but encountered error "${caughtError.message != null && caughtError.message !== '' ? caughtError.message : caughtError}"; found` + ? `, but encountered error "${getCaughtErrorMessage(caughtError)}"; found` : ', found' }: ${inspect(inputValue)}.`, path, - caughtError, + ensureGraphQLError(caughtError), ); } } @@ -454,7 +455,7 @@ function validateInputLiteralImpl( assertLeafType(type); let result; - let caughtError; + let caughtError: unknown; try { result = type.coerceInputLiteral ? type.coerceInputLiteral( @@ -479,12 +480,12 @@ function validateInputLiteralImpl( context.onError, `Expected value of type "${type}"${ caughtError != null - ? `, but encountered error "${caughtError.message != null && caughtError.message !== '' ? caughtError.message : caughtError}"; found` + ? `, but encountered error "${getCaughtErrorMessage(caughtError)}"; found` : ', found' }: ${print(valueNode)}.`, valueNode, path, - caughtError, + ensureGraphQLError(caughtError), ); } } @@ -509,7 +510,21 @@ function reportInvalidLiteral( originalError?: GraphQLError, ): void { onError( - new GraphQLError(message, { nodes: valueNode, originalError }), + new GraphQLError(message, { + nodes: valueNode, + originalError, + }), pathToArray(path), ); } + +function getCaughtErrorMessage(caughtError: unknown): string { + if (isObjectLike(caughtError)) { + const message = caughtError.message; + if (typeof message === 'string' && message !== '') { + return message; + } + } + + return String(caughtError); +} diff --git a/tsconfig.json b/tsconfig.json index 9d220dc2d3..02b1deddd8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -19,7 +19,6 @@ // Type Checking // https://www.typescriptlang.org/tsconfig#Type_Checking_6248 "strict": true, - "useUnknownInCatchVariables": false, // FIXME part of 'strict' but is temporary disabled // All checks that are not part of "strict" "allowUnreachableCode": false, "allowUnusedLabels": false,