From 686c0f1c4a1feb609e1584aa71738709bbbf984e Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Tue, 27 Jul 2021 16:58:08 +1000 Subject: [PATCH] Switch from apollo-errors to apollo-server-errors (#6200) --- .changeset/tidy-suits-refuse.md | 5 +++ packages/keystone/package.json | 2 +- .../keystone/src/lib/core/graphql-errors.ts | 31 ++++++------------- .../src/lib/core/mutations/access-control.ts | 16 +++++----- .../src/lib/core/mutations/validation.ts | 14 +++------ .../src/lib/core/queries/output-field.ts | 2 +- .../src/lib/core/queries/resolvers.ts | 15 ++++----- tests/api-tests/utils.ts | 3 ++ yarn.lock | 8 ----- 9 files changed, 38 insertions(+), 58 deletions(-) create mode 100644 .changeset/tidy-suits-refuse.md diff --git a/.changeset/tidy-suits-refuse.md b/.changeset/tidy-suits-refuse.md new file mode 100644 index 00000000000..88067ac37d5 --- /dev/null +++ b/.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`. \ No newline at end of file diff --git a/packages/keystone/package.json b/packages/keystone/package.json index f6c5a93e099..de9235bc39e 100644 --- a/packages/keystone/package.json +++ b/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", diff --git a/packages/keystone/src/lib/core/graphql-errors.ts b/packages/keystone/src/lib/core/graphql-errors.ts index ce42b9e1ee0..62fafbc0fe2 100644 --- a/packages/keystone/src/lib/core/graphql-errors.ts +++ b/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'); diff --git a/packages/keystone/src/lib/core/mutations/access-control.ts b/packages/keystone/src/lib/core/mutations/access-control.ts index 7d7db7d5513..51efe776857 100644 --- a/packages/keystone/src/lib/core/mutations/access-control.ts +++ b/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(); } } diff --git a/packages/keystone/src/lib/core/mutations/validation.ts b/packages/keystone/src/lib/core/mutations/validation.ts index 3503916f39f..6c5017d2db6 100644 --- a/packages/keystone/src/lib/core/mutations/validation.ts +++ b/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(); } } diff --git a/packages/keystone/src/lib/core/queries/output-field.ts b/packages/keystone/src/lib/core/queries/output-field.ts index 33ef64f6b20..c927730f07a 100644 --- a/packages/keystone/src/lib/core/queries/output-field.ts +++ b/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 diff --git a/packages/keystone/src/lib/core/queries/resolvers.ts b/packages/keystone/src/lib/core/queries/resolvers.ts index 73c36d81c14..35e421b4e26 100644 --- a/packages/keystone/src/lib/core/queries/resolvers.ts +++ b/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, diff --git a/tests/api-tests/utils.ts b/tests/api-tests/utils.ts index 5fef7c3b6bf..3436672c927 100644 --- a/tests/api-tests/utils.ts +++ b/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', })) diff --git a/yarn.lock b/yarn.lock index c5e0267538f..00bacc3ebf6 100644 --- a/yarn.lock +++ b/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"