Skip to content

Commit 7ee4f42

Browse files
yaacovCRyaacovCR
committed
standardize error messages prior to introducing schema coordinates (#4177)
extracted from #3808 PR #3808 uses schema coordinates to improve GraphQL-JS error messages. To reduce the size of the PR, this commit standardizes error messages according to the general pattern that will be introduced with schema coordinates without introducing the coordinates themselves, in the hopes of aiding review of the later PR. EDITED 8/26/2024: I was able to reproduce all of the standardized error messages from Definition, because the parent type is not passed. Everything else can be calculated for the error messages we are currently printing, although schema coordinates simplifies things. Extracting these changes out of #3808 and rebasing #3808 on main will therefore will better demonstrate how schema coordinates improves the clarity of some of our error messages (namely, getArgumentValues) and simplifies printing them.
1 parent e9851f2 commit 7ee4f42

18 files changed

+210
-183
lines changed

‎src/execution/__tests__/variables-test.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ describe('Execute: Handles inputs', () => {
226226
errors: [
227227
{
228228
message:
229-
'Argument "input" has invalid value ["foo", "bar", "baz"].',
229+
'Argument "input" of type "TestInputObject" has invalid value ["foo", "bar", "baz"].',
230230
path: ['fieldWithObjectInput'],
231231
locations: [{ line: 3, column: 41 }],
232232
},
@@ -263,7 +263,7 @@ describe('Execute: Handles inputs', () => {
263263
errors: [
264264
{
265265
message:
266-
'Argument "input" has invalid value { c: "foo", e: "bar" }.',
266+
'Argument "input" of type "TestInputObject" has invalid value { c: "foo", e: "bar" }.',
267267
path: ['fieldWithObjectInput'],
268268
locations: [{ line: 3, column: 39 }],
269269
},
@@ -462,7 +462,7 @@ describe('Execute: Handles inputs', () => {
462462
errors: [
463463
{
464464
message:
465-
'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "c" of required type "String!" was not provided.',
465+
'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "TestInputObject.c" of required type "String!" was not provided.',
466466
locations: [{ line: 2, column: 16 }],
467467
},
468468
],
@@ -481,12 +481,12 @@ describe('Execute: Handles inputs', () => {
481481
errors: [
482482
{
483483
message:
484-
'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "c" of required type "String!" was not provided.',
484+
'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "TestInputObject.c" of required type "String!" was not provided.',
485485
locations: [{ line: 2, column: 18 }],
486486
},
487487
{
488488
message:
489-
'Variable "$input" got invalid value { na: { a: "foo" } }; Field "nb" of required type "String!" was not provided.',
489+
'Variable "$input" got invalid value { na: { a: "foo" } }; Field "TestNestedInputObject.nb" of required type "String!" was not provided.',
490490
locations: [{ line: 2, column: 18 }],
491491
},
492492
],
@@ -1042,7 +1042,8 @@ describe('Execute: Handles inputs', () => {
10421042
},
10431043
errors: [
10441044
{
1045-
message: 'Argument "input" has invalid value WRONG_TYPE.',
1045+
message:
1046+
'Argument "input" of type "String" has invalid value WRONG_TYPE.',
10461047
locations: [{ line: 3, column: 48 }],
10471048
path: ['fieldWithDefaultArgumentValue'],
10481049
},

‎src/execution/execute.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -715,7 +715,7 @@ function completeValue(
715715
);
716716
if (completed === null) {
717717
throw new Error(
718-
`Cannot return null for non-nullable field ${info.parentType.name}.${info.fieldName}.`,
718+
`Cannot return null for non-nullable field ${info.parentType}.${info.fieldName}.`,
719719
);
720720
}
721721
return completed;
@@ -904,7 +904,7 @@ function completeListValue(
904904

905905
if (!isIterableObject(result)) {
906906
throw new GraphQLError(
907-
`Expected Iterable, but did not find one for field "${info.parentType.name}.${info.fieldName}".`,
907+
`Expected Iterable, but did not find one for field "${info.parentType}.${info.fieldName}".`,
908908
);
909909
}
910910

@@ -1134,7 +1134,7 @@ function ensureValidRuntimeType(
11341134
): GraphQLObjectType {
11351135
if (runtimeTypeName == null) {
11361136
throw new GraphQLError(
1137-
`Abstract type "${returnType.name}" must resolve to an Object type at runtime for field "${info.parentType.name}.${info.fieldName}". Either the "${returnType.name}" type should provide a "resolveType" function or each possible type should provide an "isTypeOf" function.`,
1137+
`Abstract type "${returnType}" must resolve to an Object type at runtime for field "${info.parentType}.${info.fieldName}". Either the "${returnType}" type should provide a "resolveType" function or each possible type should provide an "isTypeOf" function.`,
11381138
{ nodes: fieldGroup },
11391139
);
11401140
}
@@ -1149,29 +1149,29 @@ function ensureValidRuntimeType(
11491149

11501150
if (typeof runtimeTypeName !== 'string') {
11511151
throw new GraphQLError(
1152-
`Abstract type "${returnType.name}" must resolve to an Object type at runtime for field "${info.parentType.name}.${info.fieldName}" with ` +
1152+
`Abstract type "${returnType}" must resolve to an Object type at runtime for field "${info.parentType}.${info.fieldName}" with ` +
11531153
`value ${inspect(result)}, received "${inspect(runtimeTypeName)}".`,
11541154
);
11551155
}
11561156

11571157
const runtimeType = exeContext.schema.getType(runtimeTypeName);
11581158
if (runtimeType == null) {
11591159
throw new GraphQLError(
1160-
`Abstract type "${returnType.name}" was resolved to a type "${runtimeTypeName}" that does not exist inside the schema.`,
1160+
`Abstract type "${returnType}" was resolved to a type "${runtimeTypeName}" that does not exist inside the schema.`,
11611161
{ nodes: fieldGroup },
11621162
);
11631163
}
11641164

11651165
if (!isObjectType(runtimeType)) {
11661166
throw new GraphQLError(
1167-
`Abstract type "${returnType.name}" was resolved to a non-object type "${runtimeTypeName}".`,
1167+
`Abstract type "${returnType}" was resolved to a non-object type "${runtimeTypeName}".`,
11681168
{ nodes: fieldGroup },
11691169
);
11701170
}
11711171

11721172
if (!exeContext.schema.isSubType(returnType, runtimeType)) {
11731173
throw new GraphQLError(
1174-
`Runtime Object type "${runtimeType.name}" is not a possible type for "${returnType.name}".`,
1174+
`Runtime Object type "${runtimeType}" is not a possible type for "${returnType}".`,
11751175
{ nodes: fieldGroup },
11761176
);
11771177
}
@@ -1231,7 +1231,7 @@ function invalidReturnTypeError(
12311231
fieldGroup: FieldGroup,
12321232
): GraphQLError {
12331233
return new GraphQLError(
1234-
`Expected value of type "${returnType.name}" but got: ${inspect(result)}.`,
1234+
`Expected value of type "${returnType}" but got: ${inspect(result)}.`,
12351235
{ nodes: fieldGroup },
12361236
);
12371237
}

‎src/execution/values.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,9 @@ export function getArgumentValues(
215215
// execution. This is a runtime check to ensure execution does not
216216
// continue with an invalid argument value.
217217
throw new GraphQLError(
218-
`Argument "${name}" has invalid value ${print(valueNode)}.`,
218+
`Argument "${name}" of type "${inspect(
219+
argType,
220+
)}" has invalid value ${print(valueNode)}.`,
219221
{ nodes: valueNode },
220222
);
221223
}

‎src/type/__tests__/introspection-test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,7 +1644,7 @@ describe('Introspection', () => {
16441644
errors: [
16451645
{
16461646
message:
1647-
'Field "__type" argument "name" of type "String!" is required, but it was not provided.',
1647+
'Argument "<meta>.__type(name:)" of type "String!" is required, but it was not provided.',
16481648
locations: [{ line: 3, column: 9 }],
16491649
},
16501650
],
@@ -1738,11 +1738,11 @@ describe('Introspection', () => {
17381738
_3: any,
17391739
info: GraphQLResolveInfo,
17401740
): never {
1741-
expect.fail(`Called on ${info.parentType.name}::${info.fieldName}`);
1741+
expect.fail(`Called on ${info.parentType}::${info.fieldName}`);
17421742
}
17431743

17441744
function typeResolver(_1: any, _2: any, info: GraphQLResolveInfo): never {
1745-
expect.fail(`Called on ${info.parentType.name}::${info.fieldName}`);
1745+
expect.fail(`Called on ${info.parentType}::${info.fieldName}`);
17461746
}
17471747
/* c8 ignore stop */
17481748

‎src/type/__tests__/validation-test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2089,7 +2089,7 @@ describe('Objects must adhere to Interface they implement', () => {
20892089
expectJSON(validateSchema(schema)).toDeepEqual([
20902090
{
20912091
message:
2092-
'Object field AnotherObject.field includes required argument requiredArg that is missing from the Interface field AnotherInterface.field.',
2092+
'Argument "AnotherObject.field(requiredArg:)" must not be required type "String!" if not provided by the Interface field "AnotherInterface.field".',
20932093
locations: [
20942094
{ line: 13, column: 11 },
20952095
{ line: 7, column: 9 },
@@ -2546,7 +2546,7 @@ describe('Interfaces must adhere to Interface they implement', () => {
25462546
expectJSON(validateSchema(schema)).toDeepEqual([
25472547
{
25482548
message:
2549-
'Object field ChildInterface.field includes required argument requiredArg that is missing from the Interface field ParentInterface.field.',
2549+
'Argument "ChildInterface.field(requiredArg:)" must not be required type "String!" if not provided by the Interface field "ParentInterface.field".',
25502550
locations: [
25512551
{ line: 13, column: 11 },
25522552
{ line: 7, column: 9 },

‎src/type/validate.ts‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ function validateRootTypes(context: SchemaValidationContext): void {
148148
if (operationTypes.length > 1) {
149149
const operationList = andList(operationTypes);
150150
context.reportError(
151-
`All root types must be different, "${rootType.name}" type is used as ${operationList} root types.`,
151+
`All root types must be different, "${rootType}" type is used as ${operationList} root types.`,
152152
operationTypes.map((operationType) =>
153153
getOperationTypeNode(schema, operationType),
154154
),
@@ -185,7 +185,7 @@ function validateDirectives(context: SchemaValidationContext): void {
185185

186186
if (directive.locations.length === 0) {
187187
context.reportError(
188-
`Directive @${directive.name} must include 1 or more locations.`,
188+
`Directive ${directive} must include 1 or more locations.`,
189189
directive.astNode,
190190
);
191191
}
@@ -198,15 +198,15 @@ function validateDirectives(context: SchemaValidationContext): void {
198198
// Ensure the type is an input type.
199199
if (!isInputType(arg.type)) {
200200
context.reportError(
201-
`The type of @${directive.name}(${arg.name}:) must be Input Type ` +
201+
`The type of ${directive}(${arg.name}:) must be Input Type ` +
202202
`but got: ${inspect(arg.type)}.`,
203203
arg.astNode,
204204
);
205205
}
206206

207207
if (isRequiredArgument(arg) && arg.deprecationReason != null) {
208208
context.reportError(
209-
`Required argument @${directive.name}(${arg.name}:) cannot be deprecated.`,
209+
`Required argument ${directive}(${arg.name}:) cannot be deprecated.`,
210210
[getDeprecatedDirectiveNode(arg.astNode), arg.astNode?.type],
211211
);
212212
}
@@ -282,7 +282,7 @@ function validateFields(
282282

283283
// Objects and Interfaces both must define one or more fields.
284284
if (fields.length === 0) {
285-
context.reportError(`Type ${type.name} must define one or more fields.`, [
285+
context.reportError(`Type ${type} must define one or more fields.`, [
286286
type.astNode,
287287
...type.extensionASTNodes,
288288
]);
@@ -295,7 +295,7 @@ function validateFields(
295295
// Ensure the type is an output type
296296
if (!isOutputType(field.type)) {
297297
context.reportError(
298-
`The type of ${type.name}.${field.name} must be Output Type ` +
298+
`The type of ${type}.${field.name} must be Output Type ` +
299299
`but got: ${inspect(field.type)}.`,
300300
field.astNode?.type,
301301
);
@@ -311,15 +311,15 @@ function validateFields(
311311
// Ensure the type is an input type
312312
if (!isInputType(arg.type)) {
313313
context.reportError(
314-
`The type of ${type.name}.${field.name}(${argName}:) must be Input ` +
314+
`The type of ${type}.${field.name}(${argName}:) must be Input ` +
315315
`Type but got: ${inspect(arg.type)}.`,
316316
arg.astNode?.type,
317317
);
318318
}
319319

320320
if (isRequiredArgument(arg) && arg.deprecationReason != null) {
321321
context.reportError(
322-
`Required argument ${type.name}.${field.name}(${argName}:) cannot be deprecated.`,
322+
`Required argument ${type}.${field.name}(${argName}:) cannot be deprecated.`,
323323
[getDeprecatedDirectiveNode(arg.astNode), arg.astNode?.type],
324324
);
325325
}
@@ -344,15 +344,15 @@ function validateInterfaces(
344344

345345
if (type === iface) {
346346
context.reportError(
347-
`Type ${type.name} cannot implement itself because it would create a circular reference.`,
347+
`Type ${type} cannot implement itself because it would create a circular reference.`,
348348
getAllImplementsInterfaceNodes(type, iface),
349349
);
350350
continue;
351351
}
352352

353353
if (ifaceTypeNames.has(iface.name)) {
354354
context.reportError(
355-
`Type ${type.name} can only implement ${iface.name} once.`,
355+
`Type ${type} can only implement ${iface.name} once.`,
356356
getAllImplementsInterfaceNodes(type, iface),
357357
);
358358
continue;
@@ -380,7 +380,7 @@ function validateTypeImplementsInterface(
380380
// Assert interface field exists on type.
381381
if (typeField == null) {
382382
context.reportError(
383-
`Interface field ${iface.name}.${fieldName} expected but ${type.name} does not provide it.`,
383+
`Interface field ${iface.name}.${fieldName} expected but ${type} does not provide it.`,
384384
[ifaceField.astNode, type.astNode, ...type.extensionASTNodes],
385385
);
386386
continue;
@@ -391,7 +391,7 @@ function validateTypeImplementsInterface(
391391
if (!isTypeSubTypeOf(context.schema, typeField.type, ifaceField.type)) {
392392
context.reportError(
393393
`Interface field ${iface.name}.${fieldName} expects type ` +
394-
`${inspect(ifaceField.type)} but ${type.name}.${fieldName} ` +
394+
`${inspect(ifaceField.type)} but ${type}.${fieldName} ` +
395395
`is type ${inspect(typeField.type)}.`,
396396
[ifaceField.astNode?.type, typeField.astNode?.type],
397397
);
@@ -405,7 +405,7 @@ function validateTypeImplementsInterface(
405405
// Assert interface field arg exists on object field.
406406
if (!typeArg) {
407407
context.reportError(
408-
`Interface field argument ${iface.name}.${fieldName}(${argName}:) expected but ${type.name}.${fieldName} does not provide it.`,
408+
`Interface field argument ${iface.name}.${fieldName}(${argName}:) expected but ${type}.${fieldName} does not provide it.`,
409409
[ifaceArg.astNode, typeField.astNode],
410410
);
411411
continue;
@@ -418,7 +418,7 @@ function validateTypeImplementsInterface(
418418
context.reportError(
419419
`Interface field argument ${iface.name}.${fieldName}(${argName}:) ` +
420420
`expects type ${inspect(ifaceArg.type)} but ` +
421-
`${type.name}.${fieldName}(${argName}:) is type ` +
421+
`${type}.${fieldName}(${argName}:) is type ` +
422422
`${inspect(typeArg.type)}.`,
423423
[ifaceArg.astNode?.type, typeArg.astNode?.type],
424424
);
@@ -433,7 +433,11 @@ function validateTypeImplementsInterface(
433433
const ifaceArg = ifaceField.args.find((arg) => arg.name === argName);
434434
if (!ifaceArg && isRequiredArgument(typeArg)) {
435435
context.reportError(
436-
`Object field ${type.name}.${fieldName} includes required argument ${argName} that is missing from the Interface field ${iface.name}.${fieldName}.`,
436+
`Argument "${type}.${fieldName}(${argName}:)" must not be required type "${inspect(
437+
typeArg.type,
438+
)}" if not provided by the Interface field "${
439+
iface.name
440+
}.${fieldName}".`,
437441
[typeArg.astNode, ifaceField.astNode],
438442
);
439443
}
@@ -451,8 +455,8 @@ function validateTypeImplementsAncestors(
451455
if (!ifaceInterfaces.includes(transitive)) {
452456
context.reportError(
453457
transitive === type
454-
? `Type ${type.name} cannot implement ${iface.name} because it would create a circular reference.`
455-
: `Type ${type.name} must implement ${transitive.name} because it is implemented by ${iface.name}.`,
458+
? `Type ${type} cannot implement ${iface.name} because it would create a circular reference.`
459+
: `Type ${type} must implement ${transitive.name} because it is implemented by ${iface.name}.`,
456460
[
457461
...getAllImplementsInterfaceNodes(iface, transitive),
458462
...getAllImplementsInterfaceNodes(type, iface),
@@ -479,7 +483,7 @@ function validateUnionMembers(
479483
for (const memberType of memberTypes) {
480484
if (includedTypeNames.has(memberType.name)) {
481485
context.reportError(
482-
`Union type ${union.name} can only include type ${memberType.name} once.`,
486+
`Union type ${union.name} can only include type ${memberType} once.`,
483487
getUnionMemberTypeNodes(union, memberType.name),
484488
);
485489
continue;
@@ -503,7 +507,7 @@ function validateEnumValues(
503507

504508
if (enumValues.length === 0) {
505509
context.reportError(
506-
`Enum type ${enumType.name} must define one or more values.`,
510+
`Enum type ${enumType} must define one or more values.`,
507511
[enumType.astNode, ...enumType.extensionASTNodes],
508512
);
509513
}
@@ -561,14 +565,14 @@ function validateOneOfInputObjectField(
561565
): void {
562566
if (isNonNullType(field.type)) {
563567
context.reportError(
564-
`OneOf input field ${type.name}.${field.name} must be nullable.`,
568+
`OneOf input field ${type}.${field.name} must be nullable.`,
565569
field.astNode?.type,
566570
);
567571
}
568572

569573
if (field.defaultValue !== undefined) {
570574
context.reportError(
571-
`OneOf input field ${type.name}.${field.name} cannot have a default value.`,
575+
`OneOf input field ${type}.${field.name} cannot have a default value.`,
572576
field.astNode,
573577
);
574578
}
@@ -614,7 +618,7 @@ function createInputObjectCircularRefsValidator(
614618
const cyclePath = fieldPath.slice(cycleIndex);
615619
const pathStr = cyclePath.map((fieldObj) => fieldObj.name).join('.');
616620
context.reportError(
617-
`Cannot reference Input Object "${fieldType.name}" within itself through a series of non-null fields: "${pathStr}".`,
621+
`Cannot reference Input Object "${fieldType}" within itself through a series of non-null fields: "${pathStr}".`,
618622
cyclePath.map((fieldObj) => fieldObj.astNode),
619623
);
620624
}

‎src/utilities/__tests__/coerceInputValue-test.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,8 @@ describe('coerceInputValue', () => {
246246
const result = coerceValue({ bar: 123 }, TestInputObject);
247247
expectErrors(result).to.deep.equal([
248248
{
249-
error: 'Field "foo" of required type "Int!" was not provided.',
249+
error:
250+
'Field "TestInputObject.foo" of required type "Int!" was not provided.',
250251
path: [],
251252
value: { bar: 123 },
252253
},

0 commit comments

Comments
 (0)