Skip to content

fix: name fragment variables as such in execution errors #4712

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.

Sign up for GitHub

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
yaacovCR merged 2 commits into from
May 4, 2026
Merged

fix: name fragment variables as such in execution errors #4712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
633f8c6
fix: name fragment variables in execution errors
yaacovCR May 4, 2026
333e50a
tweak wording
yaacovCR May 4, 2026
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
30 changes: 20 additions & 10 deletions src/execution/__tests__/variables-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1412,11 +1412,16 @@ describe('Execute: Handles inputs', () => {
}
`);

expect(result).to.have.property('errors');
expect(result.errors).to.have.length(1);
expect(result.errors?.at(0)?.message).to.match(
/Argument "value" of required type "String!"/,
);
expectJSON(result).toDeepEqual({
data: null,
errors: [
{
message:
'Variable "$value" defined by fragment "a" of required type "String!" was not provided.',
locations: [{ line: 3, column: 11 }],
},
],
});
});

it('when the definition has a default and is provided', () => {
Expand Down Expand Up @@ -1480,11 +1485,16 @@ describe('Execute: Handles inputs', () => {
}
`);

expect(result).to.have.property('errors');
expect(result.errors).to.have.length(1);
expect(result.errors?.at(0)?.message).to.match(
/Argument "value" has invalid value: Expected value of non-null type "String!" not to be null./,
);
expectJSON(result).toDeepEqual({
data: null,
errors: [
{
message:
'Variable "$value" defined by fragment "a" has invalid value: Expected value of non-null type "String!" not to be null.',
locations: [{ line: 3, column: 23 }],
},
],
});
});

it('when the definition has no default and is not provided', () => {
Expand Down
14 changes: 12 additions & 2 deletions src/execution/values.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ function coerceArgument(
// continue with an invalid argument value.
throw new GraphQLError(
// TODO: clean up the naming of isRequiredArgument(), isArgument(), and argDef if/when experimental fragment variables are merged
`Argument "${isArgument(argDef) ? argDef : argName}" of required type "${argType}" was not provided.`,
`${printArgumentOrFragmentVariable(argDef, node)} of required type "${argType}" was not provided.`,
{ nodes: node },
);
}
Expand Down Expand Up @@ -291,7 +291,7 @@ function coerceArgument(
argType,
(error, path) => {
// TODO: clean up the naming of isRequiredArgument(), isArgument(), and argDef if/when experimental fragment variables are merged
error.message = `Argument "${isArgument(argDef) ? argDef : argDef.name}" has invalid value${printPathArray(
error.message = `${printArgumentOrFragmentVariable(argDef, node)} has invalid value${printPathArray(
path,
)}: ${error.message}`;
throw error;
Expand All @@ -306,6 +306,16 @@ function coerceArgument(
coercedValues[argName] = coercedValue;
}

// TODO: clean up the naming of isRequiredArgument(), isArgument(), and argDef if/when experimental fragment variables are merged
function printArgumentOrFragmentVariable(
argDef: GraphQLArgument | GraphQLVariableSignature,
node: FieldNode | DirectiveNode | FragmentSpreadNode,
): string {
return isArgument(argDef)
? `Argument "${argDef}"`
: `Variable "$${argDef.name}" defined by fragment "${node.name.value}"`;
}

/**
* Prepares an object map of argument values given a directive definition
* and a AST node which may contain directives. Optionally also accepts a map
Expand Down
Loading