From bc50481b3ed09d64b0d6d552863e95a3a76d6b92 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Fri, 13 Aug 2021 12:39:19 +1000 Subject: [PATCH] Update resolveInput error handling --- .changeset/grumpy-jobs-burn.md | 5 ++ .../src/lib/core/mutations/create-update.ts | 41 +++++++---- tests/api-tests/hooks/list-hooks.test.ts | 73 +++++++++++++++++-- 3 files changed, 99 insertions(+), 20 deletions(-) create mode 100644 .changeset/grumpy-jobs-burn.md diff --git a/.changeset/grumpy-jobs-burn.md b/.changeset/grumpy-jobs-burn.md new file mode 100644 index 00000000000..7f22cb38462 --- /dev/null +++ b/.changeset/grumpy-jobs-burn.md @@ -0,0 +1,5 @@ +--- +'@keystone-next/keystone': patch +--- + +Updated handling of errors in `resolveInput` hooks to provide developers with appropriate debug information. diff --git a/packages/keystone/src/lib/core/mutations/create-update.ts b/packages/keystone/src/lib/core/mutations/create-update.ts index eda831a0393..aa34bdef55c 100644 --- a/packages/keystone/src/lib/core/mutations/create-update.ts +++ b/packages/keystone/src/lib/core/mutations/create-update.ts @@ -9,6 +9,7 @@ import { runWithPrisma, } from '../utils'; import { resolveUniqueWhereInput, UniqueInputFilter } from '../where-inputs'; +import { extensionError } from '../graphql-errors'; import { resolveRelateToManyForCreateInput, resolveRelateToManyForUpdateInput, @@ -228,23 +229,37 @@ async function getResolvedData( ); // Resolve input hooks - resolvedData = Object.fromEntries( - await promiseAllRejectWithAllErrors( - Object.entries(list.fields).map(async ([fieldKey, field]) => { - if (field.hooks.resolveInput === undefined) { - return [fieldKey, resolvedData[fieldKey]]; - } - const value = await field.hooks.resolveInput({ + const hookName = 'resolveInput'; + // Field hooks + let _resolvedData: Record = {}; + const fieldsErrors: { error: Error; tag: string }[] = []; + for (const [fieldPath, field] of Object.entries(list.fields)) { + if (field.hooks.resolveInput === undefined) { + _resolvedData[fieldPath] = resolvedData[fieldPath]; + } else { + try { + _resolvedData[fieldPath] = await field.hooks.resolveInput({ ...hookArgs, resolvedData, - fieldPath: fieldKey, + fieldPath, }); - return [fieldKey, value]; - }) - ) - ); + } catch (error) { + fieldsErrors.push({ error, tag: `${list.listKey}.${fieldPath}` }); + } + } + } + if (fieldsErrors.length) { + throw extensionError(hookName, fieldsErrors); + } + resolvedData = _resolvedData; + + // List hooks if (list.hooks.resolveInput) { - resolvedData = (await list.hooks.resolveInput({ ...hookArgs, resolvedData })) as any; + try { + resolvedData = (await list.hooks.resolveInput({ ...hookArgs, resolvedData })) as any; + } catch (error) { + throw extensionError(hookName, [{ error, tag: list.listKey }]); + } } return resolvedData; diff --git a/tests/api-tests/hooks/list-hooks.test.ts b/tests/api-tests/hooks/list-hooks.test.ts index e862804b491..72ab482736c 100644 --- a/tests/api-tests/hooks/list-hooks.test.ts +++ b/tests/api-tests/hooks/list-hooks.test.ts @@ -1,7 +1,7 @@ import { text } from '@keystone-next/fields'; import { createSchema, list } from '@keystone-next/keystone/schema'; import { setupTestRunner } from '@keystone-next/testing'; -import { apiTestConfig } from '../utils'; +import { apiTestConfig, expectExtensionError } from '../utils'; const runner = setupTestRunner({ config: apiTestConfig({ @@ -11,6 +11,10 @@ const runner = setupTestRunner({ name: text({ hooks: { resolveInput: ({ resolvedData }) => { + if (resolvedData.name === 'trigger field error') { + throw new Error('Field error triggered'); + } + return `${resolvedData.name}-field`; }, }, @@ -18,6 +22,9 @@ const runner = setupTestRunner({ }, hooks: { resolveInput: ({ resolvedData }) => { + if (resolvedData.name === 'trigger list error-field') { + throw new Error('List error triggered'); + } return { name: `${resolvedData.name}-list`, }; @@ -29,18 +36,70 @@ const runner = setupTestRunner({ }); describe('List Hooks: #resolveInput()', () => { - it( + test( 'resolves fields first, then passes them to the list', runner(async ({ context }) => { - const user = await context.lists.User.createOne({ - data: { name: 'jess' }, - query: 'name', - }); - + const user = await context.lists.User.createOne({ data: { name: 'jess' }, query: 'name' }); // Field should be executed first, appending `-field`, then the list // should be executed which appends `-list`, and finally that total // result should be stored. expect(user.name).toBe('jess-field-list'); }) ); + + test( + 'List error', + runner(async ({ context }) => { + // Trigger an error + const { data, errors } = await context.graphql.raw({ + query: `mutation ($data: UserCreateInput!) { createUser(data: $data) { id } }`, + variables: { data: { name: `trigger list error` } }, + }); + // Returns null and throws an error + expect(data).toEqual({ createUser: null }); + const message = `List error triggered`; + expectExtensionError('dev', false, undefined, errors, `resolveInput`, [ + { + path: ['createUser'], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*resolveInput .${__filename}`) + ), + }, + ], + }, + ]); + }) + ); + + test( + 'Field error', + runner(async ({ context }) => { + // Trigger an error + const { data, errors } = await context.graphql.raw({ + query: `mutation ($data: UserCreateInput!) { createUser(data: $data) { id } }`, + variables: { data: { name: `trigger field error` } }, + }); + // Returns null and throws an error + expect(data).toEqual({ createUser: null }); + const message = `Field error triggered`; + expectExtensionError('dev', false, undefined, errors, `resolveInput`, [ + { + path: ['createUser'], + messages: [`User.name: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*resolveInput .${__filename}`) + ), + }, + ], + }, + ]); + }) + ); });