Error: make 'path' and 'locations' enumerable only if present (#1305)

* Error: make 'path' and 'locations' enumerable only if present

Also cleanup tests from 'undefined' and 'containSubset'.

* expect subset of errors

Author: IvanGoncharov

src/error/GraphQLError.js

@@ -151,7 +151,7 @@ export function GraphQLError( // eslint-disable-line no-redeclare
       // By being enumerable, JSON.stringify will include `locations` in the
       // resulting output. This ensures that the simplest possible GraphQL
       // service adheres to the spec.
-      enumerable: true,
+      enumerable: Boolean(_locations),
     },
     path: {
       // Coercing falsey values to undefined ensures they will not be included
@@ -160,7 +160,7 @@ export function GraphQLError( // eslint-disable-line no-redeclare
       // By being enumerable, JSON.stringify will include `path` in the
       // resulting output. This ensures that the simplest possible GraphQL
       // service adheres to the spec.
-      enumerable: true,
+      enumerable: Boolean(path),
     },
     nodes: {
       value: _nodes || undefined,

src/execution/__tests__/executor-test.js

@@ -8,7 +8,6 @@
 import { expect } from 'chai';
 import { describe, it } from 'mocha';
 import { execute } from '../execute';
-import { formatError } from '../../error';
 import { parse } from '../../language';
 import {
   GraphQLSchema,
@@ -461,78 +460,79 @@ describe('Execute: Handles basic execution tasks', () => {
 
     const result = await execute(schema, ast, data);
 
-    expect(result.data).to.deep.equal({
-      sync: 'sync',
-      syncError: null,
-      syncRawError: null,
-      syncReturnError: null,
-      syncReturnErrorList: ['sync0', null, 'sync2', null],
-      async: 'async',
-      asyncReject: null,
-      asyncRawReject: null,
-      asyncEmptyReject: null,
-      asyncError: null,
-      asyncRawError: null,
-      asyncReturnError: null,
-    });
-
-    expect(result.errors && result.errors.map(formatError)).to.deep.equal([
-      {
-        message: 'Error getting syncError',
-        locations: [{ line: 3, column: 7 }],
-        path: ['syncError'],
-      },
-      {
-        message: 'Error getting syncRawError',
-        locations: [{ line: 4, column: 7 }],
-        path: ['syncRawError'],
-      },
-      {
-        message: 'Error getting syncReturnError',
-        locations: [{ line: 5, column: 7 }],
-        path: ['syncReturnError'],
-      },
-      {
-        message: 'Error getting syncReturnErrorList1',
-        locations: [{ line: 6, column: 7 }],
-        path: ['syncReturnErrorList', 1],
-      },
-      {
-        message: 'Error getting syncReturnErrorList3',
-        locations: [{ line: 6, column: 7 }],
-        path: ['syncReturnErrorList', 3],
-      },
-      {
-        message: 'Error getting asyncReject',
-        locations: [{ line: 8, column: 7 }],
-        path: ['asyncReject'],
-      },
-      {
-        message: 'Error getting asyncRawReject',
-        locations: [{ line: 9, column: 7 }],
-        path: ['asyncRawReject'],
-      },
-      {
-        message: 'An unknown error occurred.',
-        locations: [{ line: 10, column: 7 }],
-        path: ['asyncEmptyReject'],
-      },
-      {
-        message: 'Error getting asyncError',
-        locations: [{ line: 11, column: 7 }],
-        path: ['asyncError'],
-      },
-      {
-        message: 'Error getting asyncRawError',
-        locations: [{ line: 12, column: 7 }],
-        path: ['asyncRawError'],
-      },
-      {
-        message: 'Error getting asyncReturnError',
-        locations: [{ line: 13, column: 7 }],
-        path: ['asyncReturnError'],
+    expect(result).to.deep.equal({
+      data: {
+        sync: 'sync',
+        syncError: null,
+        syncRawError: null,
+        syncReturnError: null,
+        syncReturnErrorList: ['sync0', null, 'sync2', null],
+        async: 'async',
+        asyncReject: null,
+        asyncRawReject: null,
+        asyncEmptyReject: null,
+        asyncError: null,
+        asyncRawError: null,
+        asyncReturnError: null,
       },
-    ]);
+      errors: [
+        {
+          message: 'Error getting syncError',
+          locations: [{ line: 3, column: 7 }],
+          path: ['syncError'],
+        },
+        {
+          message: 'Error getting syncRawError',
+          locations: [{ line: 4, column: 7 }],
+          path: ['syncRawError'],
+        },
+        {
+          message: 'Error getting syncReturnError',
+          locations: [{ line: 5, column: 7 }],
+          path: ['syncReturnError'],
+        },
+        {
+          message: 'Error getting syncReturnErrorList1',
+          locations: [{ line: 6, column: 7 }],
+          path: ['syncReturnErrorList', 1],
+        },
+        {
+          message: 'Error getting syncReturnErrorList3',
+          locations: [{ line: 6, column: 7 }],
+          path: ['syncReturnErrorList', 3],
+        },
+        {
+          message: 'Error getting asyncReject',
+          locations: [{ line: 8, column: 7 }],
+          path: ['asyncReject'],
+        },
+        {
+          message: 'Error getting asyncRawReject',
+          locations: [{ line: 9, column: 7 }],
+          path: ['asyncRawReject'],
+        },
+        {
+          message: '',
+          locations: [{ line: 10, column: 7 }],
+          path: ['asyncEmptyReject'],
+        },
+        {
+          message: 'Error getting asyncError',
+          locations: [{ line: 11, column: 7 }],
+          path: ['asyncError'],
+        },
+        {
+          message: 'Error getting asyncRawError',
+          locations: [{ line: 12, column: 7 }],
+          path: ['asyncRawError'],
+        },
+        {
+          message: 'Error getting asyncReturnError',
+          locations: [{ line: 13, column: 7 }],
+          path: ['asyncReturnError'],
+        },
+      ],
+    });
   });
 
   it('nulls error subtree for promise rejection #1071', async () => {
@@ -720,13 +720,7 @@ describe('Execute: Handles basic execution tasks', () => {
 
     const result = await execute(schema, ast, data);
     expect(result).to.deep.equal({
-      errors: [
-        {
-          message: 'Must provide an operation.',
-          locations: undefined,
-          path: undefined,
-        },
-      ],
+      errors: [{ message: 'Must provide an operation.' }],
     });
   });
 
@@ -748,10 +742,7 @@ describe('Execute: Handles basic execution tasks', () => {
       errors: [
         {
           message:
-            'Must provide operation name if query contains ' +
-            'multiple operations.',
-          locations: undefined,
-          path: undefined,
+            'Must provide operation name if query contains multiple operations.',
         },
       ],
     });
@@ -775,13 +766,7 @@ describe('Execute: Handles basic execution tasks', () => {
       operationName: 'UnknownExample',
     });
     expect(result).to.deep.equal({
-      errors: [
-        {
-          message: 'Unknown operation named "UnknownExample".',
-          locations: undefined,
-          path: undefined,
-        },
-      ],
+      errors: [{ message: 'Unknown operation named "UnknownExample".' }],
     });
   });
 
@@ -1046,17 +1031,19 @@ describe('Execute: Handles basic execution tasks', () => {
     };
     const result = await execute(schema, query, value);
 
-    expect(result.data).to.deep.equal({
-      specials: [{ value: 'foo' }, null],
-    });
-    expect(result.errors).to.have.lengthOf(1);
-    expect(result.errors).to.containSubset([
-      {
-        message:
-          'Expected value of type "SpecialType" but got: [object Object].',
-        locations: [{ line: 1, column: 3 }],
+    expect(result).to.deep.equal({
+      data: {
+        specials: [{ value: 'foo' }, null],
       },
-    ]);
+      errors: [
+        {
+          message:
+            'Expected value of type "SpecialType" but got: [object Object].',
+          locations: [{ line: 1, column: 3 }],
+          path: ['specials', 1],
+        },
+      ],
+    });
   });
 
   it('executes ignoring invalid non-executable definitions', async () => {

src/execution/__tests__/lists-test.js

@@ -7,7 +7,6 @@
 
 import { expect } from 'chai';
 import { describe, it } from 'mocha';
-import { formatError } from '../../error';
 import { execute } from '../execute';
 import { parse } from '../../language';
 import {
@@ -48,18 +47,7 @@ function check(testType, testData, expected) {
     const ast = parse('{ nest { test } }');
 
     const response = await execute(schema, ast, data);
-
-    // Formatting errors for ease of test writing.
-    let result;
-    if (response.errors) {
-      result = {
-        data: response.data,
-        errors: response.errors.map(formatError),
-      };
-    } else {
-      result = response;
-    }
-    expect(result).to.deep.equal(expected);
+    expect(response).to.deep.equal(expected);
   };
 }
 

src/execution/__tests__/mutations-test.js

@@ -167,33 +167,27 @@ describe('Execute: Handles mutation execution ordering', () => {
 
     const result = await execute(schema, parse(doc), new Root(6));
 
-    expect(result.data).to.deep.equal({
-      first: {
-        theNumber: 1,
-      },
-      second: {
-        theNumber: 2,
-      },
-      third: null,
-      fourth: {
-        theNumber: 4,
-      },
-      fifth: {
-        theNumber: 5,
-      },
-      sixth: null,
+    expect(result).to.deep.equal({
+      data: {
+        first: { theNumber: 1 },
+        second: { theNumber: 2 },
+        third: null,
+        fourth: { theNumber: 4 },
+        fifth: { theNumber: 5 },
+        sixth: null,
+      },
+      errors: [
+        {
+          message: 'Cannot change the number',
+          locations: [{ line: 8, column: 7 }],
+          path: ['third'],
+        },
+        {
+          message: 'Cannot change the number',
+          locations: [{ line: 17, column: 7 }],
+          path: ['sixth'],
+        },
+      ],
     });
-
-    expect(result.errors).to.have.length(2);
-    expect(result.errors).to.containSubset([
-      {
-        message: 'Cannot change the number',
-        locations: [{ line: 8, column: 7 }],
-      },
-      {
-        message: 'Cannot change the number',
-        locations: [{ line: 17, column: 7 }],
-      },
-    ]);
   });
 });

src/execution/__tests__/sync-test.js

@@ -10,6 +10,7 @@ import { describe, it } from 'mocha';
 import { graphqlSync } from '../../graphql';
 import { execute } from '../execute';
 import { parse } from '../../language';
+import { validate } from '../../validation/validate';
 import { GraphQLSchema, GraphQLObjectType, GraphQLString } from '../../type';
 
 describe('Execute: synchronously when possible', () => {
@@ -52,13 +53,7 @@ describe('Execute: synchronously when possible', () => {
       rootValue: 'rootValue',
     });
     expect(result).to.deep.equal({
-      errors: [
-        {
-          message: 'Must provide an operation.',
-          locations: undefined,
-          path: undefined,
-        },
-      ],
+      errors: [{ message: 'Must provide an operation.' }],
     });
   });
 
@@ -102,7 +97,7 @@ describe('Execute: synchronously when possible', () => {
         schema,
         source: doc,
       });
-      expect(result).to.containSubset({
+      expect(result).to.deep.equal({
         errors: [
           {
             message: 'Syntax Error: Expected Name, found {',
@@ -114,20 +109,12 @@ describe('Execute: synchronously when possible', () => {
 
     it('does not return a Promise for validation errors', () => {
       const doc = 'fragment Example on Query { unknownField }';
+      const validationErrors = validate(schema, parse(doc));
       const result = graphqlSync({
         schema,
         source: doc,
       });
-      expect(result).to.containSubset({
-        errors: [
-          {
-            message:
-              'Cannot query field "unknownField" on type "Query". Did you ' +
-              'mean "syncField" or "asyncField"?',
-            locations: [{ line: 1, column: 29 }],
-          },
-        ],
-      });
+      expect(result).to.deep.equal({ errors: validationErrors });
     });
 
     it('does not return a Promise for sync execution', () => {