polish: enable useUnknownInCatchVariables (#4619)

yaacovCR · web-flow · commit dda244585552 · 2026-03-10T15:32:38.000+02:00
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)] }),
     );
   }
 
diff --git 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
@@ -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,

Original file line number	Diff line number	Diff line change
`@@ -286,7 +286,9 @@ function runBenchmark(`
`286`	`286`	`results.push(computeStats(revision, samples));`
`287`	`287`	`process.stdout.write(' ' + cyan(i + 1) + ' tests completed.\u000D');`
`288`	`288`	`} catch (error) {`
`289`		`- console.log(' ' + revision + ': ' + red(error.message));`
	`289`	`+ const errorMessage =`
	`290`	`+ error instanceof Error ? error.message : String(error);`
	`291`	`+ console.log(' ' + revision + ': ' + red(errorMessage));`
`290`	`292`	`}`
`291`	`293`	`}`
`292`	`294`	`console.log('\n');`
Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@ import type { Path } from '../jsutils/Path.js';`
`8`	`8`	`import { addPath, pathToArray } from '../jsutils/Path.js';`
`9`	`9`	`import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js';`
`10`	`10`
	`11`	`+import { ensureGraphQLError } from '../error/ensureGraphQLError.js';`
`11`	`12`	`import { GraphQLError } from '../error/GraphQLError.js';`
`12`	`13`	`import { locatedError } from '../error/locatedError.js';`
`13`	`14`
`@@ -529,13 +530,13 @@ function createSourceEventStreamImpl(`
`529`	`530`	`const eventStream = executeSubscription(validatedExecutionArgs);`
`530`	`531`	`if (isPromise(eventStream)) {`
`531`	`532`	`return eventStream.then(undefined, (error: unknown) => ({`
`532`		`- errors: [error as GraphQLError],`
	`533`	`+ errors: [ensureGraphQLError(error)],`
`533`	`534`	`}));`
`534`	`535`	`}`
`535`	`536`
`536`	`537`	`return eventStream;`
`537`	`538`	`} catch (error) {`
`538`		`- return { errors: [error] };`
	`539`	`+ return { errors: [ensureGraphQLError(error)] };`
`539`	`540`	`}`
`540`	`541`	`}`
`541`	`542`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@ import type { Maybe } from '../jsutils/Maybe.js';`
`3`	`3`	`import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap.js';`
`4`	`4`	`import { printPathArray } from '../jsutils/printPathArray.js';`
`5`	`5`
	`6`	`+import { ensureGraphQLError } from '../error/ensureGraphQLError.js';`
`6`	`7`	`import { GraphQLError } from '../error/GraphQLError.js';`
`7`	`8`
`8`	`9`	`import type {`
`@@ -89,7 +90,7 @@ export function getVariableValues(`
`89`	`90`	`return { variableValues };`
`90`	`91`	`}`
`91`	`92`	`} catch (error) {`
`92`		`- errors.push(error);`
	`93`	`+ errors.push(ensureGraphQLError(error));`
`93`	`94`	`}`
`94`	`95`
`95`	`96`	`return { errors };`