Skip to content

Commit 4bcde62

Browse files
authored
Report variable coercion errors (#78)
* Report variable coercion errors * Ignore empty error list
1 parent a822992 commit 4bcde62

File tree

3 files changed

+56
-11
lines changed

3 files changed

+56
-11
lines changed

src/QueryComplexity.ts

+8-2
Original file line numberDiff line numberDiff line change
@@ -174,12 +174,18 @@ export default class QueryComplexity {
174174

175175
// Get variable values from variables that are passed from options, merged
176176
// with default values defined in the operation
177-
this.variableValues = getVariableValues(
177+
const { coerced, errors } = getVariableValues(
178178
this.context.getSchema(),
179179
// We have to create a new array here because input argument is not readonly in graphql ~14.6.0
180180
operation.variableDefinitions ? [...operation.variableDefinitions] : [],
181181
this.options.variables ?? {}
182-
).coerced;
182+
);
183+
if (errors && errors.length) {
184+
// We have input validation errors, report errors and abort
185+
errors.forEach((error) => this.context.reportError(error));
186+
return;
187+
}
188+
this.variableValues = coerced;
183189

184190
switch (operation.operation) {
185191
case 'query':

src/__tests__/QueryComplexity-test.ts

+40-9
Original file line numberDiff line numberDiff line change
@@ -254,21 +254,28 @@ describe('QueryComplexity analysis', () => {
254254
expect(visitor.complexity).to.equal(0);
255255
});
256256

257-
it('should ignore unused variables', () => {
257+
it('should report errors for unused variables', () => {
258258
const ast = parse(`
259259
query ($unusedVar: ID!) {
260260
variableScalar(count: 100)
261261
}
262262
`);
263263

264-
const context = new CompatibleValidationContext(schema, ast, typeInfo);
265-
const visitor = new ComplexityVisitor(context, {
266-
maximumComplexity: 100,
267-
estimators: [simpleEstimator({ defaultComplexity: 10 })],
268-
});
269-
270-
visit(ast, visitWithTypeInfo(typeInfo, visitor));
271-
expect(visitor.complexity).to.equal(10);
264+
const errors = validate(schema, ast, [
265+
createComplexityRule({
266+
maximumComplexity: 1000,
267+
estimators: [
268+
simpleEstimator({
269+
defaultComplexity: 1,
270+
}),
271+
],
272+
variables: {
273+
unusedVar: 'someID',
274+
},
275+
}),
276+
]);
277+
expect(errors).to.have.length(1);
278+
expect(errors[0].message).to.contain('$unusedVar');
272279
});
273280

274281
it('should ignore unknown field', () => {
@@ -860,4 +867,28 @@ describe('QueryComplexity analysis', () => {
860867
// query.scalar(5) + query.requiredArgs(5) * requiredArgs.scalar(5)
861868
expect(complexity).to.equal(30);
862869
});
870+
871+
it('reports variable coercion errors', () => {
872+
const ast = parse(`
873+
query ($input: RGB!){
874+
enumInputArg(enum: $input)
875+
}
876+
`);
877+
878+
const errors = validate(schema, ast, [
879+
createComplexityRule({
880+
maximumComplexity: 1000,
881+
estimators: [
882+
simpleEstimator({
883+
defaultComplexity: 1,
884+
}),
885+
],
886+
variables: {
887+
input: 'INVALIDVALUE',
888+
},
889+
}),
890+
]);
891+
expect(errors).to.have.length(1);
892+
expect(errors[0].message).to.contain('INVALIDVALUE');
893+
});
863894
});

src/__tests__/fixtures/schema.ts

+8
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,14 @@ const Query = new GraphQLObjectType({
164164
},
165165
},
166166
},
167+
enumInputArg: {
168+
type: GraphQLString,
169+
args: {
170+
enum: {
171+
type: EnumType,
172+
},
173+
},
174+
},
167175
_service: { type: SDLInterface },
168176
}),
169177
interfaces: () => [NameInterface, UnionInterface],

0 commit comments

Comments
 (0)