Add handling for invalid argument values, error reporting in getComplexity function #61

Author: ivome

.gitignore

@@ -2,3 +2,4 @@
 node_modules
 .DS_Store
 .idea
+.vscode

README.md

@@ -171,16 +171,21 @@ const query = parse(`
   }
 `);
 
-const complexity = getComplexity({
-  estimators: [simpleEstimator({ defaultComplexity: 1 })],
-  schema,
-  query,
-  variables: {
-    count: 10,
-  },
-});
-
-console.log(complexity); // Output: 3
+try {
+  const complexity = getComplexity({
+    estimators: [simpleEstimator({ defaultComplexity: 1 })],
+    schema,
+    query,
+    variables: {
+      count: 10,
+    },
+  });
+
+  console.log(complexity); // Output: 3
+} catch (e) {
+  // Log error in case complexity cannot be calculated (invalid query, misconfiguration, etc.)
+  console.error('Could not calculate complexity', e.message);
+}
 ```
 
 ## Prior Art

src/QueryComplexity.ts

@@ -96,11 +96,12 @@ export function getComplexity(options: {
 }): number {
   const typeInfo = new TypeInfo(options.schema);
 
+  const errors: GraphQLError[] = [];
   const context = new ValidationContext(
     options.schema,
     options.query,
     typeInfo,
-    () => null
+    (error) => errors.push(error)
   );
   const visitor = new QueryComplexity(context, {
     // Maximum complexity does not matter since we're only interested in the calculated complexity.
@@ -111,6 +112,12 @@ export function getComplexity(options: {
   });
 
   visit(options.query, visitWithTypeInfo(typeInfo, visitor));
+
+  // Throw first error if any
+  if (errors.length) {
+    throw errors.pop();
+  }
+
   return visitor.complexity;
 }
 
@@ -244,7 +251,7 @@ export default class QueryComplexity {
           (
             complexities: ComplexityMap,
             childNode: FieldNode | FragmentSpreadNode | InlineFragmentNode
-          ) => {
+          ): ComplexityMap => {
             // let nodeComplexity = 0;
             let innerComplexities = complexities;
 
@@ -297,7 +304,8 @@ export default class QueryComplexity {
                     this.variableValues || {}
                   );
                 } catch (e) {
-                  return this.context.reportError(e);
+                  this.context.reportError(e);
+                  return complexities;
                 }
 
                 // Check if we have child complexity

src/__tests__/QueryComplexity-test.ts

@@ -2,7 +2,14 @@
  * Created by Ivo Meißner on 28.07.17.
  */
 
-import { parse, TypeInfo, visit, visitWithTypeInfo } from 'graphql';
+import {
+  parse,
+  TypeInfo,
+  visit,
+  visitWithTypeInfo,
+  validate,
+  specifiedRules,
+} from 'graphql';
 
 import { expect } from 'chai';
 
@@ -523,7 +530,7 @@ describe('QueryComplexity analysis', () => {
     );
   });
 
-  it('should return NaN when no astNode available on field when using directiveEstimator', () => {
+  it('should throw error when no astNode available on field when using directiveEstimator', () => {
     const ast = parse(`
       query {
         _service {
@@ -532,12 +539,13 @@ describe('QueryComplexity analysis', () => {
       }
     `);
 
-    const complexity = getComplexity({
-      estimators: [directiveEstimator()],
-      schema,
-      query: ast,
-    });
-    expect(Number.isNaN(complexity)).to.equal(true);
+    expect(() => {
+      getComplexity({
+        estimators: [directiveEstimator()],
+        schema,
+        query: ast,
+      });
+    }).to.throw(/No complexity could be calculated for field Query._service/);
   });
 
   it('should skip complexity calculation by directiveEstimator when no astNode available on field', () => {
@@ -796,4 +804,32 @@ describe('QueryComplexity analysis', () => {
     });
     expect(complexity).to.equal(30); // 3 fields on nonNullItem * 10
   });
+
+  it('should handle invalid argument values for multiple query fields', () => {
+    const ast = parse(`
+      query {
+        requiredArgs(count: x) {
+          scalar
+          complexScalar
+        }
+        nonNullItem {
+          scalar
+          complexScalar
+          variableScalar(count: 10)
+        }
+      }
+    `);
+
+    validate(schema, ast, [
+      ...specifiedRules,
+      createComplexityRule({
+        maximumComplexity: 1000,
+        estimators: [
+          simpleEstimator({
+            defaultComplexity: 1,
+          }),
+        ],
+      }),
+    ]);
+  });
 });