Errors: fix how error messages represent arrays (#1333)

* Errors: fix how error messages represent arrays

* Add test for invalid return from 'resolveType'

Author: IvanGoncharov

src/execution/__tests__/abstract-test.js

@@ -377,6 +377,50 @@ describe('Execute: Handles execution of abstract types', () => {
     });
   });
 
+  it('returning invalid value from resolveType yields useful error', () => {
+    const fooInterface = new GraphQLInterfaceType({
+      name: 'FooInterface',
+      fields: { bar: { type: GraphQLString } },
+      resolveType: () => [],
+    });
+
+    const fooObject = new GraphQLObjectType({
+      name: 'FooObject',
+      fields: { bar: { type: GraphQLString } },
+      interfaces: [fooInterface],
+    });
+
+    const schema = new GraphQLSchema({
+      query: new GraphQLObjectType({
+        name: 'Query',
+        fields: {
+          foo: {
+            type: fooInterface,
+            resolve: () => 'dummy',
+          },
+        },
+      }),
+      types: [fooObject],
+    });
+
+    const result = graphqlSync(schema, '{ foo { bar } }');
+
+    expect(result).to.deep.equal({
+      data: { foo: null },
+      errors: [
+        {
+          message:
+            'Abstract type FooInterface must resolve to an Object type at ' +
+            'runtime for field Query.foo with value "dummy", received "[]". ' +
+            'Either the FooInterface type should provide a "resolveType" ' +
+            'function or each possible types should provide an "isTypeOf" function.',
+          locations: [{ line: 1, column: 3 }],
+          path: ['foo'],
+        },
+      ],
+    });
+  });
+
   it('resolveType allows resolving with type name', () => {
     const PetType = new GraphQLInterfaceType({
       name: 'Pet',

src/execution/execute.js

@@ -9,6 +9,7 @@
 
 import { forEach, isCollection } from 'iterall';
 import { GraphQLError, locatedError } from '../error';
+import inspect from '../jsutils/inspect';
 import invariant from '../jsutils/invariant';
 import isInvalid from '../jsutils/isInvalid';
 import isNullish from '../jsutils/isNullish';
@@ -907,7 +908,7 @@ function completeValue(
   // Not reachable. All possible output types have been considered.
   /* istanbul ignore next */
   throw new Error(
-    `Cannot complete value of unexpected type "${String(
+    `Cannot complete value of unexpected type "${inspect(
       (returnType: empty),
     )}".`,
   );
@@ -968,8 +969,8 @@ function completeLeafValue(returnType: GraphQLLeafType, result: mixed): mixed {
   const serializedResult = returnType.serialize(result);
   if (isInvalid(serializedResult)) {
     throw new Error(
-      `Expected a value of type "${String(returnType)}" but ` +
-        `received: ${String(result)}`,
+      `Expected a value of type "${inspect(returnType)}" but ` +
+        `received: ${inspect(result)}`,
     );
   }
   return serializedResult;
@@ -1045,7 +1046,7 @@ function ensureValidRuntimeType(
     throw new GraphQLError(
       `Abstract type ${returnType.name} must resolve to an Object type at ` +
         `runtime for field ${info.parentType.name}.${info.fieldName} with ` +
-        `value "${String(result)}", received "${String(runtimeType)}". ` +
+        `value "${inspect(result)}", received "${inspect(runtimeType)}". ` +
         `Either the ${returnType.name} type should provide a "resolveType" ` +
         'function or each possible types should provide an ' +
         '"isTypeOf" function.',
@@ -1116,7 +1117,7 @@ function invalidReturnTypeError(
   fieldNodes: $ReadOnlyArray<FieldNode>,
 ): GraphQLError {
   return new GraphQLError(
-    `Expected value of type "${returnType.name}" but got: ${String(result)}.`,
+    `Expected value of type "${returnType.name}" but got: ${inspect(result)}.`,
     fieldNodes,
   );
 }

src/execution/values.js

@@ -9,6 +9,7 @@
 
 import { GraphQLError } from '../error';
 import find from '../jsutils/find';
+import inspect from '../jsutils/inspect';
 import invariant from '../jsutils/invariant';
 import keyMap from '../jsutils/keyMap';
 import { coerceValue } from '../utilities/coerceValue';
@@ -78,9 +79,9 @@ export function getVariableValues(
           new GraphQLError(
             hasValue
               ? `Variable "$${varName}" of non-null type ` +
-                `"${String(varType)}" must not be null.`
+                `"${inspect(varType)}" must not be null.`
               : `Variable "$${varName}" of required type ` +
-                `"${String(varType)}" was not provided.`,
+                `"${inspect(varType)}" was not provided.`,
             [varDefNode],
           ),
         );
@@ -158,21 +159,21 @@ export function getArgumentValues(
       // non-null type (required), produce a field error.
       if (isNull) {
         throw new GraphQLError(
-          `Argument "${name}" of non-null type "${String(argType)}" ` +
+          `Argument "${name}" of non-null type "${inspect(argType)}" ` +
             'must not be null.',
           [argumentNode.value],
         );
       } else if (argumentNode && argumentNode.value.kind === Kind.VARIABLE) {
         const variableName = argumentNode.value.name.value;
         throw new GraphQLError(
-          `Argument "${name}" of required type "${String(argType)}" ` +
+          `Argument "${name}" of required type "${inspect(argType)}" ` +
             `was provided the variable "$${variableName}" ` +
             'which was not provided a runtime value.',
           [argumentNode.value],
         );
       } else {
         throw new GraphQLError(
-          `Argument "${name}" of required type "${String(argType)}" ` +
+          `Argument "${name}" of required type "${inspect(argType)}" ` +
             'was not provided.',
           [node],
         );

src/jsutils/inspect.js

@@ -0,0 +1,15 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow strict
+ */
+
+export default function inspect(value: mixed): string {
+  if (Array.isArray(value)) {
+    return '[' + String(value) + ']';
+  }
+  return String(value);
+}

src/language/parser.js

@@ -7,6 +7,7 @@
  * @flow strict
  */
 
+import inspect from '../jsutils/inspect';
 import { Source } from './source';
 import { syntaxError } from '../error';
 import type { GraphQLError } from '../error';
@@ -126,7 +127,7 @@ export function parse(
 ): DocumentNode {
   const sourceObj = typeof source === 'string' ? new Source(source) : source;
   if (!(sourceObj instanceof Source)) {
-    throw new TypeError('Must provide Source. Received: ' + String(sourceObj));
+    throw new TypeError(`Must provide Source. Received: ${inspect(sourceObj)}`);
   }
   const lexer = createLexer(sourceObj, options || {});
   return parseDocument(lexer);

src/subscription/subscribe.js

@@ -8,6 +8,7 @@
  */
 
 import { isAsyncIterable } from 'iterall';
+import inspect from '../jsutils/inspect';
 import { GraphQLError } from '../error/GraphQLError';
 import { locatedError } from '../error/locatedError';
 import {
@@ -279,7 +280,7 @@ export function createSourceEventStream(
       }
       throw new Error(
         'Subscription field must return Async Iterable. Received: ' +
-          String(eventStream),
+          inspect(eventStream),
       );
     });
   } catch (error) {

src/type/definition.js

@@ -8,6 +8,7 @@
  */
 
 import instanceOf from '../jsutils/instanceOf';
+import inspect from '../jsutils/inspect';
 import invariant from '../jsutils/invariant';
 import isInvalid from '../jsutils/isInvalid';
 import keyMap from '../jsutils/keyMap';
@@ -63,7 +64,7 @@ export function isType(type: mixed): boolean %checks {
 }
 
 export function assertType(type: mixed): GraphQLType {
-  invariant(isType(type), `Expected ${String(type)} to be a GraphQL type.`);
+  invariant(isType(type), `Expected ${inspect(type)} to be a GraphQL type.`);
   return (type: any);
 }
 
@@ -81,7 +82,7 @@ export function isScalarType(type) {
 export function assertScalarType(type: mixed): GraphQLScalarType {
   invariant(
     isScalarType(type),
-    `Expected ${String(type)} to be a GraphQL Scalar type.`,
+    `Expected ${inspect(type)} to be a GraphQL Scalar type.`,
   );
   return type;
 }
@@ -96,7 +97,7 @@ export function isObjectType(type) {
 export function assertObjectType(type: mixed): GraphQLObjectType {
   invariant(
     isObjectType(type),
-    `Expected ${String(type)} to be a GraphQL Object type.`,
+    `Expected ${inspect(type)} to be a GraphQL Object type.`,
   );
   return type;
 }
@@ -111,7 +112,7 @@ export function isInterfaceType(type) {
 export function assertInterfaceType(type: mixed): GraphQLInterfaceType {
   invariant(
     isInterfaceType(type),
-    `Expected ${String(type)} to be a GraphQL Interface type.`,
+    `Expected ${inspect(type)} to be a GraphQL Interface type.`,
   );
   return type;
 }
@@ -126,7 +127,7 @@ export function isUnionType(type) {
 export function assertUnionType(type: mixed): GraphQLUnionType {
   invariant(
     isUnionType(type),
-    `Expected ${String(type)} to be a GraphQL Union type.`,
+    `Expected ${inspect(type)} to be a GraphQL Union type.`,
   );
   return type;
 }
@@ -141,7 +142,7 @@ export function isEnumType(type) {
 export function assertEnumType(type: mixed): GraphQLEnumType {
   invariant(
     isEnumType(type),
-    `Expected ${String(type)} to be a GraphQL Enum type.`,
+    `Expected ${inspect(type)} to be a GraphQL Enum type.`,
   );
   return type;
 }
@@ -156,7 +157,7 @@ export function isInputObjectType(type) {
 export function assertInputObjectType(type: mixed): GraphQLInputObjectType {
   invariant(
     isInputObjectType(type),
-    `Expected ${String(type)} to be a GraphQL Input Object type.`,
+    `Expected ${inspect(type)} to be a GraphQL Input Object type.`,
   );
   return type;
 }
@@ -171,7 +172,7 @@ export function isListType(type) {
 export function assertListType(type: mixed): GraphQLList<any> {
   invariant(
     isListType(type),
-    `Expected ${String(type)} to be a GraphQL List type.`,
+    `Expected ${inspect(type)} to be a GraphQL List type.`,
   );
   return type;
 }
@@ -186,7 +187,7 @@ export function isNonNullType(type) {
 export function assertNonNullType(type: mixed): GraphQLNonNull<any> {
   invariant(
     isNonNullType(type),
-    `Expected ${String(type)} to be a GraphQL Non-Null type.`,
+    `Expected ${inspect(type)} to be a GraphQL Non-Null type.`,
   );
   return type;
 }
@@ -218,7 +219,7 @@ export function isInputType(type: mixed): boolean %checks {
 export function assertInputType(type: mixed): GraphQLInputType {
   invariant(
     isInputType(type),
-    `Expected ${String(type)} to be a GraphQL input type.`,
+    `Expected ${inspect(type)} to be a GraphQL input type.`,
   );
   return type;
 }
@@ -256,7 +257,7 @@ export function isOutputType(type: mixed): boolean %checks {
 export function assertOutputType(type: mixed): GraphQLOutputType {
   invariant(
     isOutputType(type),
-    `Expected ${String(type)} to be a GraphQL output type.`,
+    `Expected ${inspect(type)} to be a GraphQL output type.`,
   );
   return type;
 }
@@ -273,7 +274,7 @@ export function isLeafType(type: mixed): boolean %checks {
 export function assertLeafType(type: mixed): GraphQLLeafType {
   invariant(
     isLeafType(type),
-    `Expected ${String(type)} to be a GraphQL leaf type.`,
+    `Expected ${inspect(type)} to be a GraphQL leaf type.`,
   );
   return type;
 }
@@ -293,7 +294,7 @@ export function isCompositeType(type: mixed): boolean %checks {
 export function assertCompositeType(type: mixed): GraphQLCompositeType {
   invariant(
     isCompositeType(type),
-    `Expected ${String(type)} to be a GraphQL composite type.`,
+    `Expected ${inspect(type)} to be a GraphQL composite type.`,
   );
   return type;
 }
@@ -310,7 +311,7 @@ export function isAbstractType(type: mixed): boolean %checks {
 export function assertAbstractType(type: mixed): GraphQLAbstractType {
   invariant(
     isAbstractType(type),
-    `Expected ${String(type)} to be a GraphQL abstract type.`,
+    `Expected ${inspect(type)} to be a GraphQL abstract type.`,
   );
   return type;
 }
@@ -408,7 +409,7 @@ export function isWrappingType(type: mixed): boolean %checks {
 export function assertWrappingType(type: mixed): GraphQLWrappingType {
   invariant(
     isWrappingType(type),
-    `Expected ${String(type)} to be a GraphQL wrapping type.`,
+    `Expected ${inspect(type)} to be a GraphQL wrapping type.`,
   );
   return type;
 }
@@ -432,7 +433,7 @@ export function isNullableType(type: mixed): boolean %checks {
 export function assertNullableType(type: mixed): GraphQLNullableType {
   invariant(
     isNullableType(type),
-    `Expected ${String(type)} to be a GraphQL nullable type.`,
+    `Expected ${inspect(type)} to be a GraphQL nullable type.`,
   );
   return type;
 }
@@ -473,7 +474,7 @@ export function isNamedType(type: mixed): boolean %checks {
 export function assertNamedType(type: mixed): GraphQLNamedType {
   invariant(
     isNamedType(type),
-    `Expected ${String(type)} to be a GraphQL named type.`,
+    `Expected ${inspect(type)} to be a GraphQL named type.`,
   );
   return type;
 }
@@ -736,7 +737,7 @@ function defineFieldMap<TSource, TContext>(
     invariant(
       isValidResolver(field.resolve),
       `${type.name}.${fieldName} field resolver must be a function if ` +
-        `provided, but got: ${String(field.resolve)}.`,
+        `provided, but got: ${inspect(field.resolve)}.`,
     );
     const argsConfig = fieldConfig.args;
     if (!argsConfig) {
@@ -1148,7 +1149,7 @@ function defineEnumValues(
     invariant(
       isPlainObj(value),
       `${type.name}.${valueName} must refer to an object with a "value" key ` +
-        `representing an internal value but got: ${String(value)}.`,
+        `representing an internal value but got: ${inspect(value)}.`,
     );
     invariant(
       !value.hasOwnProperty('isDeprecated'),