Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jul 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/tidy-suits-refuse.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-next/keystone': patch
---

Updated internal error handling to use the `apollo-server-errors` package instead of `apollo-errors`.
2 changes: 1 addition & 1 deletion packages/keystone/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
31 changes: 10 additions & 21 deletions packages/keystone/src/lib/core/graphql-errors.ts
Original file line number Diff line number Diff line change
@@ -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');
16 changes: 8 additions & 8 deletions packages/keystone/src/lib/core/mutations/access-control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -68,7 +68,7 @@ export async function getAccessControlledItemForUpdate(
args,
});
if (accessControl === false) {
throw accessDeniedError('mutation');
throw accessDeniedError();
}

// List access: pass 2
Expand All @@ -82,7 +82,7 @@ export async function getAccessControlledItemForUpdate(
})
);
if (!item) {
throw accessDeniedError('mutation');
throw accessDeniedError();
}

// Field access
Expand All @@ -97,7 +97,7 @@ export async function getAccessControlledItemForUpdate(
);

if (results.some(canAccess => !canAccess)) {
throw accessDeniedError('mutation');
throw accessDeniedError();
}

return item;
Expand All @@ -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
Expand All @@ -134,7 +134,7 @@ export async function applyAccessControlForCreate(
);

if (results.some(canAccess => !canAccess)) {
throw accessDeniedError('mutation');
throw accessDeniedError();
}
}

Expand All @@ -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();
}
}
14 changes: 4 additions & 10 deletions packages/keystone/src/lib/core/mutations/validation.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ValidationFailureError } from '../graphql-errors';
import { validationFailureError } from '../graphql-errors';
import { InitialisedList } from '../types-for-lists';
import { promiseAllRejectWithAllErrors } from '../utils';

Expand All @@ -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();
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/keystone/src/lib/core/queries/output-field.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 6 additions & 9 deletions packages/keystone/src/lib/core/queries/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions tests/api-tests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}))
Expand All @@ -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',
}))
Expand Down Expand Up @@ -102,6 +104,7 @@ export const expectLimitsExceededError = (
}));
expect(unpackedErrors).toEqual(
args.map(({ path }) => ({
extensions: { code: undefined },
path,
message: 'Your request exceeded server limits',
}))
Expand Down
8 changes: 0 additions & 8 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down