Fixes #905

CHANGELOG.md

@@ -2,7 +2,12 @@
 
 ### vNEXT
 
-* ...
+* Do not annotate error symbol on primitive value(s)
+  [Issue #905](https://github.com/apollographql/graphql-tools/issues/905)
+  [PR #907](https://github.com/apollographql/graphql-tools/pull/907)
+* Use provided error message in the result for the same key and same path on resolve
+  [Issue #905](https://github.com/apollographql/graphql-tools/issues/905)
+  [PR #907](https://github.com/apollographql/graphql-tools/pull/907)
 
 ### v3.1.1
 

src/Interfaces.ts

@@ -70,11 +70,11 @@ export type MergeInfo = {
     info: GraphQLResolveInfo,
     transforms?: Array<Transform>,
   ) => any;
-  delegateToSchema<TContext>(options: IDelegateToSchemaOptions<TContext>): any;
   fragments: Array<{
     field: string;
     fragment: string;
   }>;
+  delegateToSchema<TContext>(options: IDelegateToSchemaOptions<TContext>): any;
 };
 
 export type IFieldResolver<TSource, TContext> = (

src/mock.ts

@@ -212,8 +212,8 @@ function addMockFunctionsToSchema({
       // we have to handle the root mutation and root query types differently,
       // because no resolver is called at the root.
       /* istanbul ignore next: Must provide schema DefinitionNode with query type or a type named Query. */
-      const isOnQueryType: boolean = schema.getQueryType() && schema.getQueryType().name === typeName
-      const isOnMutationType: boolean = schema.getMutationType() && schema.getMutationType().name === typeName
+      const isOnQueryType: boolean = schema.getQueryType() && schema.getQueryType().name === typeName;
+      const isOnMutationType: boolean = schema.getMutationType() && schema.getMutationType().name === typeName;
 
       if (isOnQueryType || isOnMutationType) {
         if (mockFunctionMap.has(typeName)) {

src/stitching/errors.ts

@@ -1,4 +1,4 @@
-import { GraphQLResolveInfo, responsePathAsArray, ExecutionResult, GraphQLFormattedError } from 'graphql';
+import { GraphQLResolveInfo, responsePathAsArray, ExecutionResult, GraphQLFormattedError, GraphQLError } from 'graphql';
 import { locatedError } from 'graphql/error';
 import { getResponseKeyFromInfo } from './getResponseKeyFromInfo';
 
@@ -35,29 +35,32 @@ export function annotateWithChildrenErrors(object: any, childrenErrors: Array<Gr
     });
 
     return object.map((item, index) => annotateWithChildrenErrors(item, byIndex[index]));
+  } else if (typeof (object) === 'object') {
+    // decorate the children object
+    return {
+      ...object,
+      [ERROR_SYMBOL]: childrenErrors.map(error => ({
+        ...error,
+        ...(error.path ? { path: error.path.slice(1) } : {}),
+      }))
+    };
   }
-
-  return {
-    ...object,
-    [ERROR_SYMBOL]: childrenErrors.map(error => ({
-      ...error,
-      ...(error.path ? { path: error.path.slice(1) } : {})
-    }))
-  };
+  // return this value anyway, either it is a primitive value or there is nothing wrong at all
+  return object;
 }
 
 export function getErrorsFromParent(
   object: any,
   fieldName: string
 ):
   | {
-      kind: 'OWN';
-      error: any;
-    }
+    kind: 'OWN';
+    error: any;
+  }
   | {
-      kind: 'CHILDREN';
-      errors?: Array<GraphQLFormattedError>;
-    } {
+    kind: 'CHILDREN';
+    errors?: Array<GraphQLFormattedError>;
+  } {
   const errors = (object && object[ERROR_SYMBOL]) || [];
   const childrenErrors: Array<GraphQLFormattedError> = [];
 
@@ -78,10 +81,10 @@ export function getErrorsFromParent(
   };
 }
 
-class CombinedError extends Error {
+class ResultError extends Error {
   public errors: Error[];
-  constructor(message: string, errors: Error[]) {
-    super(message);
+  constructor(error: Error, errors: Error[]) {
+    super(error.message);
     this.errors = errors;
   }
 }
@@ -96,17 +99,33 @@ export function checkResultAndHandleErrors(
   }
 
   if (result.errors && (!result.data || result.data[responseKey] == null)) {
-    // apollo-link-http & http-link-dataloader need the
+    // both apollo-link-http & http-link-dataloader needed the
     // result property to be passed through for better error handling.
-    // If there is only one error, which contains a result property, pass the error through
-    const newError =
-      result.errors.length === 1 && hasResult(result.errors[0])
-        ? result.errors[0]
-        : new CombinedError(concatErrors(result.errors), result.errors);
 
-    throw locatedError(newError, info.fieldNodes, responsePathAsArray(info.path));
-  }
+    let newError: Error = null; // Error instance that will be promoted later for a located error
+
+    const currentPath = responsePathAsArray(info.path);
+    // If there is only one error, which contains a result property
+    if (result.errors.length === 1 && hasResult(result.errors[0])) {
+      // Pass the error through
+      newError = result.errors[0];
+    } else {
+      // If an error path exists, the result is probably coming from a remote schema/Apollo server,
+      // so use the provided error in the result instead
+      const joinedCurrentPath = currentPath.join('.'); // Cache the joined path for comparison
+      const originalError = result.errors.find(
+        (error: GraphQLError) => error.path && error.path.join('.') === joinedCurrentPath
+      );
+      newError = new ResultError(
+        !!originalError // If we do have an error that matches the path of the current key
+          ? originalError // Pass the original error
+          : new Error(concatErrors(result.errors))  // otherwise fallback to concancate the error
+        , result.errors
+      );
+    }
 
+    throw locatedError(newError, info.fieldNodes, currentPath);
+  }
   let resultObject = result.data[responseKey];
   if (result.errors) {
     resultObject = annotateWithChildrenErrors(resultObject, result.errors as Array<GraphQLFormattedError>);

src/test/testErrors.ts

@@ -1,5 +1,5 @@
 import { assert } from 'chai';
-import { GraphQLResolveInfo } from 'graphql';
+import { GraphQLResolveInfo, ExecutionResult, GraphQLError } from 'graphql';
 import { checkResultAndHandleErrors, getErrorsFromParent, ERROR_SYMBOL } from '../stitching/errors';
 
 import 'mocha';
@@ -77,5 +77,57 @@ describe('Errors', () => {
         });
       }
     });
+
+    it('should not join error when corresponding path exists', () => {
+      const result: ExecutionResult = {
+        data: {
+          'a': null as any,
+          'b': null as any
+        },
+        errors: [
+          new GraphQLError('Error1', null, null, null, ['a']),
+          new GraphQLError('Error2', null, null, null, ['b'])
+        ]
+      };
+
+      const checkErrorTemplate = (key: string, expectedEntry: any) => {
+        try {
+          checkResultAndHandleErrors(result, { path: { key } } as any, key);
+        } catch (e) {
+          assert.equal(e.message, expectedEntry.message);
+          assert.deepEqual(e.path, expectedEntry.path);
+          assert.isNotEmpty(e.originalError);
+          assert.isNotEmpty(e.originalError.errors);
+          assert.lengthOf(e.originalError.errors, result.errors.length);
+          result.errors.forEach((error, i) => {
+            assert.deepEqual(e.originalError.errors[i], error);
+          });
+        }
+      };
+
+      checkErrorTemplate('a', result.errors[0]);
+      checkErrorTemplate('b', result.errors[1]);
+    });
+
+    it('should not taint primitive values on error', () => {
+      const result: ExecutionResult = {
+        data: {
+          'a': 'hello world',
+          'b': 123,
+          'c': true,
+          'd': null as any
+        },
+        errors: [
+          new GraphQLError('Error', null, null, null, ['d'])
+        ]
+      };
+
+      const checkValueOnResult =
+        (key: any) => checkResultAndHandleErrors(result, {} as GraphQLResolveInfo, key) === result.data[key];
+
+      assert.isTrue(checkValueOnResult('a'));
+      assert.isTrue(checkValueOnResult('b'));
+      assert.isTrue(checkValueOnResult('c'));
+    });
   });
 });