Skip to content

Errors: fix how error messages represent arrays #1333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions src/execution/__tests__/abstract-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
11 changes: 6 additions & 5 deletions src/execution/execute.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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),
)}".`,
);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.',
Expand Down Expand Up @@ -1156,7 +1157,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,
);
}
Expand Down
11 changes: 6 additions & 5 deletions src/execution/values.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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],
),
);
Expand Down Expand Up @@ -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],
);
Expand Down
15 changes: 15 additions & 0 deletions src/jsutils/inspect.js
Original file line number Diff line number Diff line change
@@ -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);
}
3 changes: 2 additions & 1 deletion src/language/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* @flow strict
*/

import inspect from '../jsutils/inspect';
import { Source } from './source';
import { syntaxError } from '../error';
import type { GraphQLError } from '../error';
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/subscription/subscribe.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import { isAsyncIterable } from 'iterall';
import inspect from '../jsutils/inspect';
import { GraphQLError } from '../error/GraphQLError';
import { locatedError } from '../error/locatedError';
import {
Expand Down Expand Up @@ -279,7 +280,7 @@ export function createSourceEventStream(
}
throw new Error(
'Subscription field must return Async Iterable. Received: ' +
String(eventStream),
inspect(eventStream),
);
});
} catch (error) {
Expand Down
39 changes: 20 additions & 19 deletions src/type/definition.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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'),
Expand Down
Loading