Switch from apollo-errors to apollo-server-errors (#6200)

.changeset/tidy-suits-refuse.md

@@ -0,0 +1,5 @@
+---
+'@keystone-next/keystone': patch
+---
+
+Updated internal error handling to use the `apollo-server-errors` package instead of `apollo-errors`.

packages/keystone/package.json

@@ -65,7 +65,7 @@
     "@types/source-map-support": "^0.5.4",
     "@types/uid-safe": "^2.1.2",
     "@types/uuid": "^8.3.1",
-    "apollo-errors": "^1.9.0",
+    "apollo-server-errors": "^2.5.0",
     "apollo-server-express": "^2.25.2",
     "apollo-server-micro": "^2.25.2",
     "apollo-server-types": "^0.9.0",

packages/keystone/src/lib/core/graphql-errors.ts

@@ -1,23 +1,12 @@
-import { createError } from 'apollo-errors';
+import { ApolloError } from 'apollo-server-errors';
 
-export const AccessDeniedError = createError('AccessDeniedError', {
-  message: 'You do not have access to this resource',
-  options: { showPath: true },
-});
-export const ValidationFailureError = createError('ValidationFailureError', {
-  message: 'You attempted to perform an invalid mutation',
-  options: { showPath: true },
-});
-export const LimitsExceededError = createError('LimitsExceededError', {
-  message: 'Your request exceeded server limits',
-  options: { showPath: true },
-});
+export const accessDeniedError = () => new ApolloError('You do not have access to this resource');
 
-export const accessDeniedError = (
-  type: 'query' | 'mutation',
-  target?: string,
-  internalData = {},
-  extraData = {}
-) => {
-  return new AccessDeniedError({ data: { type, target, ...extraData }, internalData });
-};
+export const validationFailureError = () =>
+  new ApolloError('You attempted to perform an invalid mutation');
+
+// FIXME: In an upcoming PR we will use these args to construct a better
+// error message, so leaving the, here for now. - TL
+// eslint-disable-next-line @typescript-eslint/no-unused-vars
+export const limitsExceededError = (args: { type: string; limit: number; list: string }) =>
+  new ApolloError('Your request exceeded server limits');

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

@@ -29,7 +29,7 @@ export async function getAccessControlledItemForDelete(
     args: { context, listKey: list.listKey, operation: 'delete', session: context.session, itemId },
   });
   if (access === false) {
-    throw accessDeniedError('mutation');
+    throw accessDeniedError();
   }
 
   // List access: pass 2
@@ -39,7 +39,7 @@ export async function getAccessControlledItemForDelete(
   }
   const item = await runWithPrisma(context, list, model => model.findFirst({ where }));
   if (item === null) {
-    throw accessDeniedError('mutation');
+    throw accessDeniedError();
   }
 
   return item;
@@ -68,7 +68,7 @@ export async function getAccessControlledItemForUpdate(
     args,
   });
   if (accessControl === false) {
-    throw accessDeniedError('mutation');
+    throw accessDeniedError();
   }
 
   // List access: pass 2
@@ -82,7 +82,7 @@ export async function getAccessControlledItemForUpdate(
     })
   );
   if (!item) {
-    throw accessDeniedError('mutation');
+    throw accessDeniedError();
   }
 
   // Field access
@@ -97,7 +97,7 @@ export async function getAccessControlledItemForUpdate(
   );
 
   if (results.some(canAccess => !canAccess)) {
-    throw accessDeniedError('mutation');
+    throw accessDeniedError();
   }
 
   return item;
@@ -119,7 +119,7 @@ export async function applyAccessControlForCreate(
   // List access
   const result = await validateCreateListAccessControl({ access: list.access.create, args });
   if (!result) {
-    throw accessDeniedError('mutation');
+    throw accessDeniedError();
   }
 
   // Field access
@@ -134,7 +134,7 @@ export async function applyAccessControlForCreate(
   );
 
   if (results.some(canAccess => !canAccess)) {
-    throw accessDeniedError('mutation');
+    throw accessDeniedError();
   }
 }
 
@@ -150,6 +150,6 @@ async function getStringifiedItemIdFromUniqueWhereInput(
     const item = await context.sudo().lists[listKey].findOne({ where: uniqueInput });
     return item.id;
   } catch (err) {
-    throw accessDeniedError('mutation');
+    throw accessDeniedError();
   }
 }

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

@@ -1,4 +1,4 @@
-import { ValidationFailureError } from '../graphql-errors';
+import { validationFailureError } from '../graphql-errors';
 import { InitialisedList } from '../types-for-lists';
 import { promiseAllRejectWithAllErrors } from '../utils';
 
@@ -19,15 +19,9 @@ export async function validationHook(
   });
 
   if (errors.length) {
-    throw new ValidationFailureError({
-      data: {
-        messages: errors.map(e => e.msg),
-        errors: errors.map(e => e.data),
-        listKey,
-        operation,
-      },
-      internalData: { errors: errors.map(e => e.internalData), data: originalInput },
-    });
+    // FIXME: We will incorporate the `msg` values, which are currently being lost
+    // into the error message in an upcoming change. -TL
+    throw validationFailureError();
   }
 }
 

packages/keystone/src/lib/core/queries/output-field.ts

@@ -107,7 +107,7 @@ export function outputTypeField(
         // If the client handles errors correctly, it should be able to
         // receive partial data (for the fields the user has access to),
         // and then an `errors` array of AccessDeniedError's
-        throw accessDeniedError('query', fieldKey, { itemId: rootVal.id });
+        throw accessDeniedError();
       }
 
       // Only static cache hints are supported at the field level until a use-case makes it clear what parameters a dynamic hint would take

packages/keystone/src/lib/core/queries/resolvers.ts

@@ -13,7 +13,7 @@ import {
   resolveWhereInput,
   UniqueInputFilter,
 } from '../where-inputs';
-import { accessDeniedError, LimitsExceededError } from '../graphql-errors';
+import { accessDeniedError, limitsExceededError } from '../graphql-errors';
 import { InitialisedList } from '../types-for-lists';
 import { getDBFieldKeyForFieldOnMultiField, runWithPrisma } from '../utils';
 
@@ -54,7 +54,7 @@ export async function accessControlledFilter(
     args: { context, listKey: list.listKey, operation: 'read', session: context.session },
   });
   if (access === false) {
-    throw accessDeniedError('query');
+    throw accessDeniedError();
   }
 
   // Merge declarative access control
@@ -80,7 +80,7 @@ export async function findOne(
   const item = await runWithPrisma(context, list, model => model.findFirst({ where: filter }));
 
   if (item === null) {
-    throw accessDeniedError('query');
+    throw accessDeniedError();
   }
   return item;
 }
@@ -186,9 +186,6 @@ export async function count(
   return count;
 }
 
-const limitsExceedError = (args: { type: string; limit: number; list: string }) =>
-  new LimitsExceededError({ data: args });
-
 function applyEarlyMaxResults(_first: number | null | undefined, list: InitialisedList) {
   const first = _first ?? Infinity;
   // We want to help devs by failing fast and noisily if limits are violated.
@@ -199,18 +196,18 @@ function applyEarlyMaxResults(_first: number | null | undefined, list: Initialis
   // * The query explicitly has a "first" that exceeds the limit
   // * The query has no "first", and has more results than the limit
   if (first < Infinity && first > list.maxResults) {
-    throw limitsExceedError({ list: list.listKey, type: 'maxResults', limit: list.maxResults });
+    throw limitsExceededError({ list: list.listKey, type: 'maxResults', limit: list.maxResults });
   }
 }
 
 function applyMaxResults(results: unknown[], list: InitialisedList, context: KeystoneContext) {
   if (results.length > list.maxResults) {
-    throw limitsExceedError({ list: list.listKey, type: 'maxResults', limit: list.maxResults });
+    throw limitsExceededError({ list: list.listKey, type: 'maxResults', limit: list.maxResults });
   }
   if (context) {
     context.totalResults += Array.isArray(results) ? results.length : 1;
     if (context.totalResults > context.maxTotalResults) {
-      throw limitsExceedError({
+      throw limitsExceededError({
         list: list.listKey,
         type: 'maxTotalResults',
         limit: context.maxTotalResults,

tests/api-tests/utils.ts

@@ -58,6 +58,7 @@ export const expectAccessDenied = (
   }));
   expect(unpackedErrors).toEqual(
     args.map(({ path }) => ({
+      extensions: { code: undefined },
       path,
       message: 'You do not have access to this resource',
     }))
@@ -73,6 +74,7 @@ export const expectValidationError = (
   }));
   expect(unpackedErrors).toEqual(
     args.map(({ path }) => ({
+      extensions: { code: undefined },
       path,
       message: 'You attempted to perform an invalid mutation',
     }))
@@ -102,6 +104,7 @@ export const expectLimitsExceededError = (
   }));
   expect(unpackedErrors).toEqual(
     args.map(({ path }) => ({
+      extensions: { code: undefined },
       path,
       message: 'Your request exceeded server limits',
     }))

yarn.lock

@@ -3551,14 +3551,6 @@ apollo-env@^0.10.0:
     node-fetch "^2.6.1"
     sha.js "^2.4.11"
 
-apollo-errors@^1.9.0:
-  version "1.9.0"
-  resolved "https://registry.yarnpkg.com/apollo-errors/-/apollo-errors-1.9.0.tgz#f1ed0ca0a6be5cd2f24e2eaa7b0860a10146ff51"
-  integrity sha512-XVukHd0KLvgY6tNjsPS3/Re3U6RQlTKrTbIpqqeTMo2N34uQMr+H1UheV21o8hOZBAFosvBORVricJiP5vfmrw==
-  dependencies:
-    assert "^1.4.1"
-    extendable-error "^0.1.5"
-
 apollo-graphql@^0.9.0:
   version "0.9.3"
   resolved "https://registry.yarnpkg.com/apollo-graphql/-/apollo-graphql-0.9.3.tgz#1ca6f625322ae10a66f57a39642849a07a7a5dc9"