Skip to content

Commit

Permalink
Fix negative take values above the list's `graphql.queryLimits.maxR…
Browse files Browse the repository at this point in the history
…esults` not causing an error before getting the values from the database (#6392)
  • Loading branch information
emmatown authored Aug 23, 2021
1 parent 489e128 commit bd120c7
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/hungry-books-help.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@keystone-next/keystone': patch
---

Fixed negative `take` values above the list's `graphql.queryLimits.maxResults` not causing an error before getting the values from the database.
4 changes: 2 additions & 2 deletions packages/keystone/src/lib/core/queries/resolvers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ export async function count(
}

function applyEarlyMaxResults(_take: number | null | undefined, list: InitialisedList) {
const take = _take ?? Infinity;
const take = Math.abs(_take ?? Infinity);
// We want to help devs by failing fast and noisily if limits are violated.
// Unfortunately, we can't always be sure of intent.
// E.g., if the query has a "take: 10", is it bad if more results could come back?
Expand All @@ -200,7 +200,7 @@ function applyMaxResults(results: unknown[], list: InitialisedList, context: Key
throw limitsExceededError({ list: list.listKey, type: 'maxResults', limit: list.maxResults });
}
if (context) {
context.totalResults += Array.isArray(results) ? results.length : 1;
context.totalResults += results.length;
if (context.totalResults > context.maxTotalResults) {
throw limitsExceededError({
list: list.listKey,
Expand Down
18 changes: 18 additions & 0 deletions tests/api-tests/queries/limits.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,24 @@ describe('maxResults Limit', () => {
`,
}));

expectLimitsExceededError(errors, [{ path: ['users'] }]);
})
);
test(
'negative take still causes the early error',
runner(async ({ context }) => {
// there are no users so this will never hit the late error so if it errors
// it must be the early error
let { errors } = await context.graphql.raw({
query: `
query {
users(take: -10) {
name
}
}
`,
});

expectLimitsExceededError(errors, [{ path: ['users'] }]);
})
);
Expand Down

0 comments on commit bd120c7

Please sign in to comment.