polish: enable useUnknownInCatchVariables

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');

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'",
       );
     }

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 {

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',
+    });
+  });
+});

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 });
+}

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);
     }
   }

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)] };
   }
 }
 

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;

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;

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 };

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<ExecutionResult> {
   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)] }),
     );
   }
 

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);
+}

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,