GraphQLError: fix empty locations if error got nodes without locations

Backport of #3325
graphql · Oct 26, 2021 · 73f1adf · 73f1adf
1 parent 283c797
commit 73f1adf
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 42 deletions.
diff --git a/src/error/GraphQLError.js b/src/error/GraphQLError.js
@@ -4,7 +4,7 @@
 import isObjectLike from '../jsutils/isObjectLike';
 import { SYMBOL_TO_STRING_TAG } from '../polyfills/symbols';
 
-import type { ASTNode } from '../language/ast';
+import type { ASTNode, Location } from '../language/ast';
 import type { Source } from '../language/source';
 import type { SourceLocation } from '../language/location';
 import { getLocation } from '../language/location';
@@ -87,51 +87,33 @@ export class GraphQLError extends Error {
     super(message);
 
     // Compute list of blame nodes.
-    const _nodes = Array.isArray(nodes)
-      ? nodes.length !== 0
-        ? nodes
-        : undefined
-      : nodes
-      ? [nodes]
-      : undefined;
+    const _nodes = undefinedIfEmpty(
+      Array.isArray(nodes) ? nodes : nodes ? [nodes] : undefined,
+    );
+
+    let nodeLocations = [];
+    for (const { loc } of (_nodes ?? [])) {
+      if (loc != null) {
+        nodeLocations.push(loc);
+      }
+    }
+    nodeLocations = undefinedIfEmpty(nodeLocations);
 
     // Compute locations in the source for the given nodes/positions.
-    let _source = source;
-    if (!_source && _nodes) {
-      _source = _nodes[0].loc?.source;
-    }
+    const _source = source ?? nodeLocations?.[0].source;
 
-    let _positions = positions;
-    if (!_positions && _nodes) {
-      _positions = _nodes.reduce((list, node) => {
-        if (node.loc) {
-          list.push(node.loc.start);
-        }
-        return list;
-      }, []);
-    }
-    if (_positions && _positions.length === 0) {
-      _positions = undefined;
-    }
+    const _positions = positions ?? nodeLocations?.map((loc) => loc.start);
 
-    let _locations;
-    if (positions && source) {
-      _locations = positions.map((pos) => getLocation(source, pos));
-    } else if (_nodes) {
-      _locations = _nodes.reduce((list, node) => {
-        if (node.loc) {
-          list.push(getLocation(node.loc.source, node.loc.start));
-        }
-        return list;
-      }, []);
-    }
+    const _locations =
+      positions && source
+        ? positions.map((pos) => getLocation(source, pos))
+        : nodeLocations?.map((loc) => getLocation(loc.source, loc.start));
 
-    let _extensions = extensions;
-    if (_extensions == null && originalError != null) {
-      const originalExtensions = originalError.extensions;
-      if (isObjectLike(originalExtensions)) {
-        _extensions = originalExtensions;
-      }
+    let _extensions = extensions ?? undefined;
+
+    let originalExtensions = originalError?.extensions;
+    if (isObjectLike(originalExtensions)) {
+      _extensions = originalExtensions;
     }
 
     Object.defineProperties((this: any), {
@@ -218,6 +200,12 @@ export class GraphQLError extends Error {
   }
 }
 
+function undefinedIfEmpty<T>(
+  array: $ReadOnlyArray<T> | void,
+): $ReadOnlyArray<T> | void {
+  return array === undefined || array.length === 0 ? undefined : array;
+}
+
 /**
  * Prints a GraphQLError to a string, representing useful location information
  * about the error's position in the source.

diff --git a/src/error/__tests__/GraphQLError-test.js b/src/error/__tests__/GraphQLError-test.js
@@ -96,6 +96,18 @@ describe('GraphQLError', () => {
     });
   });
 
+  it('converts node without location to undefined source, positions and locations', () => {
+    const documentNode = parse('{ foo }', { noLocation: true });
+
+    const e = new GraphQLError('msg', documentNode);
+    expect(e).to.deep.include({
+      nodes: [documentNode],
+      source: undefined,
+      positions: undefined,
+      locations: undefined,
+    });
+  });
+
   it('converts source and positions to locations', () => {
     const e = new GraphQLError('msg', null, source, [6]);
     expect(e).to.have.property('source', source);

diff --git a/src/validation/__tests__/validation-test.js b/src/validation/__tests__/validation-test.js
@@ -136,7 +136,6 @@ describe('Validate: Limit maximum number of validation errors', () => {
   function invalidFieldError(fieldName: string) {
     return {
       message: `Cannot query field "${fieldName}" on type "QueryRoot".`,
-      locations: [],
     };
   }