refactor(errors): proxy all the errors

Previously, errors with invalid or missing paths were lost. See: #1641 https://github.com/apollographql/apollo-server/issues/4226 They are now saved! All correctly pathed errors are now inlined into the returned data by the CheckResultAndHandleErrors transform. The data is now annotated only with the remaining "unpathed" errors. These are then returned when possible if a null is encountered including the missing or potentially invalid path. Changes to error handling obviate some existing functions including getErrorsByPathSegment, getErrors in favor of getUnpathedErrors. Utility functions including slicedError and unreleased functions extendedError and unextendedError are no longer necessary.
ardatan · Oct 1, 2020 · a79ce8f · a79ce8f
1 parent 3cb3444
commit a79ce8f
Show file tree

Hide file tree

Showing 12 changed files with 269 additions and 198 deletions.
diff --git a/packages/delegate/src/defaultMergedResolver.ts b/packages/delegate/src/defaultMergedResolver.ts
@@ -4,7 +4,7 @@ import { getResponseKeyFromInfo } from '@graphql-tools/utils';
 
 import { resolveExternalValue } from './resolveExternalValue';
 import { getSubschema } from './Subschema';
-import { getErrors, isExternalData } from './externalData';
+import { getUnpathedErrors, isExternalData } from './externalData';
 import { ExternalData } from './types';
 
 /**
@@ -32,8 +32,8 @@ export function defaultMergedResolver(
   }
 
   const data = parent[responseKey];
+  const unpathedErrors = getUnpathedErrors(parent);
   const subschema = getSubschema(parent, responseKey);
-  const errors = getErrors(parent, responseKey);
 
-  return resolveExternalValue(data, errors, subschema, context, info);
+  return resolveExternalValue(data, unpathedErrors, subschema, context, info);
 }
diff --git a/packages/delegate/src/externalData.ts b/packages/delegate/src/externalData.ts
@@ -1,19 +1,12 @@
 import { GraphQLSchema, GraphQLError, GraphQLObjectType, SelectionSetNode } from 'graphql';
 
-import {
-  slicedError,
-  extendedError,
-  mergeDeep,
-  relocatedError,
-  GraphQLExecutionContext,
-  collectFields,
-} from '@graphql-tools/utils';
+import { mergeDeep, relocatedError, GraphQLExecutionContext, collectFields } from '@graphql-tools/utils';
 
 import { SubschemaConfig, ExternalData } from './types';
-import { OBJECT_SUBSCHEMA_SYMBOL, FIELD_SUBSCHEMA_MAP_SYMBOL, ERROR_SYMBOL } from './symbols';
+import { OBJECT_SUBSCHEMA_SYMBOL, FIELD_SUBSCHEMA_MAP_SYMBOL, UNPATHED_ERRORS_SYMBOL } from './symbols';
 
 export function isExternalData(data: any): data is ExternalData {
-  return data[ERROR_SYMBOL] !== undefined;
+  return data[UNPATHED_ERRORS_SYMBOL] !== undefined;
 }
 
 export function annotateExternalData(
@@ -24,7 +17,7 @@ export function annotateExternalData(
   Object.defineProperties(data, {
     [OBJECT_SUBSCHEMA_SYMBOL]: { value: subschema },
     [FIELD_SUBSCHEMA_MAP_SYMBOL]: { value: Object.create(null) },
-    [ERROR_SYMBOL]: { value: errors },
+    [UNPATHED_ERRORS_SYMBOL]: { value: errors },
   });
   return data;
 }
@@ -33,39 +26,8 @@ export function getSubschema(data: ExternalData, responseKey: string): GraphQLSc
   return data[FIELD_SUBSCHEMA_MAP_SYMBOL][responseKey] ?? data[OBJECT_SUBSCHEMA_SYMBOL];
 }
 
-export function getErrors(data: ExternalData, pathSegment: string): Array<GraphQLError> {
-  const errors = data == null ? data : data[ERROR_SYMBOL];
-
-  if (!Array.isArray(errors)) {
-    return null;
-  }
-
-  const fieldErrors = [];
-
-  for (const error of errors) {
-    if (!error.path || error.path[0] === pathSegment) {
-      fieldErrors.push(error);
-    }
-  }
-
-  return fieldErrors;
-}
-
-export function getErrorsByPathSegment(errors: ReadonlyArray<GraphQLError>): Record<string, Array<GraphQLError>> {
-  const record = Object.create(null);
-  errors.forEach(error => {
-    if (!error.path || error.path.length < 2) {
-      return;
-    }
-
-    const pathSegment = error.path[1];
-
-    const current = pathSegment in record ? record[pathSegment] : [];
-    current.push(slicedError(error));
-    record[pathSegment] = current;
-  });
-
-  return record;
+export function getUnpathedErrors(data: ExternalData): Array<GraphQLError> {
+  return data[UNPATHED_ERRORS_SYMBOL];
 }
 
 export function mergeExternalData(
@@ -95,12 +57,11 @@ export function mergeExternalData(
       );
       const nullResult = {};
       Object.keys(fieldNodes).forEach(responseKey => {
-        errors.push(relocatedError(source, [responseKey]));
-        nullResult[responseKey] = null;
+        nullResult[responseKey] = relocatedError(source, path.concat([responseKey]));
       });
       results.push(nullResult);
     } else {
-      errors = errors.concat(source[ERROR_SYMBOL]);
+      errors = errors.concat(source[UNPATHED_ERRORS_SYMBOL]);
       results.push(source);
     }
   });
@@ -118,14 +79,7 @@ export function mergeExternalData(
     ? Object.assign({}, target[FIELD_SUBSCHEMA_MAP_SYMBOL], fieldSubschemaMap)
     : fieldSubschemaMap;
 
-  const annotatedErrors = errors.map(error => {
-    return extendedError(error, {
-      ...error.extensions,
-      graphQLToolsMergedPath: error.path != null ? [...path, ...error.path] : path,
-    });
-  });
-
-  result[ERROR_SYMBOL] = target[ERROR_SYMBOL].concat(annotatedErrors);
+  result[UNPATHED_ERRORS_SYMBOL] = target[UNPATHED_ERRORS_SYMBOL].concat(errors);
 
   return result;
 }
diff --git a/packages/delegate/src/resolveExternalValue/handleList.ts b/packages/delegate/src/resolveExternalValue/handleList.ts
@@ -11,27 +11,24 @@ import {
 } from 'graphql';
 
 import { SubschemaConfig } from '../types';
-import { getErrorsByPathSegment } from '../externalData';
 
 import { handleNull } from './handleNull';
 import { handleObject } from './handleObject';
 
 export function handleList(
   type: GraphQLList<any>,
   list: Array<any>,
-  errors: ReadonlyArray<GraphQLError>,
+  unpathedErrors: Array<GraphQLError>,
   subschema: GraphQLSchema | SubschemaConfig,
   context: Record<string, any>,
   info: GraphQLResolveInfo,
   skipTypeMerging?: boolean
 ) {
-  const childErrors = getErrorsByPathSegment(errors);
-
-  return list.map((listMember, index) =>
+  return list.map(listMember =>
     handleListMember(
       getNullableType(type.ofType),
       listMember,
-      index in childErrors ? childErrors[index] : [],
+      unpathedErrors,
       subschema,
       context,
       info,
@@ -43,21 +40,25 @@ export function handleList(
 function handleListMember(
   type: GraphQLType,
   listMember: any,
-  errors: ReadonlyArray<GraphQLError>,
+  unpathedErrors: Array<GraphQLError>,
   subschema: GraphQLSchema | SubschemaConfig,
   context: Record<string, any>,
   info: GraphQLResolveInfo,
   skipTypeMerging?: boolean
 ): any {
+  if (listMember instanceof Error) {
+    return listMember;
+  }
+
   if (listMember == null) {
-    return handleNull(errors);
+    return handleNull(unpathedErrors);
   }
 
   if (isLeafType(type)) {
     return type.parseValue(listMember);
   } else if (isCompositeType(type)) {
-    return handleObject(type, listMember, errors, subschema, context, info, skipTypeMerging);
+    return handleObject(type, listMember, unpathedErrors, subschema, context, info, skipTypeMerging);
   } else if (isListType(type)) {
-    return handleList(type, listMember, errors, subschema, context, info, skipTypeMerging);
+    return handleList(type, listMember, unpathedErrors, subschema, context, info, skipTypeMerging);
   }
 }
diff --git a/packages/delegate/src/resolveExternalValue/handleNull.ts b/packages/delegate/src/resolveExternalValue/handleNull.ts
@@ -1,28 +1,27 @@
-import { GraphQLError } from 'graphql';
+import { GraphQLError, locatedError } from 'graphql';
 
 import AggregateError from '@ardatan/aggregate-error';
 
-import { relocatedError, unextendedError } from '@graphql-tools/utils';
+const reportedErrors: WeakMap<GraphQLError, boolean> = new Map();
 
-export function handleNull(errors: ReadonlyArray<GraphQLError>) {
-  if (errors.length) {
-    const graphQLToolsMergedPath = errors[0].extensions.graphQLToolsMergedPath;
-    const unannotatedErrors = errors.map(error => unextendedError(error, 'graphQLToolsMergedPath'));
+export function handleNull(unpathedErrors: Array<GraphQLError>) {
+  if (unpathedErrors.length) {
+    const unreportedErrors: Array<GraphQLError> = [];
+    unpathedErrors.forEach(error => {
+      if (!reportedErrors.has(error)) {
+        unreportedErrors.push(error);
+        reportedErrors.set(error, true);
+      }
+    });
 
-    if (unannotatedErrors.length > 1) {
-      const combinedError = new AggregateError(unannotatedErrors);
-      return new GraphQLError(
-        combinedError.message,
-        undefined,
-        undefined,
-        undefined,
-        graphQLToolsMergedPath,
-        combinedError
-      );
-    }
+    if (unreportedErrors.length) {
+      if (unreportedErrors.length === 1) {
+        return unreportedErrors[0];
+      }
 
-    const error = unannotatedErrors[0];
-    return relocatedError(error, graphQLToolsMergedPath);
+      const combinedError = new AggregateError(unreportedErrors);
+      return locatedError(combinedError, undefined, unreportedErrors[0].path);
+    }
   }
 
   return null;

diff --git a/packages/delegate/src/resolveExternalValue/handleObject.ts b/packages/delegate/src/resolveExternalValue/handleObject.ts
@@ -1,7 +1,5 @@
 import { GraphQLCompositeType, GraphQLError, GraphQLSchema, isAbstractType, GraphQLResolveInfo } from 'graphql';
 
-import { slicedError } from '@graphql-tools/utils';
-
 import { SubschemaConfig } from '../types';
 import { annotateExternalData } from '../externalData';
 
@@ -11,19 +9,15 @@ import { getFieldsNotInSubschema } from './getFieldsNotInSubschema';
 export function handleObject(
   type: GraphQLCompositeType,
   object: any,
-  errors: ReadonlyArray<GraphQLError>,
+  unpathedErrors: Array<GraphQLError>,
   subschema: GraphQLSchema | SubschemaConfig,
   context: Record<string, any>,
   info: GraphQLResolveInfo,
   skipTypeMerging?: boolean
 ) {
   const stitchingInfo = info?.schema.extensions?.stitchingInfo;
 
-  annotateExternalData(
-    object,
-    errors.map(error => slicedError(error)),
-    subschema
-  );
+  annotateExternalData(object, unpathedErrors, subschema);
 
   if (skipTypeMerging || !stitchingInfo) {
     return object;

diff --git a/packages/delegate/src/resolveExternalValue/index.ts b/packages/delegate/src/resolveExternalValue/index.ts
@@ -6,51 +6,38 @@ import {
   isListType,
   GraphQLError,
   GraphQLSchema,
-  responsePathAsArray,
 } from 'graphql';
 
 import { SubschemaConfig } from '../types';
 
 import { handleNull } from './handleNull';
 import { handleObject } from './handleObject';
 import { handleList } from './handleList';
-import { extendedError } from '@graphql-tools/utils';
 
 export function resolveExternalValue(
   result: any,
-  errors: ReadonlyArray<GraphQLError>,
+  unpathedErrors: Array<GraphQLError>,
   subschema: GraphQLSchema | SubschemaConfig,
   context: Record<string, any>,
   info: GraphQLResolveInfo,
   returnType = info.returnType,
   skipTypeMerging?: boolean
 ): any {
-  const annotatedErrors = errors.map(error => {
-    if (error.extensions?.graphQLToolsMergedPath == null) {
-      return extendedError(error, {
-        ...error.extensions,
-        graphQLToolsMergedPath:
-          info == null
-            ? error.path
-            : error.path != null
-            ? [...responsePathAsArray(info.path), ...error.path.slice(1)]
-            : responsePathAsArray(info.path),
-      });
-    }
-    return error;
-  });
-
   const type = getNullableType(returnType);
 
+  if (result instanceof Error) {
+    return result;
+  }
+
   if (result == null) {
-    return handleNull(annotatedErrors);
+    return handleNull(unpathedErrors);
   }
 
   if (isLeafType(type)) {
     return type.parseValue(result);
   } else if (isCompositeType(type)) {
-    return handleObject(type, result, annotatedErrors, subschema, context, info, skipTypeMerging);
+    return handleObject(type, result, unpathedErrors, subschema, context, info, skipTypeMerging);
   } else if (isListType(type)) {
-    return handleList(type, result, annotatedErrors, subschema, context, info, skipTypeMerging);
+    return handleList(type, result, unpathedErrors, subschema, context, info, skipTypeMerging);
   }
 }
diff --git a/packages/delegate/src/symbols.ts b/packages/delegate/src/symbols.ts
@@ -1,3 +1,3 @@
-export const ERROR_SYMBOL = Symbol('subschemaErrors');
+export const UNPATHED_ERRORS_SYMBOL = Symbol('subschemaErrors');
 export const OBJECT_SUBSCHEMA_SYMBOL = Symbol('initialSubschema');
 export const FIELD_SUBSCHEMA_MAP_SYMBOL = Symbol('subschemaMap');