Skip to content

Commit

Permalink
Improve error messages further
Browse files Browse the repository at this point in the history
  • Loading branch information
@freiksenet
freiksenet committed Nov 11, 2015
1 parent 2ada5c6 commit 98364f6
Show file tree
Hide file tree
Showing 7 changed files with 93 additions and 51 deletions.
35 changes: 18 additions & 17 deletions src/execution/__tests__/variables.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,9 @@ describe('Execute: Handles inputs', () => {
expect(caughtError).to.containSubset({
locations: [ { line: 2, column: 17 } ],
message:
'Variable "$input" expected value of type "TestInputObject" but ' +
'got: {"a":"foo","b":"bar","c":null}. ' +
'In field c: Expected non-null value.'
'Variable "$input" got invalid value ' +
'{"a":"foo","b":"bar","c":null}.' +
'\nIn field "c": Expected "String!", found null.'
});
});

Expand All @@ -267,8 +267,8 @@ describe('Execute: Handles inputs', () => {
expect(caughtError).to.containSubset({
locations: [ { line: 2, column: 17 } ],
message:
'Variable "$input" expected value of type "TestInputObject" but ' +
'got: "foo bar". Not an object.'
'Variable "$input" got invalid value "foo bar".' +
'\nExpected "TestInputObject", found not an object.'
});
});

Expand All @@ -285,8 +285,8 @@ describe('Execute: Handles inputs', () => {
expect(caughtError).to.containSubset({
locations: [ { line: 2, column: 17 } ],
message:
'Variable "$input" expected value of type "TestInputObject" but ' +
'got: {"a":"foo","b":"bar"}. In field c: Expected non-null value.'
'Variable "$input" got invalid value {"a":"foo","b":"bar"}.' +
'\nIn field "c": Expected "String!", found null.'
});
});

Expand All @@ -309,10 +309,9 @@ describe('Execute: Handles inputs', () => {
expect(caughtError).to.containSubset({
locations: [ { line: 2, column: 19 } ],
message:
'Variable "$input" expected value of type "TestNestedInputObject" but ' +
'got: {"na":{"a":"foo"}}.' +
' In field na: In field c: Expected non-null value.' +
' In field nb: Expected non-null value.'
'Variable "$input" got invalid value {"na":{"a":"foo"}}.' +
'\nIn field "na": In field "c": Expected "String!", found null.' +
'\nIn field "nb": Expected "String!", found null.'
});

});
Expand All @@ -330,8 +329,10 @@ describe('Execute: Handles inputs', () => {
expect(caughtError).to.containSubset({
locations: [ { line: 2, column: 17 } ],
message:
'Variable "$input" expected value of type "TestInputObject" but ' +
'got: {"a":"foo","b":"bar","c":"baz","d":"dog"}.'
'Variable "$input" got invalid value ' +
'{"a":"foo","b":"bar","c":"baz","d":"dog"}.' +
'\nIn field "d": Expected type "ComplexScalar", found "dog".'

});
});

Expand Down Expand Up @@ -687,8 +688,8 @@ describe('Execute: Handles inputs', () => {
expect(caughtError).to.containSubset({
locations: [ { line: 2, column: 17 } ],
message:
'Variable "$input" expected value of type "[String!]" but got: ' +
'["A",null,"B"]. In element #1: Expected non-null value.'
'Variable "$input" got invalid value ["A",null,"B"].' +
'\nIn element #1: Expected "String!", found null.'
});
});

Expand Down Expand Up @@ -750,8 +751,8 @@ describe('Execute: Handles inputs', () => {
expect(caughtError).to.containSubset({
locations: [ { line: 2, column: 17 } ],
message:
'Variable "$input" expected value of type "[String!]!" but got: ' +
'["A",null,"B"]. In element #1: Expected non-null value.'
'Variable "$input" got invalid value ["A",null,"B"].' +
'\nIn element #1: Expected "String!", found null.'
});
});

Expand Down
5 changes: 2 additions & 3 deletions src/execution/values.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,8 @@ function getVariableValue(
}
var message = errors ? '\n' + errors.join('\n') : '';
throw new GraphQLError(
`Variable "$${variable.name.value}" expected value of type ` +
`"${print(definitionAST.type)}" but got: ${JSON.stringify(input)}.` +
`${message}`,
`Variable "$${variable.name.value}" got invalid value ` +
`${JSON.stringify(input)}.${message}`,
[ definitionAST ]
);
}
Expand Down
19 changes: 13 additions & 6 deletions src/type/__tests__/enumType.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,10 @@ describe('Type System: Enum Values', () => {
await graphql(schema, '{ colorEnum(fromEnum: "GREEN") }')
).to.deep.equal({
errors: [
{ message:
'Argument "fromEnum" expected type "Color" but got: "GREEN".' }
{
message: 'Argument "fromEnum" has invalid value "GREEN".' +
'\nExpected type \"Color\", found "GREEN".'
}
]
});
});
Expand All @@ -128,7 +130,8 @@ describe('Type System: Enum Values', () => {
await graphql(schema, '{ colorEnum(fromEnum: 1) }')
).to.deep.equal({
errors: [
{ message: 'Argument "fromEnum" expected type "Color" but got: 1.' }
{ message: 'Argument "fromEnum" has invalid value 1.' +
'\nExpected type "Color", found 1.' }
]
});
});
Expand All @@ -138,7 +141,8 @@ describe('Type System: Enum Values', () => {
await graphql(schema, '{ colorEnum(fromInt: GREEN) }')
).to.deep.equal({
errors: [
{ message: 'Argument "fromInt" expected type "Int" but got: GREEN.' }
{ message: 'Argument "fromInt" has invalid value GREEN.' +
'\nExpected type "Int", found GREEN.' }
]
});
});
Expand Down Expand Up @@ -183,8 +187,11 @@ describe('Type System: Enum Values', () => {
)
).to.deep.equal({
errors: [
{ message:
'Variable "$color" expected value of type "Color!" but got: 2.' }
{
message:
'Variable "\$color" got invalid value 2.' +
'\nExpected type "Color", found 2.'
}
]
});
});
Expand Down
15 changes: 10 additions & 5 deletions src/utilities/isValidJSValue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,14 @@ import type { GraphQLInputType } from '../type/definition';
export function isValidJSValue(value: any, type: GraphQLInputType): [ string ] {
// A value must be provided if the type is non-null.
if (type instanceof GraphQLNonNull) {
var ofType: GraphQLInputType = (type.ofType: any);
if (isNullish(value)) {
return [ 'Expected non-null value.' ];
if (ofType.name) {
return [ `Expected "${ofType.name}!", found null.` ];
}
return [ 'Expected non-null value, found null.' ];
}
var nullableType: GraphQLInputType = (type.ofType: any);
return isValidJSValue(value, nullableType);
return isValidJSValue(value, ofType);
}

if (isNullish(value)) {
Expand All @@ -56,7 +59,7 @@ export function isValidJSValue(value: any, type: GraphQLInputType): [ string ] {
// Input objects check each defined field.
if (type instanceof GraphQLInputObjectType) {
if (typeof value !== 'object') {
return [ 'Not an object.' ];
return [ `Expected "${type.name}", found not an object.` ];
}
var fields = type.getFields();

Expand Down Expand Up @@ -88,7 +91,9 @@ export function isValidJSValue(value: any, type: GraphQLInputType): [ string ] {
// a non-null value.
var parseResult = type.parseValue(value);
if (isNullish(parseResult)) {
return [ `Expected type "${type.name}", found "{JSON.strigify(value)}".` ];
return [
`Expected type "${type.name}", found ${JSON.stringify(value)}.`
];
}

return [];
Expand Down
9 changes: 6 additions & 3 deletions src/utilities/isValidLiteralValue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ export function isValidLiteralValue(
): [ string ] {
// A value must be provided if the type is non-null.
if (type instanceof GraphQLNonNull) {
var ofType: GraphQLInputType = (type.ofType: any);
if (!valueAST) {
return [ 'Expected non-null value.' ];
if (ofType.name) {
return [ `Expected "${ofType.name}!", found null.` ];
}
return [ 'Expected non-null value, found null.' ];
}
var ofType: GraphQLInputType = (type.ofType: any);
return isValidLiteralValue(ofType, valueAST);
}

Expand Down Expand Up @@ -75,7 +78,7 @@ export function isValidLiteralValue(
// Input objects check each defined field and look for undefined fields.
if (type instanceof GraphQLInputObjectType) {
if (valueAST.kind !== OBJECT) {
return [ 'Not an object.' ];
return [ `Expected "${type.name}", found not an object.` ];
}
var fields = type.getFields();

Expand Down
35 changes: 24 additions & 11 deletions src/validation/__tests__/ArgumentsOfCorrectType.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,21 @@ import {


function badValue(argName, typeName, value, line, column, errors) {
var realErrors;
if (!errors) {
realErrors = [
`Expected type "${typeName}", found ${value}.`
];
} else {
realErrors = errors;
}
return {
message: badValueMessage(argName, typeName, value, errors),
message: badValueMessage(argName, typeName, value, realErrors),
locations: [ { line, column } ],
};
}


describe('Validate: Argument values of correct type', () => {

describe('Valid values', () => {
Expand Down Expand Up @@ -484,7 +493,9 @@ describe('Validate: Argument values of correct type', () => {
}
}
`, [
badValue('stringListArg', '[String]', '["one", 2]', 4, 47),
badValue('stringListArg', '[String]', '["one", 2]', 4, 47, [
'In element #1: Expected type "String", found 2.'
]),
]);
});

Expand All @@ -496,7 +507,7 @@ describe('Validate: Argument values of correct type', () => {
}
}
`, [
badValue('stringListArg', '[String]', '1', 4, 47),
badValue('stringListArg', 'String', '1', 4, 47),
]);
});

Expand Down Expand Up @@ -618,8 +629,8 @@ describe('Validate: Argument values of correct type', () => {
}
}
`, [
badValue('req2', 'Int!', '"two"', 4, 32),
badValue('req1', 'Int!', '"one"', 4, 45),
badValue('req2', 'Int', '"two"', 4, 32),
badValue('req1', 'Int', '"one"', 4, 45),
]);
});

Expand All @@ -631,7 +642,7 @@ describe('Validate: Argument values of correct type', () => {
}
}
`, [
badValue('req1', 'Int!', '"one"', 4, 32),
badValue('req1', 'Int', '"one"', 4, 32),
]);
});

Expand Down Expand Up @@ -726,7 +737,7 @@ describe('Validate: Argument values of correct type', () => {
}
`, [
badValue('complexArg', 'ComplexInput', '{intField: 4}', 4, 41, [
'In field requiredField: Expected non-null value.'
'In field "requiredField": Expected "Boolean!", found null.'
]),
]);
});
Expand All @@ -747,7 +758,9 @@ describe('Validate: Argument values of correct type', () => {
'ComplexInput',
'{stringListField: ["one", 2], requiredField: true}',
4,
41
41,
[ 'In field "stringListField": In element #1: ' +
'Expected type "String", found 2.' ]
),
]);
});
Expand All @@ -769,7 +782,7 @@ describe('Validate: Argument values of correct type', () => {
'{requiredField: true, unknownField: "value"}',
4,
41,
[ 'Unknown field unknownField.' ]
[ 'Unknown field "unknownField".' ]
),
]);
});
Expand Down Expand Up @@ -799,8 +812,8 @@ describe('Validate: Argument values of correct type', () => {
}
}
`, [
badValue('if', 'Boolean!', '"yes"', 3, 28),
badValue('if', 'Boolean!', 'ENUM', 4, 28),
badValue('if', 'Boolean', '"yes"', 3, 28),
badValue('if', 'Boolean', 'ENUM', 4, 28),
]);
});

Expand Down
26 changes: 20 additions & 6 deletions src/validation/__tests__/DefaultValuesOfCorrectType.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,16 @@ function defaultForNonNullArg(varName, typeName, guessTypeName, line, column) {
}

function badValue(varName, typeName, val, line, column, errors) {
var realErrors;
if (!errors) {
realErrors = [
`Expected type "${typeName}", found ${val}.`
];
} else {
realErrors = errors;
}
return {
message: badValueForDefaultArgMessage(varName, typeName, val, errors),
message: badValueForDefaultArgMessage(varName, typeName, val, realErrors),
locations: [ { line, column } ],
};
}
Expand Down Expand Up @@ -81,10 +89,14 @@ describe('Validate: Variable default values of correct type', () => {
dog { name }
}
`, [
badValue('a', 'Int', '"one"', 3, 19),
badValue('b', 'String', '4', 4, 22),
badValue('a', 'Int', '"one"', 3, 19, [
'Expected type "Int", found "one".'
]),
badValue('b', 'String', '4', 4, 22, [
'Expected type "String", found 4.'
]),
badValue('c', 'ComplexInput', '"notverycomplex"', 5, 28, [
'Not an object.'
'Expected "ComplexInput", found not an object.'
])
]);
});
Expand All @@ -96,7 +108,7 @@ describe('Validate: Variable default values of correct type', () => {
}
`, [
badValue('a', 'ComplexInput', '{intField: 3}', 2, 53, [
'In field requiredField: Expected non-null value.'
'In field "requiredField": Expected "Boolean!", found null.'
])
]);
});
Expand All @@ -107,7 +119,9 @@ describe('Validate: Variable default values of correct type', () => {
dog { name }
}
`, [
badValue('a', '[String]', '["one", 2]', 2, 40)
badValue('a', '[String]', '["one", 2]', 2, 40, [
'In element #1: Expected type "String", found 2.'
])
]);
});

Expand Down

0 comments on commit 98364f6

Please sign in to comment.