From 95ada8f1c06024991b1c7ed052a71c95eb9c0f5b Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 4 May 2026 18:31:51 +0300 Subject: [PATCH 1/2] fix: enhance runtime invalid default value error messages --- src/execution/__tests__/oneof-test.ts | 10 +- src/execution/__tests__/variables-test.ts | 152 ++++++++++++++++++ src/execution/values.ts | 90 +++++++++-- src/type/validate.ts | 109 +++++++------ .../__tests__/coerceInputValue-test.ts | 36 +++++ src/utilities/coerceInputValue.ts | 13 +- 6 files changed, 344 insertions(+), 66 deletions(-) diff --git a/src/execution/__tests__/oneof-test.ts b/src/execution/__tests__/oneof-test.ts index 4cb53af924..0416cccd2d 100644 --- a/src/execution/__tests__/oneof-test.ts +++ b/src/execution/__tests__/oneof-test.ts @@ -79,17 +79,11 @@ describe('Execute: Handles OneOf Input Objects', () => { const result = executeQuery(query, rootValue); expectJSON(result).toDeepEqual({ - data: { - test: null, - }, errors: [ { - locations: [{ column: 23, line: 3 }], + locations: [{ column: 16, line: 2 }], message: - // This type of error would be caught at validation-time - // hence the vague error message here. - 'Argument "Query.test(input:)" has invalid value: Expected variable "$input" provided to type "TestInputObject!" to provide a runtime value.', - path: ['test'], + 'Variable "$input" has invalid default value: OneOf Input Object "TestInputObject" must specify exactly one key.', }, ], }); diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index 2bd0ec7d99..dd6b664084 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -192,6 +192,34 @@ const TestType = new GraphQLObjectType({ }, }); +const TestTypeWithInvalidDefaultArgumentValue = new GraphQLObjectType({ + name: 'TestTypeWithInvalidDefaultArgumentValue', + fields: { + fieldWithInvalidDefaultArgumentValue: fieldWithInputArg({ + type: GraphQLString, + default: { value: 123 }, + }), + }, +}); + +const TestTypeWithInvalidNestedDefaultArgumentValue = new GraphQLObjectType({ + name: 'TestTypeWithInvalidNestedDefaultArgumentValue', + fields: { + fieldWithInvalidNestedDefaultArgumentValue: fieldWithInputArg({ + type: new GraphQLInputObjectType({ + name: 'InputWithInvalidNestedFieldDefault', + fields: { + foo: { + type: GraphQLString, + default: { value: 123 }, + }, + }, + }), + default: { value: {} }, + }), + }, +}); + const schema = new GraphQLSchema({ query: TestType, directives: [ @@ -450,6 +478,61 @@ describe('Execute: Handles inputs', () => { }); }); + it('reports invalid default values with variable definition locations', () => { + const result = executeQuery( + 'query ($input: String = 123) { fieldWithNullableStringInput(input: $input) }', + ); + + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" has invalid default value: String cannot represent a non string value: 123', + locations: [{ line: 1, column: 8 }], + }, + ], + }); + }); + + it('includes suggestions for invalid default values', () => { + const result = executeSync({ + schema, + document: parse( + 'query ($input: TestInputObject = { c: "ok", aa: "x" }) { fieldWithObjectInput(input: $input) }', + ), + }); + + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" has invalid default value: Expected value of type "TestInputObject" not to include unknown field "aa". Did you mean "a"? Found: { c: "ok", aa: "x" }.', + locations: [{ line: 1, column: 8 }], + }, + ], + }); + }); + + it('hides suggestions for invalid default values when specified', () => { + const result = executeSync({ + schema, + document: parse( + 'query ($input: TestInputObject = { c: "ok", aa: "x" }) { fieldWithObjectInput(input: $input) }', + ), + hideSuggestions: true, + }); + + expectJSON(result).toDeepEqual({ + errors: [ + { + message: + 'Variable "$input" has invalid default value: Expected value of type "TestInputObject" not to include unknown field "aa", found: { c: "ok", aa: "x" }.', + locations: [{ line: 1, column: 8 }], + }, + ], + }); + }); + it('does not use default value when provided', () => { const result = executeQuery( ` @@ -1296,6 +1379,58 @@ describe('Execute: Handles inputs', () => { }, }); }); + + it('localizes invalid default value errors during execution', () => { + const schemaWithInvalidDefaultArgumentValue = new GraphQLSchema({ + query: TestTypeWithInvalidDefaultArgumentValue, + assumeValid: true, + }); + + const result = executeSync({ + schema: schemaWithInvalidDefaultArgumentValue, + document: parse('{ fieldWithInvalidDefaultArgumentValue }'), + }); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithInvalidDefaultArgumentValue: null, + }, + errors: [ + { + message: + 'Argument "TestTypeWithInvalidDefaultArgumentValue.fieldWithInvalidDefaultArgumentValue(input:)" has invalid default value: String cannot represent a non string value: 123', + locations: [{ line: 1, column: 3 }], + path: ['fieldWithInvalidDefaultArgumentValue'], + }, + ], + }); + }); + + it('localizes nested invalid field default value errors during execution', () => { + const schemaWithInvalidNestedDefaultArgumentValue = new GraphQLSchema({ + query: TestTypeWithInvalidNestedDefaultArgumentValue, + assumeValid: true, + }); + + const result = executeSync({ + schema: schemaWithInvalidNestedDefaultArgumentValue, + document: parse('{ fieldWithInvalidNestedDefaultArgumentValue }'), + }); + + expectJSON(result).toDeepEqual({ + data: { + fieldWithInvalidNestedDefaultArgumentValue: null, + }, + errors: [ + { + message: + 'Argument "TestTypeWithInvalidNestedDefaultArgumentValue.fieldWithInvalidNestedDefaultArgumentValue(input:)" has invalid default value: Expected value of type "String" to be valid, found: 123.', + locations: [{ line: 1, column: 3 }], + path: ['fieldWithInvalidNestedDefaultArgumentValue'], + }, + ], + }); + }); }); describe('getVariableValues: limit maximum number of coercion errors', () => { @@ -1456,6 +1591,23 @@ describe('Execute: Handles inputs', () => { }); }); + it('when the definition has an invalid default and is not provided', () => { + const result = executeQueryWithFragmentArguments( + 'query { ...a } fragment a($value: String = 123) on TestType { fieldWithNullableStringInput(input: $value) }', + ); + + expectJSON(result).toDeepEqual({ + data: null, + errors: [ + { + message: + 'Variable "$value" defined by fragment "a" has invalid default value: String cannot represent a non string value: 123', + locations: [{ line: 1, column: 9 }], + }, + ], + }); + }); + it('when a definition has a default, is not provided, and spreads another fragment', () => { const result = executeQueryWithFragmentArguments(` query { diff --git a/src/execution/values.ts b/src/execution/values.ts index ce5fd6f78d..7e9d49296b 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -24,6 +24,7 @@ import { } from '../type/definition.js'; import type { GraphQLDirective } from '../type/directives.js'; import type { GraphQLSchema } from '../type/schema.js'; +import { validateDefaultInput } from '../type/validate.js'; import { coerceDefaultValue, @@ -117,7 +118,20 @@ function coerceVariableValues( if (value === undefined) { sources[varName] = { signature: varSignature }; if (varDefNode.defaultValue) { - coerced[varName] = coerceInputLiteral(varDefNode.defaultValue, varType); + maybeSetDefaultValue( + coerced, + varName, + varSignature, + (error, path) => { + onError( + new GraphQLError( + `Variable "$${varName}" has invalid default value${printPathArray(path)}: ${error.message}`, + { nodes: varDefNode }, + ), + ); + }, + hideSuggestions, + ); continue; } else if (!isNonNullType(varType)) { // Non-provided values for nullable variables are omitted. @@ -152,6 +166,49 @@ function coerceVariableValues( return { sources, coerced }; } +function maybeSetDefaultValue( + coercedValues: ObjMap, + name: string, + inputValue: GraphQLArgument | GraphQLVariableSignature, + onError: (error: GraphQLError, path: ReadonlyArray) => void, + hideSuggestions?: Maybe, +): void { + try { + // coerceDefaultValue assumes validation has already rejected invalid + // defaults. If validation was skipped, invalid defaults or nested input + // field defaults can throw here; recover with validation-style errors below. + const coercedDefaultValue = coerceDefaultValue(inputValue); + if (coercedDefaultValue !== undefined) { + coercedValues[name] = coercedDefaultValue; + } + } catch (error) { + const defaultInput = inputValue.default; + // Defensive: coerceDefaultValue should only throw while coercing a default. + /* c8 ignore next 3 */ + if (defaultInput === undefined) { + throw error; + } + + // Prefer validation's user-facing errors for invalid defaults. + let reportedValidationError = false; + validateDefaultInput( + defaultInput, + inputValue.type, + (defaultError, path) => { + reportedValidationError = true; + onError(defaultError, path); + }, + hideSuggestions, + ); + + if (!reportedValidationError) { + // The default itself validated, so coercion failed while applying a nested + // input field default. Surface the original coercion error. + onError(ensureGraphQLError(error), []); + } + } +} + export function getFragmentVariableValues( fragmentSpreadNode: FragmentSpreadNode, fragmentSignatures: ReadOnlyObjMap, @@ -236,6 +293,15 @@ function coerceArgument( hideSuggestions?: Maybe, ): void { const argType = argDef.type; + const onArgDefaultValueError = ( + error: GraphQLError, + path: ReadonlyArray, + ): never => { + throw new GraphQLError( + `${printArgumentOrFragmentVariable(argDef, node)} has invalid default value${printPathArray(path)}: ${error.message}`, + { nodes: node }, + ); + }; if (!argumentNode) { if (isRequiredArgument(argDef)) { @@ -248,10 +314,13 @@ function coerceArgument( { nodes: node }, ); } - const coercedDefaultValue = coerceDefaultValue(argDef); - if (coercedDefaultValue !== undefined) { - coercedValues[argName] = coercedDefaultValue; - } + maybeSetDefaultValue( + coercedValues, + argName, + argDef, + onArgDefaultValueError, + hideSuggestions, + ); return; } @@ -269,10 +338,13 @@ function coerceArgument( !Object.hasOwn(scopedVariableValues.coerced, variableName)) && !isRequiredArgument(argDef) ) { - const coercedDefaultValue = coerceDefaultValue(argDef); - if (coercedDefaultValue !== undefined) { - coercedValues[argName] = coercedDefaultValue; - } + maybeSetDefaultValue( + coercedValues, + argName, + argDef, + onArgDefaultValueError, + hideSuggestions, + ); return; } } diff --git a/src/type/validate.ts b/src/type/validate.ts index ede99c8054..1ddb4a9b54 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -36,6 +36,7 @@ import { import type { GraphQLArgument, + GraphQLDefaultInput, GraphQLEnumType, GraphQLInputField, GraphQLInputObjectType, @@ -241,64 +242,76 @@ function validateDefaultValue( return; } - if (defaultInput.literal) { - validateInputLiteral( - defaultInput.literal, - inputValue.type, - (error, path) => { - context.reportError( - `${inputValue} has invalid default value${printPathArray(path)}: ${ - error.message - }`, - error.nodes, - ); - }, - ); - } else { - const errors: Array<[GraphQLError, ReadonlyArray]> = []; - validateInputValue(defaultInput.value, inputValue.type, (error, path) => { - errors.push([error, path]); - }); + const errors: Array<[GraphQLError, ReadonlyArray]> = []; + validateDefaultInput(defaultInput, inputValue.type, (error, path) => { + errors.push([error, path]); + }); + + if (errors.length === 0) { + return; + } + if (!defaultInput.literal) { // If there were validation errors, check to see if it can be "uncoerced" // and then correctly validated. If so, report a clear error with a path // to resolution. - if (errors.length > 0) { - try { - const uncoercedValue = uncoerceDefaultValue( - defaultInput.value, - inputValue.type, - ); + try { + const uncoercedValue = uncoerceDefaultValue( + defaultInput.value, + inputValue.type, + ); - const uncoercedErrors = []; - validateInputValue(uncoercedValue, inputValue.type, (error, path) => { - uncoercedErrors.push([error, path]); - }); + const uncoercedErrors = []; + validateInputValue(uncoercedValue, inputValue.type, (error, path) => { + uncoercedErrors.push([error, path]); + }); - if (uncoercedErrors.length === 0) { - context.reportError( - `${inputValue} has invalid default value: ${inspect( - defaultInput.value, - )}. Did you mean: ${inspect(uncoercedValue)}?`, - inputValue.astNode?.defaultValue, - ); - return; - } - } catch (_error) { - // ignore + if (uncoercedErrors.length === 0) { + context.reportError( + `${inputValue} has invalid default value: ${inspect( + defaultInput.value, + )}. Did you mean: ${inspect(uncoercedValue)}?`, + inputValue.astNode?.defaultValue, + ); + return; } + } catch (_error) { + // ignore } + } - // Otherwise report the original set of errors. - for (const [error, path] of errors) { - context.reportError( - `${inputValue} has invalid default value${printPathArray(path)}: ${ - error.message - }`, - inputValue.astNode?.defaultValue, - ); - } + // Otherwise report the original set of errors. + for (const [error, path] of errors) { + context.reportError( + `${inputValue} has invalid default value${printPathArray(path)}: ${ + error.message + }`, + error.nodes ?? inputValue.astNode?.defaultValue, + ); + } +} + +/** + * @internal + */ +export function validateDefaultInput( + defaultInput: GraphQLDefaultInput, + inputType: GraphQLInputType, + onError: (error: GraphQLError, path: ReadonlyArray) => void, + hideSuggestions?: Maybe, +): void { + if (defaultInput.literal) { + validateInputLiteral( + defaultInput.literal, + inputType, + onError, + undefined, + undefined, + hideSuggestions, + ); + return; } + validateInputValue(defaultInput.value, inputType, onError, hideSuggestions); } /** diff --git a/src/utilities/__tests__/coerceInputValue-test.ts b/src/utilities/__tests__/coerceInputValue-test.ts index 23ea89a9a8..cf799ef0c2 100644 --- a/src/utilities/__tests__/coerceInputValue-test.ts +++ b/src/utilities/__tests__/coerceInputValue-test.ts @@ -241,6 +241,23 @@ describe('coerceInputValue', () => { .to.have.property('foo') .that.satisfy(Number.isNaN); }); + + it('invalid when coercing an invalid field default throws', () => { + // Invalid default values should be caught during validation. + const typeWithInvalidDefault = new GraphQLInputObjectType({ + name: 'TypeWithInvalidDefault', + fields: { + foo: { + type: GraphQLString, + default: { value: 123 }, + }, + }, + }); + + expect(() => coerceInputValue({}, typeWithInvalidDefault)).to.throw( + 'Expected value of type "String" to be valid, found: 123.', + ); + }); }); describe('for GraphQLList', () => { @@ -510,6 +527,25 @@ describe('coerceInputLiteral', () => { test('{}', type, { int: 42, float: 3.14 }); }); + it('invalid when coercing an invalid literal default throws', () => { + // Invalid default values should be caught during validation. + const typeWithInvalidDefault = new GraphQLInputObjectType({ + name: 'TypeWithInvalidLiteralDefault', + fields: { + foo: { + type: GraphQLString, + default: { literal: { kind: Kind.INT, value: '123' } }, + }, + }, + }); + + expect(() => + coerceInputLiteral(parseValue('{}'), typeWithInvalidDefault), + ).to.throw( + 'Expected value of type "String" to be valid, found: { kind: "IntValue", value: "123" }.', + ); + }); + const testInputObj = new GraphQLInputObjectType({ name: 'TestInput', fields: { diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index f93fd26edd..a1836d786c 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -1,3 +1,4 @@ +import { inspect } from '../jsutils/inspect.js'; import { invariant } from '../jsutils/invariant.js'; import { isIterableObject } from '../jsutils/isIterableObject.js'; import { isObjectLike } from '../jsutils/isObjectLike.js'; @@ -316,6 +317,11 @@ interface InputValue { } /** + * Returns the coerced default value for an input value definition, if it exists. + * + * If the default value is invalid, this will throw an error. Invalid default + * values should be caught during validation, however, so this function assumes + * that the default value is valid. * @internal */ export function coerceDefaultValue(inputValue: InputValue): unknown { @@ -330,7 +336,12 @@ export function coerceDefaultValue(inputValue: InputValue): unknown { coercedDefaultValue = defaultInput.literal ? coerceInputLiteral(defaultInput.literal, inputValue.type) : coerceInputValue(defaultInput.value, inputValue.type); - invariant(coercedDefaultValue !== undefined); + invariant( + coercedDefaultValue !== undefined, + `Expected value of type "${inputValue.type}" to be valid, found: ${inspect( + defaultInput.literal ?? defaultInput.value, + )}.`, + ); (inputValue as any)._memoizedCoercedDefaultValue = coercedDefaultValue; return coercedDefaultValue; } From 621b1ada56228dd9d3cdba34f378f3a7f91d4c19 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 5 May 2026 12:03:53 +0300 Subject: [PATCH 2/2] rename maybeSetDefaultValue => maybeUseDefaultValue --- src/execution/values.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/execution/values.ts b/src/execution/values.ts index 7e9d49296b..24705a15e8 100644 --- a/src/execution/values.ts +++ b/src/execution/values.ts @@ -118,7 +118,7 @@ function coerceVariableValues( if (value === undefined) { sources[varName] = { signature: varSignature }; if (varDefNode.defaultValue) { - maybeSetDefaultValue( + maybeUseDefaultValue( coerced, varName, varSignature, @@ -166,7 +166,7 @@ function coerceVariableValues( return { sources, coerced }; } -function maybeSetDefaultValue( +function maybeUseDefaultValue( coercedValues: ObjMap, name: string, inputValue: GraphQLArgument | GraphQLVariableSignature, @@ -314,7 +314,7 @@ function coerceArgument( { nodes: node }, ); } - maybeSetDefaultValue( + maybeUseDefaultValue( coercedValues, argName, argDef, @@ -338,7 +338,7 @@ function coerceArgument( !Object.hasOwn(scopedVariableValues.coerced, variableName)) && !isRequiredArgument(argDef) ) { - maybeSetDefaultValue( + maybeUseDefaultValue( coercedValues, argName, argDef,