From 1030296d1f304dc44246e895089ac1f992e80590 Mon Sep 17 00:00:00 2001 From: Tim Leslie Date: Tue, 10 Aug 2021 12:20:49 +1000 Subject: [PATCH] Include stacktrace flag (#6267) --- .changeset/real-sheep-end.md | 6 + docs/pages/docs/apis/config.mdx | 3 + .../next-graphql.ts | 2 +- .../keystone/src/admin-ui/templates/api.ts | 2 +- .../keystone/src/lib/core/graphql-errors.ts | 8 +- .../src/lib/server/createApolloServer.ts | 44 +- .../src/lib/server/createExpressServer.ts | 16 +- packages/types/src/config/index.ts | 34 + tests/api-tests/access-control/authed.test.ts | 14 +- .../mutations-field-static.test.ts | 14 +- .../mutations-list-declarative.test.ts | 14 +- .../mutations-list-static.test.ts | 21 +- .../access-control/not-authed.test.ts | 18 +- tests/api-tests/auth-header.test.ts | 2 +- tests/api-tests/hooks/hook-errors.test.ts | 995 +++++++++--------- .../nested-mutations/create-many.test.ts | 4 +- tests/api-tests/utils.ts | 45 +- 17 files changed, 695 insertions(+), 547 deletions(-) create mode 100644 .changeset/real-sheep-end.md diff --git a/.changeset/real-sheep-end.md b/.changeset/real-sheep-end.md new file mode 100644 index 00000000000..523ab3557d4 --- /dev/null +++ b/.changeset/real-sheep-end.md @@ -0,0 +1,6 @@ +--- +'@keystone-next/keystone': minor +'@keystone-next/types': minor +--- + +Added `config.graphql.debug` option, which can be used to control whether debug information such as stack traces are included in the errors returned by the GraphQL API. diff --git a/docs/pages/docs/apis/config.mdx b/docs/pages/docs/apis/config.mdx index aa881ffab0e..bab6cab20d9 100644 --- a/docs/pages/docs/apis/config.mdx +++ b/docs/pages/docs/apis/config.mdx @@ -277,6 +277,8 @@ It has a TypeScript type of `GraphQLConfig`. Options: +- `debug` (default: `process.env.NODE_ENV !== 'production'`): If `true`, stacktraces from both Apollo errors and Keystone errors will be included in the errors returned from the GraphQL API. + These can be filtered out with `apolloConfig.formatError` if you need to process them, but do not want them returned over the GraphQL API. - `queryLimits` (default: `undefined`): Allows you to limit the total number of results returned from a query to your GraphQL API. See also the per-list `graphql.queryLimits` option in the [Schema API](./schema). - `apolloConfig` (default: `undefined`): Allows you to pass extra options into the `ApolloServer` constructor. @@ -287,6 +289,7 @@ Options: ```typescript export default config({ graphql: { + debug: process.env.NODE_ENV !== 'production', queryLimits: { maxTotalResults: 100 }, apolloConfig: { playground: process.env.NODE_ENV !== 'production', diff --git a/packages/keystone/src/___internal-do-not-use-will-break-in-patch/next-graphql.ts b/packages/keystone/src/___internal-do-not-use-will-break-in-patch/next-graphql.ts index 0a032bfe62b..78ec432b4c4 100644 --- a/packages/keystone/src/___internal-do-not-use-will-break-in-patch/next-graphql.ts +++ b/packages/keystone/src/___internal-do-not-use-will-break-in-patch/next-graphql.ts @@ -15,7 +15,7 @@ export function nextGraphQLAPIRoute(keystoneConfig: KeystoneConfig, prismaClient graphQLSchema, createContext: keystone.createContext, sessionStrategy: initializedKeystoneConfig.session, - apolloConfig: initializedKeystoneConfig.graphql?.apolloConfig, + graphqlConfig: initializedKeystoneConfig.graphql, connectionPromise: keystone.connect(), }); diff --git a/packages/keystone/src/admin-ui/templates/api.ts b/packages/keystone/src/admin-ui/templates/api.ts index 01d75117313..c779651e582 100644 --- a/packages/keystone/src/admin-ui/templates/api.ts +++ b/packages/keystone/src/admin-ui/templates/api.ts @@ -9,7 +9,7 @@ const apolloServer = createApolloServerMicro({ graphQLSchema, createContext, sessionStrategy: initializedKeystoneConfig.session ? initializedKeystoneConfig.session() : undefined, - apolloConfig: initializedKeystoneConfig.graphql?.apolloConfig, + graphqlConfig: initializedKeystoneConfig.graphql, connectionPromise: keystone.connect(), }); diff --git a/packages/keystone/src/lib/core/graphql-errors.ts b/packages/keystone/src/lib/core/graphql-errors.ts index 33b58384f66..6587864b984 100644 --- a/packages/keystone/src/lib/core/graphql-errors.ts +++ b/packages/keystone/src/lib/core/graphql-errors.ts @@ -12,12 +12,8 @@ export const extensionError = (extension: string, things: { error: Error; tag: s return new ApolloError( `An error occured while running "${extension}".\n${s}`, 'INTERNAL_SERVER_ERROR', - // Make the original stack traces available in non-production modes. - // TODO: We need to have a way to make these stack traces available - // for logging in production mode. - process.env.NODE_ENV !== 'production' - ? { errors: things.map(t => ({ stacktrace: t.error.stack, message: t.error.message })) } - : undefined + // Make the original stack traces available. + { debug: things.map(t => ({ stacktrace: t.error.stack, message: t.error.message })) } ); }; diff --git a/packages/keystone/src/lib/server/createApolloServer.ts b/packages/keystone/src/lib/server/createApolloServer.ts index 70cce699e4d..af4d51b3985 100644 --- a/packages/keystone/src/lib/server/createApolloServer.ts +++ b/packages/keystone/src/lib/server/createApolloServer.ts @@ -1,22 +1,22 @@ import type { IncomingMessage, ServerResponse } from 'http'; -import { GraphQLSchema } from 'graphql'; +import { GraphQLError, GraphQLSchema } from 'graphql'; import { ApolloServer as ApolloServerMicro } from 'apollo-server-micro'; import { ApolloServer as ApolloServerExpress } from 'apollo-server-express'; import type { Config } from 'apollo-server-express'; -import type { CreateContext, SessionStrategy } from '@keystone-next/types'; +import type { CreateContext, GraphQLConfig, SessionStrategy } from '@keystone-next/types'; import { createSessionContext } from '../../session'; export const createApolloServerMicro = ({ graphQLSchema, createContext, sessionStrategy, - apolloConfig, + graphqlConfig, connectionPromise, }: { graphQLSchema: GraphQLSchema; createContext: CreateContext; sessionStrategy?: SessionStrategy; - apolloConfig?: Config; + graphqlConfig?: GraphQLConfig; connectionPromise: Promise; }) => { const context = async ({ req, res }: { req: IncomingMessage; res: ServerResponse }) => { @@ -28,7 +28,7 @@ export const createApolloServerMicro = ({ req, }); }; - const serverConfig = _createApolloServerConfig({ graphQLSchema, apolloConfig }); + const serverConfig = _createApolloServerConfig({ graphQLSchema, graphqlConfig }); return new ApolloServerMicro({ ...serverConfig, context }); }; @@ -36,12 +36,12 @@ export const createApolloServerExpress = ({ graphQLSchema, createContext, sessionStrategy, - apolloConfig, + graphqlConfig, }: { graphQLSchema: GraphQLSchema; createContext: CreateContext; sessionStrategy?: SessionStrategy; - apolloConfig?: Config; + graphqlConfig?: GraphQLConfig; }) => { const context = async ({ req, res }: { req: IncomingMessage; res: ServerResponse }) => createContext({ @@ -50,18 +50,19 @@ export const createApolloServerExpress = ({ : undefined, req, }); - const serverConfig = _createApolloServerConfig({ graphQLSchema, apolloConfig }); + const serverConfig = _createApolloServerConfig({ graphQLSchema, graphqlConfig }); return new ApolloServerExpress({ ...serverConfig, context }); }; const _createApolloServerConfig = ({ graphQLSchema, - apolloConfig, + graphqlConfig, }: { graphQLSchema: GraphQLSchema; - apolloConfig?: Config; + graphqlConfig?: GraphQLConfig; }) => { // Playground config, is /api/graphql available? + const apolloConfig = graphqlConfig?.apolloConfig; const pp = apolloConfig?.playground; let playground: Config['playground']; const settings = { 'request.credentials': 'same-origin' }; @@ -86,6 +87,7 @@ const _createApolloServerConfig = ({ return { uploads: false, schema: graphQLSchema, + debug: graphqlConfig?.debug, // If undefined, use Apollo default of NODE_ENV !== 'production' // FIXME: support for apollo studio tracing // ...(process.env.ENGINE_API_KEY || process.env.APOLLO_KEY // ? { tracing: true } @@ -97,6 +99,7 @@ const _createApolloServerConfig = ({ // tracing: dev, // }), ...apolloConfig, + formatError: formatError(graphqlConfig), // Carefully inject the playground playground, // FIXME: Support for file handling configuration @@ -104,3 +107,24 @@ const _createApolloServerConfig = ({ // maxFiles: 5, }; }; + +const formatError = (graphqlConfig: GraphQLConfig | undefined) => { + return (err: GraphQLError) => { + let debug = graphqlConfig?.debug; + if (debug === undefined) { + debug = process.env.NODE_ENV !== 'production'; + } + + if (!debug && err.extensions) { + // Strip out any `debug` extensions + delete err.extensions.debug; + delete err.extensions.exception; + } + + if (graphqlConfig?.apolloConfig?.formatError) { + return graphqlConfig.apolloConfig.formatError(err); + } else { + return err; + } + }; +}; diff --git a/packages/keystone/src/lib/server/createExpressServer.ts b/packages/keystone/src/lib/server/createExpressServer.ts index f4342538093..e94fe7c1297 100644 --- a/packages/keystone/src/lib/server/createExpressServer.ts +++ b/packages/keystone/src/lib/server/createExpressServer.ts @@ -1,9 +1,13 @@ -import type { Config } from 'apollo-server-express'; import cors, { CorsOptions } from 'cors'; import express from 'express'; import { GraphQLSchema } from 'graphql'; import { graphqlUploadExpress } from 'graphql-upload'; -import type { KeystoneConfig, CreateContext, SessionStrategy } from '@keystone-next/types'; +import type { + KeystoneConfig, + CreateContext, + SessionStrategy, + GraphQLConfig, +} from '@keystone-next/types'; import { createAdminUIServer } from '../../admin-ui/system'; import { createApolloServerExpress } from './createApolloServer'; import { addHealthCheck } from './addHealthCheck'; @@ -16,20 +20,20 @@ const addApolloServer = ({ graphQLSchema, createContext, sessionStrategy, - apolloConfig, + graphqlConfig, }: { server: express.Express; config: KeystoneConfig; graphQLSchema: GraphQLSchema; createContext: CreateContext; sessionStrategy?: SessionStrategy; - apolloConfig?: Config; + graphqlConfig?: GraphQLConfig; }) => { const apolloServer = createApolloServerExpress({ graphQLSchema, createContext, sessionStrategy, - apolloConfig, + graphqlConfig, }); const maxFileSize = config.server?.maxFileSize || DEFAULT_MAX_FILE_SIZE; @@ -68,7 +72,7 @@ export const createExpressServer = async ( graphQLSchema, createContext, sessionStrategy: config.session, - apolloConfig: config.graphql?.apolloConfig, + graphqlConfig: config.graphql, }); if (config.ui?.isDisabled) { diff --git a/packages/types/src/config/index.ts b/packages/types/src/config/index.ts index 0717d6c62aa..c7023c74d30 100644 --- a/packages/types/src/config/index.ts +++ b/packages/types/src/config/index.ts @@ -143,6 +143,40 @@ export type GraphQLConfig = { * @see https://www.apollographql.com/docs/apollo-server/api/apollo-server/#constructor */ apolloConfig?: Config; + /* + * When an error is returned from the GraphQL API, Apollo can include a stacktrace + * indicating where the error occurred. When Keystone is processing mutations, it + * will sometimes captures more than one error at a time, and then group these into + * a single error returned from the GraphQL API. Each of these errors will include + * a stacktrace. + * + * In general both categories of stacktrace are useful for debugging while developing, + * but should not be exposed in production, and this is the default behaviour of Keystone. + * + * You can use the `debug` option to change this behaviour. A use case for this + * would be if you need to send the stacktraces to a log, but do not want to return them + * from the API. In this case you could set `debug: true` and use + * `apolloConfig.formatError` to log the stacktraces and then strip them out before + * returning the error. + * + * ``` + * graphql: { + * debug: true, + * apolloConfig: { + * formatError: err => { + * console.error(err); + * delete err.extensions?.errors; + * delete err.extensions?.exception?.errors; + * delete err.extensions?.exception?.stacktrace; + * return err; + * }, + * }, + * } + * ``` + * * + * Default: process.env.NODE_ENV !== 'production' + */ + debug?: boolean; }; // config.extendGraphqlSchema diff --git a/tests/api-tests/access-control/authed.test.ts b/tests/api-tests/access-control/authed.test.ts index 963c6e670e9..9bd62aa84d0 100644 --- a/tests/api-tests/access-control/authed.test.ts +++ b/tests/api-tests/access-control/authed.test.ts @@ -22,7 +22,7 @@ const expectNoAccess = ( errors: readonly GraphQLError[] | undefined, name: N ) => { - expectAccessDenied(errors, [{ path: [name] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: [name] }]); expect(data?.[name]).toBe(null); }; @@ -158,7 +158,9 @@ describe('Authed', () => { expect(data).toEqual({ authenticatedItem: { id: user.id, yesRead: user.yesRead, noRead: null }, }); - expectAccessDenied(errors, [{ path: ['authenticatedItem', 'noRead'] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['authenticatedItem', 'noRead'] }, + ]); }); (['imperative', 'declarative'] as const).forEach(mode => { @@ -442,7 +444,7 @@ describe('Authed', () => { if (mode === 'imperative') { expectNamedArray(data, errors, multiDeleteMutationName, [validId1, validId2]); } else { - expectAccessDenied(errors, [ + expectAccessDenied('dev', false, undefined, errors, [ { path: [multiDeleteMutationName, 0] }, { path: [multiDeleteMutationName, 1] }, ]); @@ -459,7 +461,9 @@ describe('Authed', () => { if (mode === 'imperative') { expectNamedArray(data, errors, multiDeleteMutationName, [validId1, invalidId]); } else { - expectAccessDenied(errors, [{ path: [multiDeleteMutationName, 1] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: [multiDeleteMutationName, 1] }, + ]); expect(data).toEqual({ [multiDeleteMutationName]: [{ id: validId1 }, null] }); } }); @@ -468,7 +472,7 @@ describe('Authed', () => { const multiDeleteMutationName = `delete${nameFn[mode](access)}s`; const query = `mutation { ${multiDeleteMutationName}(where: [{ id: "${FAKE_ID[provider]}" }, { id: "${FAKE_ID_2[provider]}" }]) { id } }`; const { data, errors } = await context.graphql.raw({ query }); - expectAccessDenied(errors, [ + expectAccessDenied('dev', false, undefined, errors, [ { path: [multiDeleteMutationName, 0] }, { path: [multiDeleteMutationName, 1] }, ]); diff --git a/tests/api-tests/access-control/mutations-field-static.test.ts b/tests/api-tests/access-control/mutations-field-static.test.ts index 5ce3f47daca..7eebb4e8b4e 100644 --- a/tests/api-tests/access-control/mutations-field-static.test.ts +++ b/tests/api-tests/access-control/mutations-field-static.test.ts @@ -51,7 +51,7 @@ describe('Access control - Imperative => static', () => { // Returns null and throws an error expect(data).toEqual({ createUser: null }); - expectAccessDenied(errors, [{ path: ['createUser'] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: ['createUser'] }]); // Only the original user should exist const _users = await context.lists.User.findMany({ query: 'id name other' }); @@ -79,7 +79,7 @@ describe('Access control - Imperative => static', () => { // Returns null and throws an error expect(data).toEqual({ updateUser: null }); - expectAccessDenied(errors, [{ path: ['updateUser'] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: ['updateUser'] }]); // User should have its original name const _users = await context.lists.User.findMany({ query: 'id name other' }); @@ -118,7 +118,10 @@ describe('Access control - Imperative => static', () => { }); // The invalid updates should have errors which point to the nulls in their path - expectAccessDenied(errors, [{ path: ['createUsers', 1] }, { path: ['createUsers', 3] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['createUsers', 1] }, + { path: ['createUsers', 3] }, + ]); // Valid users should exist in the database const users = await context.lists.User.findMany({ @@ -172,7 +175,10 @@ describe('Access control - Imperative => static', () => { }); // The invalid updates should have errors which point to the nulls in their path - expectAccessDenied(errors, [{ path: ['updateUsers', 1] }, { path: ['updateUsers', 3] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['updateUsers', 1] }, + { path: ['updateUsers', 3] }, + ]); // All users should still exist in the database const _users = await context.lists.User.findMany({ diff --git a/tests/api-tests/access-control/mutations-list-declarative.test.ts b/tests/api-tests/access-control/mutations-list-declarative.test.ts index cbe015f6a77..db97e7edb1b 100644 --- a/tests/api-tests/access-control/mutations-list-declarative.test.ts +++ b/tests/api-tests/access-control/mutations-list-declarative.test.ts @@ -40,7 +40,7 @@ describe('Access control - Imperative => declarative', () => { // Returns null and throws an error expect(data).toEqual({ updateUser: null }); - expectAccessDenied(errors, [{ path: ['updateUser'] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: ['updateUser'] }]); // User should have its original name const _users = await context.lists.User.findMany({ query: 'id name' }); @@ -65,7 +65,7 @@ describe('Access control - Imperative => declarative', () => { // Returns null and throws an error expect(data).toEqual({ deleteUser: null }); - expectAccessDenied(errors, [{ path: ['deleteUser'] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: ['deleteUser'] }]); // Bad users should still be in the database. const _users = await context.lists.User.findMany({ query: 'id name' }); @@ -120,7 +120,10 @@ describe('Access control - Imperative => declarative', () => { null, ], }); - expectAccessDenied(errors, [{ path: ['updateUsers', 1] }, { path: ['updateUsers', 3] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['updateUsers', 1] }, + { path: ['updateUsers', 3] }, + ]); // All users should still exist in the database const _users = await context.lists.User.findMany({ @@ -161,7 +164,10 @@ describe('Access control - Imperative => declarative', () => { }, }); - expectAccessDenied(errors, [{ path: ['deleteUsers', 1] }, { path: ['deleteUsers', 3] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['deleteUsers', 1] }, + { path: ['deleteUsers', 3] }, + ]); // Valid users are returned, invalid come back as null // The invalid deletes should have errors which point to the nulls in their path diff --git a/tests/api-tests/access-control/mutations-list-static.test.ts b/tests/api-tests/access-control/mutations-list-static.test.ts index 3984c0c9e6f..3c2ce0d1a0a 100644 --- a/tests/api-tests/access-control/mutations-list-static.test.ts +++ b/tests/api-tests/access-control/mutations-list-static.test.ts @@ -46,7 +46,7 @@ describe('Access control - Imperative => static', () => { // Returns null and throws an error expect(data).toEqual({ createUser: null }); - expectAccessDenied(errors, [{ path: ['createUser'] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: ['createUser'] }]); // Only the original user should exist const _users = await context.lists.User.findMany({ query: 'id name' }); @@ -70,7 +70,7 @@ describe('Access control - Imperative => static', () => { // Returns null and throws an error expect(data).toEqual({ updateUser: null }); - expectAccessDenied(errors, [{ path: ['updateUser'] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: ['updateUser'] }]); // User should have its original name const _users = await context.lists.User.findMany({ query: 'id name' }); @@ -95,7 +95,7 @@ describe('Access control - Imperative => static', () => { // Returns null and throws an error expect(data).toEqual({ deleteUser: null }); - expectAccessDenied(errors, [{ path: ['deleteUser'] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: ['deleteUser'] }]); // Bad users should still be in the database. const _users = await context.lists.User.findMany({ query: 'id name' }); @@ -133,7 +133,10 @@ describe('Access control - Imperative => static', () => { }); // The invalid updates should have errors which point to the nulls in their path - expectAccessDenied(errors, [{ path: ['createUsers', 1] }, { path: ['createUsers', 3] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['createUsers', 1] }, + { path: ['createUsers', 3] }, + ]); // The good users should exist in the database const users = await context.lists.User.findMany(); @@ -182,7 +185,10 @@ describe('Access control - Imperative => static', () => { ]); // The invalid updates should have errors which point to the nulls in their path - expectAccessDenied(errors, [{ path: ['updateUsers', 1] }, { path: ['updateUsers', 3] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['updateUsers', 1] }, + { path: ['updateUsers', 3] }, + ]); // All users should still exist in the database const _users = await context.lists.User.findMany({ @@ -232,7 +238,10 @@ describe('Access control - Imperative => static', () => { ]); // The invalid updates should have errors which point to the nulls in their path - expectAccessDenied(errors, [{ path: ['deleteUsers', 1] }, { path: ['deleteUsers', 3] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['deleteUsers', 1] }, + { path: ['deleteUsers', 3] }, + ]); const _users = await context.lists.User.findMany({ orderBy: { name: 'asc' }, diff --git a/tests/api-tests/access-control/not-authed.test.ts b/tests/api-tests/access-control/not-authed.test.ts index 2ac02923d9d..d95b7f3f4f8 100644 --- a/tests/api-tests/access-control/not-authed.test.ts +++ b/tests/api-tests/access-control/not-authed.test.ts @@ -20,7 +20,7 @@ const expectNoAccess = ( name: N ) => { expect(data?.[name]).toBe(null); - expectAccessDenied(errors, [{ path: [name] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: [name] }]); }; type IdType = any; @@ -136,7 +136,7 @@ describe(`Not authed`, () => { const query = `mutation { ${createMutationName}(data: { ${fieldName}: "bar" }) { id } }`; const { data, errors } = await context.graphql.raw({ query }); expect(data).toEqual({ [createMutationName]: null }); - expectAccessDenied(errors, [{ path: [createMutationName] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: [createMutationName] }]); }); }); }); @@ -163,7 +163,7 @@ describe(`Not authed`, () => { const query = `query { ${countName} }`; const { data, errors } = await context.graphql.raw({ query }); expect(data).toEqual({ [countName]: null }); - expectAccessDenied(errors, [{ path: [countName] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: [countName] }]); }); test(`single denied: ${JSON.stringify(access)}`, async () => { @@ -197,7 +197,9 @@ describe(`Not authed`, () => { }); const query = `query { ${singleQueryName}(where: { id: "${item.id}" }) { id ${fieldName} } }`; const { data, errors } = await context.graphql.raw({ query }); - expectAccessDenied(errors, [{ path: [singleQueryName, fieldName] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: [singleQueryName, fieldName] }, + ]); expect(data).toEqual({ [singleQueryName]: { id: item.id, [fieldName]: null } }); }); test(`field allowed - multi: ${JSON.stringify(access)}`, async () => { @@ -217,7 +219,7 @@ describe(`Not authed`, () => { }); const query = `query { ${allQueryName} { id ${fieldName} } }`; const { data, errors } = await context.graphql.raw({ query }); - expectAccessDenied(errors, [ + expectAccessDenied('dev', false, undefined, errors, [ { path: [allQueryName, 0, fieldName] }, { path: [allQueryName, 1, fieldName] }, ]); @@ -351,7 +353,7 @@ describe(`Not authed`, () => { const query = `mutation { ${updateMutationName}(where: { id: "${item.id}" }, data: { ${fieldName}: "bar" }) { id } }`; const { data, errors } = await context.graphql.raw({ query }); expect(data).toEqual({ [updateMutationName]: null }); - expectAccessDenied(errors, [{ path: [updateMutationName] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: [updateMutationName] }]); }); }); }); @@ -377,7 +379,9 @@ describe(`Not authed`, () => { const { data, errors } = await context.graphql.raw({ query }); expect(data).toEqual({ [multiDeleteMutationName]: [null] }); - expectAccessDenied(errors, [{ path: [multiDeleteMutationName, 0] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: [multiDeleteMutationName, 0] }, + ]); }); }); }); diff --git a/tests/api-tests/auth-header.test.ts b/tests/api-tests/auth-header.test.ts index 13c4e9f1a6c..e9d027d55d3 100644 --- a/tests/api-tests/auth-header.test.ts +++ b/tests/api-tests/auth-header.test.ts @@ -83,7 +83,7 @@ describe('Auth testing', () => { } const { data, errors } = await context.graphql.raw({ query: '{ users { id } }' }); expect(data).toEqual({ users: null }); - expectAccessDenied(errors, [{ path: ['users'] }]); + expectAccessDenied('dev', false, undefined, errors, [{ path: ['users'] }]); }) ); diff --git a/tests/api-tests/hooks/hook-errors.test.ts b/tests/api-tests/hooks/hook-errors.test.ts index 387fdd71842..bd2dad79fab 100644 --- a/tests/api-tests/hooks/hook-errors.test.ts +++ b/tests/api-tests/hooks/hook-errors.test.ts @@ -1,556 +1,575 @@ import { text } from '@keystone-next/fields'; import { createSchema, list } from '@keystone-next/keystone/schema'; -import { setupTestRunner } from '@keystone-next/testing'; +import { GraphQLRequest, setupTestRunner } from '@keystone-next/testing'; +import { KeystoneContext } from '../../../packages/types/src'; import { apiTestConfig, expectAccessDenied, expectExtensionError } from '../utils'; -const runner = setupTestRunner({ - config: apiTestConfig({ - lists: createSchema({ - User: list({ - fields: { name: text() }, - hooks: { - beforeChange: ({ resolvedData }) => { - if (resolvedData.name === 'trigger before') { - throw new Error('Simulated error: beforeChange'); - } - }, - afterChange: ({ resolvedData }) => { - if (resolvedData.name === 'trigger after') { - throw new Error('Simulated error: afterChange'); - } - }, - beforeDelete: ({ existingItem }) => { - if (existingItem.name === 'trigger before delete') { - throw new Error('Simulated error: beforeDelete'); - } - }, - afterDelete: ({ existingItem }) => { - if (existingItem.name === 'trigger after delete') { - throw new Error('Simulated error: afterDelete'); - } - }, - }, - }), - Post: list({ - fields: { - title: text({ - hooks: { - beforeChange: ({ resolvedData }) => { - if (resolvedData.title === 'trigger before') { - throw new Error('Simulated error: title: beforeChange'); - } - }, - afterChange: ({ resolvedData }) => { - if (resolvedData.title === 'trigger after') { - throw new Error('Simulated error: title: afterChange'); - } - }, - beforeDelete: ({ existingItem }) => { - if (existingItem.title === 'trigger before delete') { - throw new Error('Simulated error: title: beforeDelete'); - } - }, - afterDelete: ({ existingItem }) => { - if (existingItem.title === 'trigger after delete') { - throw new Error('Simulated error: title: afterDelete'); - } - }, +const runner = (debug: boolean | undefined) => + setupTestRunner({ + config: apiTestConfig({ + lists: createSchema({ + User: list({ + fields: { name: text() }, + hooks: { + beforeChange: ({ resolvedData }) => { + if (resolvedData.name === 'trigger before') { + throw new Error('Simulated error: beforeChange'); + } }, - }), - content: text({ - hooks: { - beforeChange: ({ resolvedData }) => { - if (resolvedData.content === 'trigger before') { - throw new Error('Simulated error: content: beforeChange'); - } - }, - afterChange: ({ resolvedData }) => { - if (resolvedData.content === 'trigger after') { - throw new Error('Simulated error: content: afterChange'); - } - }, - beforeDelete: ({ existingItem }) => { - if (existingItem.content === 'trigger before delete') { - throw new Error('Simulated error: content: beforeDelete'); - } + afterChange: ({ resolvedData }) => { + if (resolvedData.name === 'trigger after') { + throw new Error('Simulated error: afterChange'); + } + }, + beforeDelete: ({ existingItem }) => { + if (existingItem.name === 'trigger before delete') { + throw new Error('Simulated error: beforeDelete'); + } + }, + afterDelete: ({ existingItem }) => { + if (existingItem.name === 'trigger after delete') { + throw new Error('Simulated error: afterDelete'); + } + }, + }, + }), + Post: list({ + fields: { + title: text({ + hooks: { + beforeChange: ({ resolvedData }) => { + if (resolvedData.title === 'trigger before') { + throw new Error('Simulated error: title: beforeChange'); + } + }, + afterChange: ({ resolvedData }) => { + if (resolvedData.title === 'trigger after') { + throw new Error('Simulated error: title: afterChange'); + } + }, + beforeDelete: ({ existingItem }) => { + if (existingItem.title === 'trigger before delete') { + throw new Error('Simulated error: title: beforeDelete'); + } + }, + afterDelete: ({ existingItem }) => { + if (existingItem.title === 'trigger after delete') { + throw new Error('Simulated error: title: afterDelete'); + } + }, }, - afterDelete: ({ existingItem }) => { - if (existingItem.content === 'trigger after delete') { - throw new Error('Simulated error: content: afterDelete'); - } + }), + content: text({ + hooks: { + beforeChange: ({ resolvedData }) => { + if (resolvedData.content === 'trigger before') { + throw new Error('Simulated error: content: beforeChange'); + } + }, + afterChange: ({ resolvedData }) => { + if (resolvedData.content === 'trigger after') { + throw new Error('Simulated error: content: afterChange'); + } + }, + beforeDelete: ({ existingItem }) => { + if (existingItem.content === 'trigger before delete') { + throw new Error('Simulated error: content: beforeDelete'); + } + }, + afterDelete: ({ existingItem }) => { + if (existingItem.content === 'trigger after delete') { + throw new Error('Simulated error: content: afterDelete'); + } + }, }, - }, - }), - }, + }), + }, + }), }), + graphql: { debug }, }), - }), -}); + }); -(['dev', 'production'] as const).map(mode => - describe(`NODE_ENV=${mode}`, () => { - beforeAll(() => { - // @ts-ignore - process.env.NODE_ENV = mode; - }); - afterAll(() => { - // @ts-ignore - process.env.NODE_ENV = 'test'; - }); +[true, false].map(useHttp => { + const runQuery = async ( + context: KeystoneContext, + graphQLRequest: GraphQLRequest, + query: { query: string; variables?: Record } + ) => { + if (useHttp) { + const { body } = await graphQLRequest(query); + return body; + } else { + return await context.graphql.raw(query); + } + }; - ['before', 'after'].map(phase => { - describe(`List Hooks: ${phase}Change/${phase}Delete()`, () => { - test( - 'createOne', - runner(async ({ context }) => { - // Valid name should pass - await context.lists.User.createOne({ data: { name: 'good' } }); + [true, false, undefined].map(debug => { + (['dev', 'production'] as const).map(mode => + describe(`NODE_ENV=${mode}, debug=${debug} useHttp=${useHttp}`, () => { + beforeAll(() => { + // @ts-ignore + process.env.NODE_ENV = mode; + }); + afterAll(() => { + // @ts-ignore + process.env.NODE_ENV = 'test'; + }); - // Trigger an error - const { data, errors } = await context.graphql.raw({ - query: `mutation ($data: UserCreateInput!) { createUser(data: $data) { id } }`, - variables: { data: { name: `trigger ${phase}` } }, - }); + ['before', 'after'].map(phase => { + describe(`List Hooks: ${phase}Change/${phase}Delete()`, () => { + test( + 'createOne', + runner(debug)(async ({ context, graphQLRequest }) => { + // Valid name should pass + await context.lists.User.createOne({ data: { name: 'good' } }); - // Returns null and throws an error - expect(data).toEqual({ createUser: null }); - const message = `Simulated error: ${phase}Change`; - expectExtensionError(mode, false, errors, `${phase}Change`, [ - { - path: ['createUser'], - messages: [`User: Simulated error: ${phase}Change`], - errors: [ + // Trigger an error + const { data, errors } = await runQuery(context, graphQLRequest, { + query: `mutation ($data: UserCreateInput!) { createUser(data: $data) { id } }`, + variables: { data: { name: `trigger ${phase}` } }, + }); + + // Returns null and throws an error + expect(data).toEqual({ createUser: null }); + const message = `Simulated error: ${phase}Change`; + expectExtensionError(mode, useHttp, debug, errors, `${phase}Change`, [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) - ), + path: ['createUser'], + messages: [`User: Simulated error: ${phase}Change`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) + ), + }, + ], }, - ], - }, - ]); + ]); - // Only the original user should exist for 'before', both exist for 'after' - const _users = await context.lists.User.findMany({ query: 'id name' }); - expect(_users.map(({ name }) => name)).toEqual( - phase === 'before' ? ['good'] : ['good', 'trigger after'] + // Only the original user should exist for 'before', both exist for 'after' + const _users = await context.lists.User.findMany({ query: 'id name' }); + expect(_users.map(({ name }) => name)).toEqual( + phase === 'before' ? ['good'] : ['good', 'trigger after'] + ); + }) ); - }) - ); - test( - 'updateOne', - runner(async ({ context }) => { - // Valid name should pass - const user = await context.lists.User.createOne({ data: { name: 'good' } }); - await context.lists.User.updateOne({ - where: { id: user.id }, - data: { name: 'better' }, - }); + test( + 'updateOne', + runner(debug)(async ({ context, graphQLRequest }) => { + // Valid name should pass + const user = await context.lists.User.createOne({ data: { name: 'good' } }); + await context.lists.User.updateOne({ + where: { id: user.id }, + data: { name: 'better' }, + }); - // Invalid name - const { data, errors } = await context.graphql.raw({ - query: `mutation ($id: ID! $data: UserUpdateInput!) { updateUser(where: { id: $id }, data: $data) { id } }`, - variables: { id: user.id, data: { name: `trigger ${phase}` } }, - }); + // Invalid name + const { data, errors } = await runQuery(context, graphQLRequest, { + query: `mutation ($id: ID! $data: UserUpdateInput!) { updateUser(where: { id: $id }, data: $data) { id } }`, + variables: { id: user.id, data: { name: `trigger ${phase}` } }, + }); - // Returns null and throws an error - expect(data).toEqual({ updateUser: null }); - const message = `Simulated error: ${phase}Change`; - expectExtensionError(mode, false, errors, `${phase}Change`, [ - { - path: ['updateUser'], - messages: [`User: ${message}`], - errors: [ + // Returns null and throws an error + expect(data).toEqual({ updateUser: null }); + const message = `Simulated error: ${phase}Change`; + expectExtensionError(mode, useHttp, debug, errors, `${phase}Change`, [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) - ), + path: ['updateUser'], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) + ), + }, + ], }, - ], - }, - ]); + ]); - // User should have its original name for 'before', and the new name for 'after'. - const _users = await context.lists.User.findMany({ query: 'id name' }); - expect(_users.map(({ name }) => name)).toEqual( - phase === 'before' ? ['better'] : ['trigger after'] + // User should have its original name for 'before', and the new name for 'after'. + const _users = await context.lists.User.findMany({ query: 'id name' }); + expect(_users.map(({ name }) => name)).toEqual( + phase === 'before' ? ['better'] : ['trigger after'] + ); + }) ); - }) - ); - test( - 'deleteOne', - runner(async ({ context }) => { - // Valid names should pass - const user1 = await context.lists.User.createOne({ data: { name: 'good' } }); - const user2 = await context.lists.User.createOne({ - data: { name: `trigger ${phase} delete` }, - }); - await context.lists.User.deleteOne({ where: { id: user1.id } }); + test( + 'deleteOne', + runner(debug)(async ({ context, graphQLRequest }) => { + // Valid names should pass + const user1 = await context.lists.User.createOne({ data: { name: 'good' } }); + const user2 = await context.lists.User.createOne({ + data: { name: `trigger ${phase} delete` }, + }); + await context.lists.User.deleteOne({ where: { id: user1.id } }); - // Invalid name - const { data, errors } = await context.graphql.raw({ - query: `mutation ($id: ID!) { deleteUser(where: { id: $id }) { id } }`, - variables: { id: user2.id }, - }); + // Invalid name + const { data, errors } = await runQuery(context, graphQLRequest, { + query: `mutation ($id: ID!) { deleteUser(where: { id: $id }) { id } }`, + variables: { id: user2.id }, + }); - // Returns null and throws an error - expect(data).toEqual({ deleteUser: null }); - const message = `Simulated error: ${phase}Delete`; - expectExtensionError(mode, false, errors, `${phase}Delete`, [ - { - path: ['deleteUser'], - messages: [`User: ${message}`], - errors: [ + // Returns null and throws an error + expect(data).toEqual({ deleteUser: null }); + const message = `Simulated error: ${phase}Delete`; + expectExtensionError(mode, useHttp, debug, errors, `${phase}Delete`, [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Delete .${__filename}`) - ), + path: ['deleteUser'], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Delete .${__filename}`) + ), + }, + ], }, - ], - }, - ]); + ]); - // Bad users should still be in the database for 'before', deleted for 'after'. - const _users = await context.lists.User.findMany({ query: 'id name' }); - expect(_users.map(({ name }) => name)).toEqual( - phase === 'before' ? ['trigger before delete'] : [] + // Bad users should still be in the database for 'before', deleted for 'after'. + const _users = await context.lists.User.findMany({ query: 'id name' }); + expect(_users.map(({ name }) => name)).toEqual( + phase === 'before' ? ['trigger before delete'] : [] + ); + }) ); - }) - ); - test( - 'createMany', - runner(async ({ context }) => { - // Mix of good and bad names - const { data, errors } = await context.graphql.raw({ - query: `mutation ($data: [UserCreateInput!]!) { createUsers(data: $data) { id name } }`, - variables: { - data: [ - { name: 'good 1' }, - { name: `trigger ${phase}` }, - { name: 'good 2' }, - { name: `trigger ${phase}` }, - { name: 'good 3' }, - ], - }, - }); + test( + 'createMany', + runner(debug)(async ({ context, graphQLRequest }) => { + // Mix of good and bad names + const { data, errors } = await runQuery(context, graphQLRequest, { + query: `mutation ($data: [UserCreateInput!]!) { createUsers(data: $data) { id name } }`, + variables: { + data: [ + { name: 'good 1' }, + { name: `trigger ${phase}` }, + { name: 'good 2' }, + { name: `trigger ${phase}` }, + { name: 'good 3' }, + ], + }, + }); - // Valid users are returned, invalid come back as null - expect(data).toEqual({ - createUsers: [ - { id: expect.any(String), name: 'good 1' }, - null, - { id: expect.any(String), name: 'good 2' }, - null, - { id: expect.any(String), name: 'good 3' }, - ], - }); - // The invalid creates should have errors which point to the nulls in their path - const message = `Simulated error: ${phase}Change`; - expectExtensionError(mode, false, errors, `${phase}Change`, [ - { - path: ['createUsers', 1], - messages: [`User: ${message}`], - errors: [ + // Valid users are returned, invalid come back as null + expect(data).toEqual({ + createUsers: [ + { id: expect.any(String), name: 'good 1' }, + null, + { id: expect.any(String), name: 'good 2' }, + null, + { id: expect.any(String), name: 'good 3' }, + ], + }); + // The invalid creates should have errors which point to the nulls in their path + const message = `Simulated error: ${phase}Change`; + expectExtensionError(mode, useHttp, debug, errors, `${phase}Change`, [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) - ), + path: ['createUsers', 1], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) + ), + }, + ], }, - ], - }, - { - path: ['createUsers', 3], - messages: [`User: ${message}`], - errors: [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) - ), + path: ['createUsers', 3], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) + ), + }, + ], }, - ], - }, - ]); + ]); - // Three users should exist in the database for 'before,' five for 'after'. - const users = await context.lists.User.findMany({ - orderBy: { name: 'asc' }, - query: 'id name', - }); - expect(users.map(({ name }) => name)).toEqual( - phase === 'before' - ? ['good 1', 'good 2', 'good 3'] - : ['good 1', 'good 2', 'good 3', 'trigger after', 'trigger after'] + // Three users should exist in the database for 'before,' five for 'after'. + const users = await context.lists.User.findMany({ + orderBy: { name: 'asc' }, + query: 'id name', + }); + expect(users.map(({ name }) => name)).toEqual( + phase === 'before' + ? ['good 1', 'good 2', 'good 3'] + : ['good 1', 'good 2', 'good 3', 'trigger after', 'trigger after'] + ); + }) ); - }) - ); - test( - 'updateMany', - runner(async ({ context }) => { - // Start with some users - const users = await context.lists.User.createMany({ - data: [ - { name: 'good 1' }, - { name: 'good 2' }, - { name: 'good 3' }, - { name: 'good 4' }, - { name: 'good 5' }, - ], - query: 'id name', - }); + test( + 'updateMany', + runner(debug)(async ({ context, graphQLRequest }) => { + // Start with some users + const users = await context.lists.User.createMany({ + data: [ + { name: 'good 1' }, + { name: 'good 2' }, + { name: 'good 3' }, + { name: 'good 4' }, + { name: 'good 5' }, + ], + query: 'id name', + }); - // Mix of good and bad names - const { data, errors } = await context.graphql.raw({ - query: `mutation ($data: [UserUpdateArgs!]!) { updateUsers(data: $data) { id name } }`, - variables: { - data: [ - { where: { id: users[0].id }, data: { name: 'still good 1' } }, - { where: { id: users[1].id }, data: { name: `trigger ${phase}` } }, - { where: { id: users[2].id }, data: { name: 'still good 3' } }, - { where: { id: users[3].id }, data: { name: `trigger ${phase}` } }, - ], - }, - }); + // Mix of good and bad names + const { data, errors } = await runQuery(context, graphQLRequest, { + query: `mutation ($data: [UserUpdateArgs!]!) { updateUsers(data: $data) { id name } }`, + variables: { + data: [ + { where: { id: users[0].id }, data: { name: 'still good 1' } }, + { where: { id: users[1].id }, data: { name: `trigger ${phase}` } }, + { where: { id: users[2].id }, data: { name: 'still good 3' } }, + { where: { id: users[3].id }, data: { name: `trigger ${phase}` } }, + ], + }, + }); - // Valid users are returned, invalid come back as null - expect(data).toEqual({ - updateUsers: [ - { id: users[0].id, name: 'still good 1' }, - null, - { id: users[2].id, name: 'still good 3' }, - null, - ], - }); - // The invalid updates should have errors which point to the nulls in their path - const message = `Simulated error: ${phase}Change`; - expectExtensionError(mode, false, errors, `${phase}Change`, [ - { - path: ['updateUsers', 1], - messages: [`User: ${message}`], - errors: [ + // Valid users are returned, invalid come back as null + expect(data).toEqual({ + updateUsers: [ + { id: users[0].id, name: 'still good 1' }, + null, + { id: users[2].id, name: 'still good 3' }, + null, + ], + }); + // The invalid updates should have errors which point to the nulls in their path + const message = `Simulated error: ${phase}Change`; + expectExtensionError(mode, useHttp, debug, errors, `${phase}Change`, [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) - ), + path: ['updateUsers', 1], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) + ), + }, + ], }, - ], - }, - { - path: ['updateUsers', 3], - messages: [`User: ${message}`], - errors: [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) - ), + path: ['updateUsers', 3], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Change .${__filename}`) + ), + }, + ], }, - ], - }, - ]); + ]); - // All users should still exist in the database, un-changed for `before`, changed for `after`. - const _users = await context.lists.User.findMany({ - orderBy: { name: 'asc' }, - query: 'id name', - }); - expect(_users.map(({ name }) => name)).toEqual( - phase === 'before' - ? ['good 2', 'good 4', 'good 5', 'still good 1', 'still good 3'] - : ['good 5', 'still good 1', 'still good 3', 'trigger after', 'trigger after'] + // All users should still exist in the database, un-changed for `before`, changed for `after`. + const _users = await context.lists.User.findMany({ + orderBy: { name: 'asc' }, + query: 'id name', + }); + expect(_users.map(({ name }) => name)).toEqual( + phase === 'before' + ? ['good 2', 'good 4', 'good 5', 'still good 1', 'still good 3'] + : ['good 5', 'still good 1', 'still good 3', 'trigger after', 'trigger after'] + ); + }) ); - }) - ); - test( - 'deleteMany', - runner(async ({ context }) => { - // Start with some users - const users = await context.lists.User.createMany({ - data: [ - { name: 'good 1' }, - { name: `trigger ${phase} delete` }, - { name: 'good 3' }, - { name: `trigger ${phase} delete` }, - { name: 'good 5' }, - ], - query: 'id name', - }); + test( + 'deleteMany', + runner(debug)(async ({ context, graphQLRequest }) => { + // Start with some users + const users = await context.lists.User.createMany({ + data: [ + { name: 'good 1' }, + { name: `trigger ${phase} delete` }, + { name: 'good 3' }, + { name: `trigger ${phase} delete` }, + { name: 'good 5' }, + ], + query: 'id name', + }); - // Mix of good and bad names - const { data, errors } = await context.graphql.raw({ - query: `mutation ($where: [UserWhereUniqueInput!]!) { deleteUsers(where: $where) { id name } }`, - variables: { - where: [users[0].id, users[1].id, users[2].id, users[3].id].map(id => ({ id })), - }, - }); + // Mix of good and bad names + const { data, errors } = await runQuery(context, graphQLRequest, { + query: `mutation ($where: [UserWhereUniqueInput!]!) { deleteUsers(where: $where) { id name } }`, + variables: { + where: [users[0].id, users[1].id, users[2].id, users[3].id].map(id => ({ id })), + }, + }); - // Valid users are returned, invalid come back as null - expect(data).toEqual({ - deleteUsers: [ - { id: users[0].id, name: 'good 1' }, - null, - { id: users[2].id, name: 'good 3' }, - null, - ], - }); - // The invalid deletes should have errors which point to the nulls in their path - const message = `Simulated error: ${phase}Delete`; - expectExtensionError(mode, false, errors, `${phase}Delete`, [ - { - path: ['deleteUsers', 1], - messages: [`User: ${message}`], - errors: [ + // Valid users are returned, invalid come back as null + expect(data).toEqual({ + deleteUsers: [ + { id: users[0].id, name: 'good 1' }, + null, + { id: users[2].id, name: 'good 3' }, + null, + ], + }); + // The invalid deletes should have errors which point to the nulls in their path + const message = `Simulated error: ${phase}Delete`; + expectExtensionError(mode, useHttp, debug, errors, `${phase}Delete`, [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Delete .${__filename}`) - ), + path: ['deleteUsers', 1], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Delete .${__filename}`) + ), + }, + ], }, - ], - }, - { - path: ['deleteUsers', 3], - messages: [`User: ${message}`], - errors: [ { - message, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message}\n[^\n]*${phase}Delete .${__filename}`) - ), + path: ['deleteUsers', 3], + messages: [`User: ${message}`], + debug: [ + { + message, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message}\n[^\n]*${phase}Delete .${__filename}`) + ), + }, + ], }, - ], - }, - ]); + ]); - // Three users should still exist in the database for `before`, only 1 for `after`. - const _users = await context.lists.User.findMany({ - orderBy: { name: 'asc' }, - query: 'id name', - }); - expect(_users.map(({ name }) => name)).toEqual( - phase === 'before' - ? ['good 5', 'trigger before delete', 'trigger before delete'] - : ['good 5'] + // Three users should still exist in the database for `before`, only 1 for `after`. + const _users = await context.lists.User.findMany({ + orderBy: { name: 'asc' }, + query: 'id name', + }); + expect(_users.map(({ name }) => name)).toEqual( + phase === 'before' + ? ['good 5', 'trigger before delete', 'trigger before delete'] + : ['good 5'] + ); + }) ); - }) - ); - }); - }); + }); + }); - ['before', 'after'].map(phase => { - describe(`Field Hooks: ${phase}Change/${phase}Delete()`, () => { - test( - 'update', - runner(async ({ context }) => { - const post = await context.lists.Post.createOne({ - data: { title: 'original title', content: 'original content' }, - }); + ['before', 'after'].map(phase => { + describe(`Field Hooks: ${phase}Change/${phase}Delete()`, () => { + test( + 'update', + runner(debug)(async ({ context, graphQLRequest }) => { + const post = await context.lists.Post.createOne({ + data: { title: 'original title', content: 'original content' }, + }); - const { data, errors } = await context.graphql.raw({ - query: `mutation ($id: ID! $data: PostUpdateInput!) { updatePost(where: { id: $id }, data: $data) { id } }`, - variables: { - id: post.id, - data: { title: `trigger ${phase}`, content: `trigger ${phase}` }, - }, - }); - const message1 = `Simulated error: title: ${phase}Change`; - const message2 = `Simulated error: content: ${phase}Change`; - expectExtensionError(mode, false, errors, `${phase}Change`, [ - { - path: ['updatePost'], - messages: [`Post.title: ${message1}`, `Post.content: ${message2}`], - errors: [ - { - message: message1, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message1}\n[^\n]*${phase}Change .${__filename}`) - ), + const { data, errors } = await runQuery(context, graphQLRequest, { + query: `mutation ($id: ID! $data: PostUpdateInput!) { updatePost(where: { id: $id }, data: $data) { id } }`, + variables: { + id: post.id, + data: { title: `trigger ${phase}`, content: `trigger ${phase}` }, }, + }); + const message1 = `Simulated error: title: ${phase}Change`; + const message2 = `Simulated error: content: ${phase}Change`; + expectExtensionError(mode, useHttp, debug, errors, `${phase}Change`, [ { - message: message2, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message2}\n[^\n]*${phase}Change .${__filename}`) - ), + path: ['updatePost'], + messages: [`Post.title: ${message1}`, `Post.content: ${message2}`], + debug: [ + { + message: message1, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message1}\n[^\n]*${phase}Change .${__filename}`) + ), + }, + { + message: message2, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message2}\n[^\n]*${phase}Change .${__filename}`) + ), + }, + ], }, - ], - }, - ]); - expect(data).toEqual({ updatePost: null }); + ]); + expect(data).toEqual({ updatePost: null }); - // Post should have its original data for 'before', and the new data for 'after'. - const _post = await context.lists.Post.findOne({ - where: { id: post.id }, - query: 'title content', - }); - expect(_post).toEqual( - phase === 'before' - ? { title: 'original title', content: 'original content' } - : { title: 'trigger after', content: 'trigger after' } + // Post should have its original data for 'before', and the new data for 'after'. + const _post = await context.lists.Post.findOne({ + where: { id: post.id }, + query: 'title content', + }); + expect(_post).toEqual( + phase === 'before' + ? { title: 'original title', content: 'original content' } + : { title: 'trigger after', content: 'trigger after' } + ); + }) ); - }) - ); - test( - `delete`, - runner(async ({ context, graphQLRequest }) => { - const post = await context.lists.Post.createOne({ - data: { title: `trigger ${phase} delete`, content: `trigger ${phase} delete` }, - }); - const { body } = await graphQLRequest({ - query: `mutation ($id: ID!) { deletePost(where: { id: $id }) { id } }`, - variables: { id: post.id }, - }); - const { data, errors } = body; - const message1 = `Simulated error: title: ${phase}Delete`; - const message2 = `Simulated error: content: ${phase}Delete`; - expectExtensionError(mode, true, errors, `${phase}Delete`, [ - { - path: ['deletePost'], - messages: [`Post.title: ${message1}`, `Post.content: ${message2}`], - errors: [ + test( + `delete`, + runner(debug)(async ({ context, graphQLRequest }) => { + const post = await context.lists.Post.createOne({ + data: { title: `trigger ${phase} delete`, content: `trigger ${phase} delete` }, + }); + const { data, errors } = await runQuery(context, graphQLRequest, { + query: `mutation ($id: ID!) { deletePost(where: { id: $id }) { id } }`, + variables: { id: post.id }, + }); + const message1 = `Simulated error: title: ${phase}Delete`; + const message2 = `Simulated error: content: ${phase}Delete`; + expectExtensionError(mode, useHttp, debug, errors, `${phase}Delete`, [ { - message: message1, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message1}\n[^\n]*${phase}Delete .${__filename}`) - ), + path: ['deletePost'], + messages: [`Post.title: ${message1}`, `Post.content: ${message2}`], + debug: [ + { + message: message1, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message1}\n[^\n]*${phase}Delete .${__filename}`) + ), + }, + { + message: message2, + stacktrace: expect.stringMatching( + new RegExp(`Error: ${message2}\n[^\n]*${phase}Delete .${__filename}`) + ), + }, + ], }, - { - message: message2, - stacktrace: expect.stringMatching( - new RegExp(`Error: ${message2}\n[^\n]*${phase}Delete .${__filename}`) - ), - }, - ], - }, - ]); - expect(data).toEqual({ deletePost: null }); + ]); + expect(data).toEqual({ deletePost: null }); - // Post should have its original data for 'before', and not exist for 'after'. - const result = await context.graphql.raw({ - query: `query ($id: ID!) { post(where: { id: $id }) { title content} }`, - variables: { id: post.id }, - }); - if (phase === 'before') { - expect(result.errors).toBe(undefined); - expect(result.data).toEqual({ - post: { title: 'trigger before delete', content: 'trigger before delete' }, - }); - } else { - expectAccessDenied(result.errors, [{ path: ['post'] }]); - expect(result.data).toEqual({ post: null }); - } - }) - ); - }); - }); - }) -); + // Post should have its original data for 'before', and not exist for 'after'. + const result = await runQuery(context, graphQLRequest, { + query: `query ($id: ID!) { post(where: { id: $id }) { title content} }`, + variables: { id: post.id }, + }); + if (phase === 'before') { + expect(result.errors).toBe(undefined); + expect(result.data).toEqual({ + post: { title: 'trigger before delete', content: 'trigger before delete' }, + }); + } else { + expectAccessDenied(mode, useHttp, debug, result.errors, [{ path: ['post'] }]); + expect(result.data).toEqual({ post: null }); + } + }) + ); + }); + }); + }) + ); + }); +}); diff --git a/tests/api-tests/relationships/nested-mutations/create-many.test.ts b/tests/api-tests/relationships/nested-mutations/create-many.test.ts index 92b0dcb2226..9d63f120fc2 100644 --- a/tests/api-tests/relationships/nested-mutations/create-many.test.ts +++ b/tests/api-tests/relationships/nested-mutations/create-many.test.ts @@ -218,7 +218,9 @@ describe('with access control', () => { }); expect(data).toEqual({ createUserToNotesNoRead: { id: expect.any(String), notes: null } }); - expectAccessDenied(errors, [{ path: ['createUserToNotesNoRead', 'notes'] }]); + expectAccessDenied('dev', false, undefined, errors, [ + { path: ['createUserToNotesNoRead', 'notes'] }, + ]); }) ); diff --git a/tests/api-tests/utils.ts b/tests/api-tests/utils.ts index cdda1f790bf..acbeb667dcd 100644 --- a/tests/api-tests/utils.ts +++ b/tests/api-tests/utils.ts @@ -55,17 +55,36 @@ export const expectGraphQLValidationError = ( }; export const expectAccessDenied = ( + mode: 'dev' | 'production', + httpQuery: boolean, + _debug: boolean | undefined, errors: readonly any[] | undefined, args: { path: (string | number)[] }[] ) => { const unpackedErrors = (errors || []).map(({ locations, ...unpacked }) => ({ ...unpacked, })); + const message = 'You do not have access to this resource'; + // We expect to see debug details if: + // - httpQuery is false + // - graphql.debug is true or + // - graphql.debug is undefined and mode !== production or + const expectDebug = + _debug === true || (_debug === undefined && mode !== 'production') || !httpQuery; + // We expect to see the Apollo exception under the same conditions, but only if + // httpQuery is also true. + const expectException = httpQuery && expectDebug; + expect(unpackedErrors).toEqual( args.map(({ path }) => ({ - extensions: { code: undefined }, + extensions: { + code: httpQuery ? 'INTERNAL_SERVER_ERROR' : undefined, + ...(expectException + ? { exception: { stacktrace: expect.arrayContaining([`Error: ${message}`]) } } + : {}), + }, path, - message: 'You do not have access to this resource', + message, })) ); }; @@ -89,23 +108,35 @@ export const expectValidationError = ( export const expectExtensionError = ( mode: 'dev' | 'production', httpQuery: boolean, + _debug: boolean | undefined, errors: readonly any[] | undefined, extensionName: string, - args: { path: (string | number)[]; messages: string[]; errors: any[] }[] + args: { path: (string | number)[]; messages: string[]; debug: any[] }[] ) => { const unpackedErrors = unpackErrors(errors); expect(unpackedErrors).toEqual( - args.map(({ path, messages, errors }) => { + args.map(({ path, messages, debug }) => { const message = `An error occured while running "${extensionName}".\n${j(messages)}`; const stacktrace = message.split('\n'); stacktrace[0] = `Error: ${stacktrace[0]}`; + + // We expect to see debug details if: + // - httpQuery is false + // - graphql.debug is true or + // - graphql.debug is undefined and mode !== production or + const expectDebug = + _debug === true || (_debug === undefined && mode !== 'production') || !httpQuery; + // We expect to see the Apollo exception under the same conditions, but only if + // httpQuery is also true. + const expectException = httpQuery && expectDebug; + return { extensions: { code: 'INTERNAL_SERVER_ERROR', - ...(httpQuery && mode !== 'production' - ? { exception: { errors, stacktrace: expect.arrayContaining(stacktrace) } } + ...(expectException + ? { exception: { debug, stacktrace: expect.arrayContaining(stacktrace) } } : {}), - ...(mode !== 'production' ? { errors } : {}), + ...(expectDebug ? { debug } : {}), }, path, message,