Use extensionError for all extensions

.changeset/little-colts-deliver.md

@@ -0,0 +1,5 @@
+---
+'@keystone-next/keystone': patch
+---
+
+Improved the error messages provided from the GraphQL API when extension code (e.g access control functions, hooks, etc) throw exceptions.

packages/keystone/src/lib/core/access-control.ts

@@ -13,7 +13,7 @@ import {
   ListFilterAccessControl,
   KeystoneContext,
 } from '../../types';
-import { accessReturnError } from './graphql-errors';
+import { accessReturnError, extensionError } from './graphql-errors';
 import { InitialisedList } from './types-for-lists';
 import { InputFilter } from './where-inputs';
 
@@ -25,8 +25,15 @@ export async function getOperationAccess(
   const args = { operation, session: context.session, listKey: list.listKey, context };
   // Check the mutation access
   const access = list.access.operation[operation];
-  // @ts-ignore
-  const result = await access(args);
+  let result;
+  try {
+    // @ts-ignore
+    result = await access(args);
+  } catch (error: any) {
+    throw extensionError('Access control', [
+      { error, tag: `${list.listKey}.access.operation.${args.operation}` },
+    ]);
+  }
 
   const resultType = typeof result;
 
@@ -49,20 +56,14 @@ export async function getAccessFilters(
   const args = { operation, session: context.session, listKey: list.listKey, context };
   // Check the mutation access
   const access = list.access.filter[operation];
-  // @ts-ignore
-  return typeof access === 'function' ? await access(args) : access;
-}
-
-export async function validateFieldAccessControl<
-  Args extends { listKey: string; fieldKey: string; operation: 'read' | 'create' | 'update' }
->({
-  access,
-  args,
-}: {
-  access: ((args: Args) => boolean | Promise<boolean>) | boolean;
-  args: Args;
-}) {
-  return typeof access === 'function' ? await access(args) : access;
+  try {
+    // @ts-ignore
+    return typeof access === 'function' ? await access(args) : access;
+  } catch (error: any) {
+    throw extensionError('Access control', [
+      { error, tag: `${args.listKey}.access.filter.${args.operation}` },
+    ]);
+  }
 }
 
 export function parseFieldAccessControl(

packages/keystone/src/lib/core/filter-order-access.ts

@@ -1,14 +1,16 @@
 import { KeystoneContext } from '../../types';
-import { accessReturnError, filterAccessError } from './graphql-errors';
+import { accessReturnError, extensionError, filterAccessError } from './graphql-errors';
 import { InitialisedList } from './types-for-lists';
 
 export async function checkFilterOrderAccess(
   things: { fieldKey: string; list: InitialisedList }[],
   context: KeystoneContext,
   operation: 'filter' | 'orderBy'
 ) {
+  const func = operation === 'filter' ? 'isFilterable' : 'isOrderable';
   const failures: string[] = [];
   const returnTypeErrors: any[] = [];
+  const accessErrors: any[] = [];
   for (const { fieldKey, list } of things) {
     const field = list.fields[fieldKey];
     const rule = field.graphql.isEnabled[operation];
@@ -18,24 +20,28 @@ export async function checkFilterOrderAccess(
       throw new Error('Assert failed');
     } else if (typeof rule === 'function') {
       // Apply dynamic rules
-      const result = await rule({
-        context,
-        session: context.session,
-        listKey: list.listKey,
-        fieldKey,
-      });
+      let result;
+      try {
+        result = await rule({ context, session: context.session, listKey: list.listKey, fieldKey });
+      } catch (error: any) {
+        accessErrors.push({ error, tag: `${list.listKey}.${fieldKey}.${func}` });
+        continue;
+      }
       const resultType = typeof result;
 
       // It's important that we don't cast objects to truthy values, as there's a strong chance that the user
       // has made a mistake.
       if (resultType !== 'boolean') {
-        const func = operation === 'filter' ? 'isFilterable' : 'isOrderable';
         returnTypeErrors.push({ tag: `${list.listKey}.${fieldKey}.${func}`, returned: resultType });
       } else if (!result) {
         failures.push(`${list.listKey}.${fieldKey}`);
       }
     }
   }
+  if (accessErrors.length) {
+    throw extensionError(func, accessErrors);
+  }
+
   if (returnTypeErrors.length) {
     throw accessReturnError(returnTypeErrors);
   }

packages/keystone/src/lib/core/mutations/access-control.ts

@@ -1,6 +1,5 @@
 import { KeystoneContext } from '../../../types';
-import { validateFieldAccessControl } from '../access-control';
-import { accessDeniedError, accessReturnError } from '../graphql-errors';
+import { accessDeniedError, accessReturnError, extensionError } from '../graphql-errors';
 import { mapUniqueWhereToWhere } from '../queries/resolvers';
 import { InitialisedList } from '../types-for-lists';
 import { runWithPrisma } from '../utils';
@@ -52,7 +51,15 @@ export async function getAccessControlledItemForDelete(
   const args = { operation, session: context.session, listKey: list.listKey, context, item };
 
   // List level 'item' access control
-  const result = await access(args);
+  let result;
+  try {
+    result = await access(args);
+  } catch (error: any) {
+    throw extensionError('Access control', [
+      { error, tag: `${args.listKey}.access.item.${args.operation}` },
+    ]);
+  }
+
   const resultType = typeof result;
 
   // It's important that we don't cast objects to truthy values, as there's a strong chance that the user
@@ -102,7 +109,14 @@ export async function getAccessControlledItemForUpdate(
   };
 
   // List level 'item' access control
-  const result = await access(args);
+  let result;
+  try {
+    result = await access(args);
+  } catch (error: any) {
+    throw extensionError('Access control', [
+      { error, tag: `${args.listKey}.access.item.${args.operation}` },
+    ]);
+  }
   const resultType = typeof result;
 
   // It's important that we don't cast objects to truthy values, as there's a strong chance that the user
@@ -125,32 +139,37 @@ export async function getAccessControlledItemForUpdate(
   }
 
   // Field level 'item' access control
-  const fieldAccess = await Promise.all(
-    Object.keys(originalInput!).map(async fieldKey => [
-      fieldKey,
-      await validateFieldAccessControl({
-        access: list.fields[fieldKey].access[operation],
-        args: { ...args, fieldKey },
-      }),
-    ])
-  );
-
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  const nonBooleans = fieldAccess.filter(([_, result]) => typeof result !== 'boolean');
-  if (nonBooleans.length) {
-    throw accessReturnError(
-      nonBooleans.map(([fieldKey, result]) => ({
+  const nonBooleans = [];
+  const fieldsDenied = [];
+  const accessErrors = [];
+  for (const fieldKey of Object.keys(originalInput)) {
+    let result;
+    try {
+      result =
+        typeof list.fields[fieldKey].access[operation] === 'function'
+          ? await list.fields[fieldKey].access[operation]({ ...args, fieldKey })
+          : access;
+    } catch (error: any) {
+      accessErrors.push({ error, tag: `${args.listKey}.${fieldKey}.access.${args.operation}` });
+      continue;
+    }
+    if (typeof result !== 'boolean') {
+      nonBooleans.push({
         tag: `${args.listKey}.${fieldKey}.access.${args.operation}`,
         returned: typeof result,
-      }))
-    );
+      });
+    } else if (!result) {
+      fieldsDenied.push(fieldKey);
+    }
+  }
+
+  if (accessErrors.length) {
+    throw extensionError('Access control', accessErrors);
   }
 
-  const fieldsDenied = fieldAccess
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    .filter(([_, result]) => !result)
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    .map(([fieldKey, _]) => fieldKey);
+  if (nonBooleans.length) {
+    throw accessReturnError(nonBooleans);
+  }
 
   if (fieldsDenied.length) {
     throw accessDeniedError(
@@ -181,7 +200,15 @@ export async function applyAccessControlForCreate(
   };
 
   // List level 'item' access control
-  const result = await access(args);
+  let result;
+  try {
+    result = await access(args);
+  } catch (error: any) {
+    throw extensionError('Access control', [
+      { error, tag: `${args.listKey}.access.item.${args.operation}` },
+    ]);
+  }
+
   const resultType = typeof result;
 
   // It's important that we don't cast objects to truthy values, as there's a strong chance that the user
@@ -204,32 +231,38 @@ export async function applyAccessControlForCreate(
   }
 
   // Field level 'item' access control
-  const fieldAccess = await Promise.all(
-    Object.keys(originalInput!).map(async fieldKey => [
-      fieldKey,
-      await validateFieldAccessControl({
-        access: list.fields[fieldKey].access[operation],
-        args: { ...args, fieldKey },
-      }),
-    ])
-  );
-
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  const nonBooleans = fieldAccess.filter(([_, result]) => typeof result !== 'boolean');
-  if (nonBooleans.length) {
-    throw accessReturnError(
-      nonBooleans.map(([fieldKey, result]) => ({
+  // Field level 'item' access control
+  const nonBooleans = [];
+  const fieldsDenied = [];
+  const accessErrors = [];
+  for (const fieldKey of Object.keys(originalInput)) {
+    let result;
+    try {
+      result =
+        typeof list.fields[fieldKey].access[operation] === 'function'
+          ? await list.fields[fieldKey].access[operation]({ ...args, fieldKey })
+          : access;
+    } catch (error: any) {
+      accessErrors.push({ error, tag: `${args.listKey}.${fieldKey}.access.${args.operation}` });
+      continue;
+    }
+    if (typeof result !== 'boolean') {
+      nonBooleans.push({
         tag: `${args.listKey}.${fieldKey}.access.${args.operation}`,
         returned: typeof result,
-      }))
-    );
+      });
+    } else if (!result) {
+      fieldsDenied.push(fieldKey);
+    }
+  }
+
+  if (accessErrors.length) {
+    throw extensionError('Access control', accessErrors);
   }
 
-  const fieldsDenied = fieldAccess
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    .filter(([_, result]) => !result)
-    // eslint-disable-next-line @typescript-eslint/no-unused-vars
-    .map(([fieldKey, _]) => fieldKey);
+  if (nonBooleans.length) {
+    throw accessReturnError(nonBooleans);
+  }
 
   if (fieldsDenied.length) {
     throw accessDeniedError(

packages/keystone/src/lib/core/mutations/create-update.ts

@@ -328,7 +328,7 @@ async function getResolvedData(
           fieldKey,
         });
       } catch (error: any) {
-        fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}` });
+        fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.${hookName}` });
       }
     }
   }
@@ -342,7 +342,7 @@ async function getResolvedData(
     try {
       resolvedData = (await list.hooks.resolveInput({ ...hookArgs, resolvedData })) as any;
     } catch (error: any) {
-      throw extensionError(hookName, [{ error, tag: list.listKey }]);
+      throw extensionError(hookName, [{ error, tag: `${list.listKey}.hooks.${hookName}` }]);
     }
   }
 

packages/keystone/src/lib/core/mutations/hooks.ts

@@ -37,7 +37,7 @@ export async function runSideEffectOnlyHook<
       try {
         await field.hooks[hookName]?.({ fieldKey, ...args });
       } catch (error: any) {
-        fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}` });
+        fieldsErrors.push({ error, tag: `${list.listKey}.${fieldKey}.hooks.${hookName}` });
       }
     }
   }
@@ -49,6 +49,6 @@ export async function runSideEffectOnlyHook<
   try {
     await list.hooks[hookName]?.(args);
   } catch (error: any) {
-    throw extensionError(hookName, [{ error, tag: list.listKey }]);
+    throw extensionError(hookName, [{ error, tag: `${list.listKey}.hooks.${hookName}` }]);
   }
 }