From 0151e2949f091abd4fe6884cbd541e57b4fcd43c Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Wed, 2 May 2018 15:49:54 +0300 Subject: [PATCH 1/2] Errors: fix how error messages represent arrays --- src/execution/execute.js | 11 +++--- src/execution/values.js | 11 +++--- src/jsutils/inspect.js | 15 +++++++ src/language/parser.js | 3 +- src/subscription/subscribe.js | 3 +- src/type/definition.js | 39 ++++++++++--------- src/type/scalars.js | 9 +++-- src/type/schema.js | 7 ++-- src/type/validate.js | 39 ++++++++++--------- src/utilities/astFromValue.js | 3 +- src/utilities/coerceValue.js | 5 ++- .../rules/FragmentsOnCompositeTypes.js | 5 ++- .../rules/OverlappingFieldsCanBeMerged.js | 3 +- .../rules/PossibleFragmentSpreads.js | 5 ++- .../rules/ProvidedRequiredArguments.js | 5 ++- src/validation/rules/ScalarLeafs.js | 5 ++- src/validation/rules/ValuesOfCorrectType.js | 11 +++--- .../rules/VariablesInAllowedPosition.js | 5 ++- 18 files changed, 109 insertions(+), 75 deletions(-) create mode 100644 src/jsutils/inspect.js diff --git a/src/execution/execute.js b/src/execution/execute.js index 6ec28d5ea2..207877e155 100644 --- a/src/execution/execute.js +++ b/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'; @@ -947,7 +948,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), )}".`, ); @@ -1008,8 +1009,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; @@ -1085,7 +1086,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.', @@ -1156,7 +1157,7 @@ function invalidReturnTypeError( fieldNodes: $ReadOnlyArray, ): 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, ); } diff --git a/src/execution/values.js b/src/execution/values.js index 5690bbe366..5df2440678 100644 --- a/src/execution/values.js +++ b/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], ); diff --git a/src/jsutils/inspect.js b/src/jsutils/inspect.js new file mode 100644 index 0000000000..70bc6bd21d --- /dev/null +++ b/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); +} diff --git a/src/language/parser.js b/src/language/parser.js index 6fe84fe9da..2876a7e639 100644 --- a/src/language/parser.js +++ b/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); diff --git a/src/subscription/subscribe.js b/src/subscription/subscribe.js index a7fc1f69a8..547c5d4fc9 100644 --- a/src/subscription/subscribe.js +++ b/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) { diff --git a/src/type/definition.js b/src/type/definition.js index 2d7025a470..605d7ebac7 100644 --- a/src/type/definition.js +++ b/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 { 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 { 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( 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'), diff --git a/src/type/scalars.js b/src/type/scalars.js index c60bcc8cdc..50c7b13386 100644 --- a/src/type/scalars.js +++ b/src/type/scalars.js @@ -7,6 +7,7 @@ * @flow strict */ +import inspect from '../jsutils/inspect'; import { GraphQLScalarType, isNamedType } from './definition'; import { Kind } from '../language/kinds'; @@ -27,13 +28,13 @@ function coerceInt(value: mixed): ?number { const num = Number(value); if (num !== num || num > MAX_INT || num < MIN_INT) { throw new TypeError( - 'Int cannot represent non 32-bit signed integer value: ' + String(value), + 'Int cannot represent non 32-bit signed integer value: ' + inspect(value), ); } const int = Math.floor(num); if (int !== num) { throw new TypeError( - 'Int cannot represent non-integer value: ' + String(value), + 'Int cannot represent non-integer value: ' + inspect(value), ); } return int; @@ -68,7 +69,7 @@ function coerceFloat(value: mixed): ?number { return num; } throw new TypeError( - 'Float cannot represent non numeric value: ' + String(value), + 'Float cannot represent non numeric value: ' + inspect(value), ); } @@ -90,7 +91,7 @@ export const GraphQLFloat = new GraphQLScalarType({ function coerceString(value: mixed): ?string { if (Array.isArray(value)) { throw new TypeError( - `String cannot represent an array value: [${String(value)}]`, + `String cannot represent an array value: ${inspect(value)}`, ); } return String(value); diff --git a/src/type/schema.js b/src/type/schema.js index 96ac0167b3..d7f1fa1c93 100644 --- a/src/type/schema.js +++ b/src/type/schema.js @@ -31,6 +31,7 @@ import { specifiedDirectives, } from './directives'; import type { GraphQLError } from '../error/GraphQLError'; +import inspect from '../jsutils/inspect'; import { __Schema } from './introspection'; import find from '../jsutils/find'; import instanceOf from '../jsutils/instanceOf'; @@ -103,17 +104,17 @@ export class GraphQLSchema { ); invariant( !config.types || Array.isArray(config.types), - `"types" must be Array if provided but got: ${String(config.types)}.`, + `"types" must be Array if provided but got: ${inspect(config.types)}.`, ); invariant( !config.directives || Array.isArray(config.directives), '"directives" must be Array if provided but got: ' + - `${String(config.directives)}.`, + `${inspect(config.directives)}.`, ); invariant( !config.allowedLegacyNames || Array.isArray(config.allowedLegacyNames), '"allowedLegacyNames" must be Array if provided but got: ' + - `${String(config.allowedLegacyNames)}.`, + `${inspect(config.allowedLegacyNames)}.`, ); } diff --git a/src/type/validate.js b/src/type/validate.js index 830418d122..de13bf1adb 100644 --- a/src/type/validate.js +++ b/src/type/validate.js @@ -30,6 +30,7 @@ import type { GraphQLDirective } from './directives'; import { isIntrospectionType } from './introspection'; import { isSchema } from './schema'; import type { GraphQLSchema } from './schema'; +import inspect from '../jsutils/inspect'; import find from '../jsutils/find'; import invariant from '../jsutils/invariant'; import objectValues from '../jsutils/objectValues'; @@ -58,7 +59,7 @@ export function validateSchema( // First check to ensure the provided value is in fact a GraphQLSchema. invariant( isSchema(schema), - `Expected ${String(schema)} to be a GraphQL schema.`, + `Expected ${inspect(schema)} to be a GraphQL schema.`, ); // If this Schema has already been validated, return the previous results. @@ -123,7 +124,9 @@ function validateRootTypes(context) { context.reportError(`Query root type must be provided.`, schema.astNode); } else if (!isObjectType(queryType)) { context.reportError( - `Query root type must be Object type, it cannot be ${String(queryType)}.`, + `Query root type must be Object type, it cannot be ${inspect( + queryType, + )}.`, getOperationTypeNode(schema, queryType, 'query'), ); } @@ -132,7 +135,7 @@ function validateRootTypes(context) { if (mutationType && !isObjectType(mutationType)) { context.reportError( 'Mutation root type must be Object type if provided, it cannot be ' + - `${String(mutationType)}.`, + `${inspect(mutationType)}.`, getOperationTypeNode(schema, mutationType, 'mutation'), ); } @@ -141,7 +144,7 @@ function validateRootTypes(context) { if (subscriptionType && !isObjectType(subscriptionType)) { context.reportError( 'Subscription root type must be Object type if provided, it cannot be ' + - `${String(subscriptionType)}.`, + `${inspect(subscriptionType)}.`, getOperationTypeNode(schema, subscriptionType, 'subscription'), ); } @@ -171,7 +174,7 @@ function validateDirectives(context: SchemaValidationContext): void { // Ensure all directives are in fact GraphQL directives. if (!isDirective(directive)) { context.reportError( - `Expected directive but got: ${String(directive)}.`, + `Expected directive but got: ${inspect(directive)}.`, directive && directive.astNode, ); return; @@ -204,7 +207,7 @@ function validateDirectives(context: SchemaValidationContext): void { if (!isInputType(arg.type)) { context.reportError( `The type of @${directive.name}(${argName}:) must be Input Type ` + - `but got: ${String(arg.type)}.`, + `but got: ${inspect(arg.type)}.`, getDirectiveArgTypeNode(directive, argName), ); } @@ -237,7 +240,7 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure all provided types are in fact GraphQL type. if (!isNamedType(type)) { context.reportError( - `Expected GraphQL named type but got: ${String(type)}.`, + `Expected GraphQL named type but got: ${inspect(type)}.`, type && type.astNode, ); return; @@ -305,7 +308,7 @@ function validateFields( if (!isOutputType(field.type)) { context.reportError( `The type of ${type.name}.${field.name} must be Output Type ` + - `but got: ${String(field.type)}.`, + `but got: ${inspect(field.type)}.`, getFieldTypeNode(type, field.name), ); } @@ -332,7 +335,7 @@ function validateFields( if (!isInputType(arg.type)) { context.reportError( `The type of ${type.name}.${field.name}(${argName}:) must be Input ` + - `Type but got: ${String(arg.type)}.`, + `Type but got: ${inspect(arg.type)}.`, getFieldArgTypeNode(type, field.name, argName), ); } @@ -348,8 +351,8 @@ function validateObjectInterfaces( object.getInterfaces().forEach(iface => { if (!isInterfaceType(iface)) { context.reportError( - `Type ${String(object)} must only implement Interface types, ` + - `it cannot implement ${String(iface)}.`, + `Type ${inspect(object)} must only implement Interface types, ` + + `it cannot implement ${inspect(iface)}.`, getImplementsInterfaceNode(object, iface), ); return; @@ -411,8 +414,8 @@ function validateObjectImplementsInterface( if (!isTypeSubTypeOf(context.schema, objectField.type, ifaceField.type)) { context.reportError( `Interface field ${iface.name}.${fieldName} expects type ` + - `${String(ifaceField.type)} but ${object.name}.${fieldName} ` + - `is type ${String(objectField.type)}.`, + `${inspect(ifaceField.type)} but ${object.name}.${fieldName} ` + + `is type ${inspect(objectField.type)}.`, [ getFieldTypeNode(iface, fieldName), getFieldTypeNode(object, fieldName), @@ -445,9 +448,9 @@ function validateObjectImplementsInterface( if (!isEqualType(ifaceArg.type, objectArg.type)) { context.reportError( `Interface field argument ${iface.name}.${fieldName}(${argName}:) ` + - `expects type ${String(ifaceArg.type)} but ` + + `expects type ${inspect(ifaceArg.type)} but ` + `${object.name}.${fieldName}(${argName}:) is type ` + - `${String(objectArg.type)}.`, + `${inspect(objectArg.type)}.`, [ getFieldArgTypeNode(iface, fieldName, argName), getFieldArgTypeNode(object, fieldName, argName), @@ -465,7 +468,7 @@ function validateObjectImplementsInterface( if (!ifaceArg && isNonNullType(objectArg.type)) { context.reportError( `Object field argument ${object.name}.${fieldName}(${argName}:) ` + - `is of required type ${String(objectArg.type)} but is not also ` + + `is of required type ${inspect(objectArg.type)} but is not also ` + `provided by the Interface field ${iface.name}.${fieldName}.`, [ getFieldArgTypeNode(object, fieldName, argName), @@ -504,7 +507,7 @@ function validateUnionMembers( if (!isObjectType(memberType)) { context.reportError( `Union type ${union.name} can only include Object types, ` + - `it cannot include ${String(memberType)}.`, + `it cannot include ${inspect(memberType)}.`, getUnionMemberTypeNodes(union, String(memberType)), ); } @@ -571,7 +574,7 @@ function validateInputFields( if (!isInputType(field.type)) { context.reportError( `The type of ${inputObj.name}.${field.name} must be Input Type ` + - `but got: ${String(field.type)}.`, + `but got: ${inspect(field.type)}.`, field.astNode && field.astNode.type, ); } diff --git a/src/utilities/astFromValue.js b/src/utilities/astFromValue.js index 988430d61f..883eb79221 100644 --- a/src/utilities/astFromValue.js +++ b/src/utilities/astFromValue.js @@ -9,6 +9,7 @@ import { forEach, isCollection } from 'iterall'; +import inspect from '../jsutils/inspect'; import isNullish from '../jsutils/isNullish'; import isInvalid from '../jsutils/isInvalid'; import objectValues from '../jsutils/objectValues'; @@ -136,7 +137,7 @@ export function astFromValue(value: mixed, type: GraphQLInputType): ?ValueNode { }; } - throw new TypeError('Cannot convert value to AST: ' + String(serialized)); + throw new TypeError(`Cannot convert value to AST: ${inspect(serialized)}`); } /* istanbul ignore next */ diff --git a/src/utilities/coerceValue.js b/src/utilities/coerceValue.js index 38a42d635c..183d87054a 100644 --- a/src/utilities/coerceValue.js +++ b/src/utilities/coerceValue.js @@ -8,6 +8,7 @@ */ import { forEach, isCollection } from 'iterall'; +import inspect from '../jsutils/inspect'; import isInvalid from '../jsutils/isInvalid'; import isNullish from '../jsutils/isNullish'; import orList from '../jsutils/orList'; @@ -48,7 +49,7 @@ export function coerceValue( if (isNullish(value)) { return ofErrors([ coercionError( - `Expected non-nullable type ${String(type)} not to be null`, + `Expected non-nullable type ${inspect(type)} not to be null`, blameNode, path, ), @@ -159,7 +160,7 @@ export function coerceValue( errors, coercionError( `Field ${printPath(atPath(path, fieldName))} of required ` + - `type ${String(field.type)} was not provided`, + `type ${inspect(field.type)} was not provided`, blameNode, ), ); diff --git a/src/validation/rules/FragmentsOnCompositeTypes.js b/src/validation/rules/FragmentsOnCompositeTypes.js index ea03734d85..9d285348ef 100644 --- a/src/validation/rules/FragmentsOnCompositeTypes.js +++ b/src/validation/rules/FragmentsOnCompositeTypes.js @@ -7,6 +7,7 @@ * @flow strict */ +import inspect from '../../jsutils/inspect'; import type ValidationContext from '../ValidationContext'; import { GraphQLError } from '../../error'; import { print } from '../../language/printer'; @@ -18,7 +19,7 @@ import { typeFromAST } from '../../utilities/typeFromAST'; export function inlineFragmentOnNonCompositeErrorMessage( type: GraphQLType, ): string { - return `Fragment cannot condition on non composite type "${String(type)}".`; + return `Fragment cannot condition on non composite type "${inspect(type)}".`; } export function fragmentOnNonCompositeErrorMessage( @@ -27,7 +28,7 @@ export function fragmentOnNonCompositeErrorMessage( ): string { return ( `Fragment "${fragName}" cannot condition on non composite ` + - `type "${String(type)}".` + `type "${inspect(type)}".` ); } diff --git a/src/validation/rules/OverlappingFieldsCanBeMerged.js b/src/validation/rules/OverlappingFieldsCanBeMerged.js index 6dea9df741..e3b5ca359d 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMerged.js +++ b/src/validation/rules/OverlappingFieldsCanBeMerged.js @@ -9,6 +9,7 @@ import type ValidationContext from '../ValidationContext'; import { GraphQLError } from '../../error'; +import inspect from '../../jsutils/inspect'; import find from '../../jsutils/find'; import type { ObjMap } from '../../jsutils/ObjMap'; import type { @@ -612,7 +613,7 @@ function findConflict( return [ [ responseName, - `they return conflicting types ${String(type1)} and ${String(type2)}`, + `they return conflicting types ${inspect(type1)} and ${inspect(type2)}`, ], [node1], [node2], diff --git a/src/validation/rules/PossibleFragmentSpreads.js b/src/validation/rules/PossibleFragmentSpreads.js index 31bca238d8..95ee3212a9 100644 --- a/src/validation/rules/PossibleFragmentSpreads.js +++ b/src/validation/rules/PossibleFragmentSpreads.js @@ -7,6 +7,7 @@ * @flow strict */ +import inspect from '../../jsutils/inspect'; import type ValidationContext from '../ValidationContext'; import { GraphQLError } from '../../error'; import type { ASTVisitor } from '../../language/visitor'; @@ -22,7 +23,7 @@ export function typeIncompatibleSpreadMessage( ): string { return ( `Fragment "${fragName}" cannot be spread here as objects of ` + - `type "${String(parentType)}" can never be of type "${String(fragType)}".` + `type "${inspect(parentType)}" can never be of type "${inspect(fragType)}".` ); } @@ -32,7 +33,7 @@ export function typeIncompatibleAnonSpreadMessage( ): string { return ( 'Fragment cannot be spread here as objects of ' + - `type "${String(parentType)}" can never be of type "${String(fragType)}".` + `type "${inspect(parentType)}" can never be of type "${inspect(fragType)}".` ); } diff --git a/src/validation/rules/ProvidedRequiredArguments.js b/src/validation/rules/ProvidedRequiredArguments.js index 4819e54592..b6b60a09bb 100644 --- a/src/validation/rules/ProvidedRequiredArguments.js +++ b/src/validation/rules/ProvidedRequiredArguments.js @@ -9,6 +9,7 @@ import type ValidationContext from '../ValidationContext'; import { GraphQLError } from '../../error'; +import inspect from '../../jsutils/inspect'; import keyMap from '../../jsutils/keyMap'; import { isNonNullType } from '../../type/definition'; import type { GraphQLType } from '../../type/definition'; @@ -21,7 +22,7 @@ export function missingFieldArgMessage( ): string { return ( `Field "${fieldName}" argument "${argName}" of type ` + - `"${String(type)}" is required but not provided.` + `"${inspect(type)}" is required but not provided.` ); } @@ -32,7 +33,7 @@ export function missingDirectiveArgMessage( ): string { return ( `Directive "@${directiveName}" argument "${argName}" of type ` + - `"${String(type)}" is required but not provided.` + `"${inspect(type)}" is required but not provided.` ); } diff --git a/src/validation/rules/ScalarLeafs.js b/src/validation/rules/ScalarLeafs.js index 8f2291c3c3..85f8f38fb1 100644 --- a/src/validation/rules/ScalarLeafs.js +++ b/src/validation/rules/ScalarLeafs.js @@ -7,6 +7,7 @@ * @flow strict */ +import inspect from '../../jsutils/inspect'; import type ValidationContext from '../ValidationContext'; import { GraphQLError } from '../../error'; import type { FieldNode } from '../../language/ast'; @@ -20,7 +21,7 @@ export function noSubselectionAllowedMessage( ): string { return ( `Field "${fieldName}" must not have a selection since ` + - `type "${String(type)}" has no subfields.` + `type "${inspect(type)}" has no subfields.` ); } @@ -29,7 +30,7 @@ export function requiredSubselectionMessage( type: GraphQLType, ): string { return ( - `Field "${fieldName}" of type "${String(type)}" must have a ` + + `Field "${fieldName}" of type "${inspect(type)}" must have a ` + `selection of subfields. Did you mean "${fieldName} { ... }"?` ); } diff --git a/src/validation/rules/ValuesOfCorrectType.js b/src/validation/rules/ValuesOfCorrectType.js index d17a7a80c0..4b51563c95 100644 --- a/src/validation/rules/ValuesOfCorrectType.js +++ b/src/validation/rules/ValuesOfCorrectType.js @@ -22,6 +22,7 @@ import { getNamedType, } from '../../type/definition'; import type { GraphQLType } from '../../type/definition'; +import inspect from '../../jsutils/inspect'; import isInvalid from '../../jsutils/isInvalid'; import keyMap from '../../jsutils/keyMap'; import orList from '../../jsutils/orList'; @@ -72,7 +73,7 @@ export function ValuesOfCorrectType(context: ValidationContext): ASTVisitor { const type = context.getInputType(); if (isNonNullType(type)) { context.reportError( - new GraphQLError(badValueMessage(String(type), print(node)), node), + new GraphQLError(badValueMessage(inspect(type), print(node)), node), ); } }, @@ -105,7 +106,7 @@ export function ValuesOfCorrectType(context: ValidationContext): ASTVisitor { ) { context.reportError( new GraphQLError( - requiredFieldMessage(type.name, fieldName, String(fieldType)), + requiredFieldMessage(type.name, fieldName, inspect(fieldType)), node, ), ); @@ -173,7 +174,7 @@ function isValidScalar(context: ValidationContext, node: ValueNode): void { context.reportError( new GraphQLError( badValueMessage( - String(locationType), + inspect(locationType), print(node), enumTypeSuggestion(type, node), ), @@ -190,7 +191,7 @@ function isValidScalar(context: ValidationContext, node: ValueNode): void { if (isInvalid(parseResult)) { context.reportError( new GraphQLError( - badValueMessage(String(locationType), print(node)), + badValueMessage(inspect(locationType), print(node)), node, ), ); @@ -199,7 +200,7 @@ function isValidScalar(context: ValidationContext, node: ValueNode): void { // Ensure a reference to the original error is maintained. context.reportError( new GraphQLError( - badValueMessage(String(locationType), print(node), error.message), + badValueMessage(inspect(locationType), print(node), error.message), node, undefined, undefined, diff --git a/src/validation/rules/VariablesInAllowedPosition.js b/src/validation/rules/VariablesInAllowedPosition.js index 20805feb49..21c04d5570 100644 --- a/src/validation/rules/VariablesInAllowedPosition.js +++ b/src/validation/rules/VariablesInAllowedPosition.js @@ -7,6 +7,7 @@ * @flow strict */ +import inspect from '../../jsutils/inspect'; import type ValidationContext from '../ValidationContext'; import { GraphQLError } from '../../error'; import { Kind } from '../../language/kinds'; @@ -24,8 +25,8 @@ export function badVarPosMessage( expectedType: GraphQLType, ): string { return ( - `Variable "$${varName}" of type "${String(varType)}" used in ` + - `position expecting type "${String(expectedType)}".` + `Variable "$${varName}" of type "${inspect(varType)}" used in ` + + `position expecting type "${inspect(expectedType)}".` ); } From 6834ae76dce3c1311934f88e3b15751c54b69aa1 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Wed, 2 May 2018 17:58:39 +0300 Subject: [PATCH 2/2] Add test for invalid return from 'resolveType' --- src/execution/__tests__/abstract-test.js | 44 ++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/execution/__tests__/abstract-test.js b/src/execution/__tests__/abstract-test.js index 6c654b02bc..d835282e4a 100644 --- a/src/execution/__tests__/abstract-test.js +++ b/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',