From 6b61714d7e8ced1b0f9fc0b84abddbbb0d87db7a Mon Sep 17 00:00:00 2001 From: Dan Ribbens Date: Tue, 1 Dec 2020 09:46:57 -0500 Subject: [PATCH 1/3] types for error handler --- src/config/types.ts | 2 +- src/errors/APIError.ts | 6 +++--- src/express/middleware/errorHandler.ts | 12 +++++------- src/express/responses/formatError.ts | 10 ++++++---- src/index.ts | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/config/types.ts b/src/config/types.ts index 2d0bc4942f..345ef9f7e8 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -127,7 +127,7 @@ export type PayloadConfig = { components?: { [key: string]: JSX.Element | (() => JSX.Element) }; paths?: { [key: string]: string }; hooks?: { - afterError?: (err: Error, res: unknown) => { response: unknown, status: number} | unknown; + afterError?: (err: Error, res: unknown) => { response: any, status: number } | void; }; webpack?: (config: Configuration) => Configuration; serverModules?: string[]; diff --git a/src/errors/APIError.ts b/src/errors/APIError.ts index dca496770f..b800f7be2d 100644 --- a/src/errors/APIError.ts +++ b/src/errors/APIError.ts @@ -7,13 +7,13 @@ import httpStatus from 'http-status'; class ExtendableError extends Error { status: number; - data: any; + data: {[key: string]: unknown}; isPublic: boolean; isOperational: boolean; - constructor(message: string, status: number, data: any, isPublic: boolean) { + constructor(message: string, status: number, data: { [key: string]: unknown }, isPublic: boolean) { super(message); this.name = this.constructor.name; this.message = message; @@ -39,7 +39,7 @@ class APIError extends ExtendableError { * @param {object} data - response data to be returned. * @param {boolean} isPublic - Whether the message should be visible to user or not. */ - constructor(message: string, status: number = httpStatus.INTERNAL_SERVER_ERROR, data: any = null, isPublic = false) { + constructor(message: string, status: number = httpStatus.INTERNAL_SERVER_ERROR, data: { [key: string]: unknown } = null, isPublic = false) { super(message, status, data, isPublic); } } diff --git a/src/express/middleware/errorHandler.ts b/src/express/middleware/errorHandler.ts index 28b2e8595a..b188b1ce1a 100644 --- a/src/express/middleware/errorHandler.ts +++ b/src/express/middleware/errorHandler.ts @@ -1,28 +1,26 @@ /* eslint-disable @typescript-eslint/no-unused-vars */ import httpStatus from 'http-status'; import { Response, NextFunction } from 'express'; +import { Logger } from 'pino'; import { Config } from '../../config/types'; import formatErrorResponse, { ErrorResponse } from '../responses/formatError'; import { PayloadRequest } from '../types/payloadRequest'; +import APIError from '../../errors/APIError'; export type ErrorHandler = (err: Error, req: PayloadRequest, res: Response, next: NextFunction) => Promise> // NextFunction must be passed for Express to use this middleware as error handler -const errorHandler = (config: Config, logger) => async (err: Error, req: PayloadRequest, res: Response, next: NextFunction): Promise => { - const data = formatErrorResponse(err); +const errorHandler = (config: Config, logger: Logger) => async (err: APIError, req: PayloadRequest, res: Response, next: NextFunction): Promise => { + const errorResponse = formatErrorResponse(err); let response; let status = err.status || httpStatus.INTERNAL_SERVER_ERROR; logger.error(err.stack); if (config.debug && config.debug === true) { - data.stack = err.stack; + errorResponse.stack = err.stack; } - response = { - ...data, - }; - if (typeof config.hooks.afterError === 'function') { ({ response, status } = await config.hooks.afterError(err, response) || { response, status }); } diff --git a/src/express/responses/formatError.ts b/src/express/responses/formatError.ts index c50393a314..ff6aa835a7 100644 --- a/src/express/responses/formatError.ts +++ b/src/express/responses/formatError.ts @@ -1,8 +1,10 @@ -export type ErrorResponse = unknown; +import APIError from '../../errors/APIError'; -const formatErrorResponse = (incoming) => { +export type ErrorResponse = { errors: unknown[], data?: any, stack?: string }; + +const formatErrorResponse = (incoming: Error | APIError | { [key: string]: unknown }): ErrorResponse => { if (incoming) { - if (incoming && incoming.data && incoming.data.length > 0) { + if (incoming instanceof APIError && incoming.data && incoming.data.length > 0) { return { errors: [{ name: incoming.name, @@ -13,7 +15,7 @@ const formatErrorResponse = (incoming) => { } // mongoose - if (incoming.errors) { + if (!(incoming instanceof APIError || incoming instanceof Error) && incoming.errors) { return { errors: Object.keys(incoming.errors) .reduce((acc, key) => { diff --git a/src/index.ts b/src/index.ts index 2a2c7fc79a..d191f56b0b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -135,7 +135,7 @@ export class Payload { } // Configure email service - this.email = buildEmail(this.config.email); + this.email = buildEmail(this.emailOptions); // Initialize collections & globals initCollections(this); From 9d94c509e7be9094c45cac1124b13cc0b0895536 Mon Sep 17 00:00:00 2001 From: Dan Ribbens Date: Tue, 1 Dec 2020 11:47:07 -0500 Subject: [PATCH 2/3] bring back collection after error hook for restful calls --- src/collections/config/types.ts | 3 +++ src/config/types.ts | 4 ++-- src/express/middleware/errorHandler.spec.js | 2 ++ src/express/middleware/errorHandler.ts | 17 ++++++++++------- 4 files changed, 17 insertions(+), 9 deletions(-) diff --git a/src/collections/config/types.ts b/src/collections/config/types.ts index e93f95a7f9..161aa0bba9 100644 --- a/src/collections/config/types.ts +++ b/src/collections/config/types.ts @@ -67,6 +67,8 @@ export type AfterDeleteHook = (args?: { doc: any; }) => any; +export type AfterErrorHook = (err: Error, res: unknown) => { response: any, status: number } | void; + export type BeforeLoginHook = (args?: { req: PayloadRequest; }) => any; @@ -104,6 +106,7 @@ export type PayloadCollectionConfig = { afterRead?: AfterReadHook[]; beforeDelete?: BeforeDeleteHook[]; afterDelete?: AfterDeleteHook[]; + afterError?: AfterErrorHook; beforeLogin?: BeforeLoginHook[]; afterLogin?: AfterLoginHook[]; afterForgotPassword?: AfterForgotPasswordHook[]; diff --git a/src/config/types.ts b/src/config/types.ts index 345ef9f7e8..db37c3e7ca 100644 --- a/src/config/types.ts +++ b/src/config/types.ts @@ -5,7 +5,7 @@ import { Configuration } from 'webpack'; import SMTPConnection from 'nodemailer/lib/smtp-connection'; import { GraphQLType } from 'graphql'; import { Payload } from '..'; -import { PayloadCollectionConfig } from '../collections/config/types'; +import { AfterErrorHook, PayloadCollectionConfig } from '../collections/config/types'; import { PayloadGlobalConfig } from '../globals/config/types'; import { PayloadRequest } from '../express/types/payloadRequest'; import InitializeGraphQL from '../graphql'; @@ -127,7 +127,7 @@ export type PayloadConfig = { components?: { [key: string]: JSX.Element | (() => JSX.Element) }; paths?: { [key: string]: string }; hooks?: { - afterError?: (err: Error, res: unknown) => { response: any, status: number } | void; + afterError?: AfterErrorHook; }; webpack?: (config: Configuration) => Configuration; serverModules?: string[]; diff --git a/src/express/middleware/errorHandler.spec.js b/src/express/middleware/errorHandler.spec.js index eecd44528b..be76a55065 100644 --- a/src/express/middleware/errorHandler.spec.js +++ b/src/express/middleware/errorHandler.spec.js @@ -103,6 +103,7 @@ describe('errorHandler', () => { }, logger); await handler(testError, req, res); expect(afterError) + // eslint-disable-next-line jest/prefer-called-with .toHaveBeenCalled(); }); @@ -113,6 +114,7 @@ describe('errorHandler', () => { }, logger); await handler(testError, req, res); expect(req.collection.config.hooks.afterError) + // eslint-disable-next-line jest/prefer-called-with .toHaveBeenCalled(); }); }); diff --git a/src/express/middleware/errorHandler.ts b/src/express/middleware/errorHandler.ts index b188b1ce1a..6f5e55c342 100644 --- a/src/express/middleware/errorHandler.ts +++ b/src/express/middleware/errorHandler.ts @@ -1,24 +1,27 @@ -/* eslint-disable @typescript-eslint/no-unused-vars */ import httpStatus from 'http-status'; -import { Response, NextFunction } from 'express'; +import { Response } from 'express'; import { Logger } from 'pino'; import { Config } from '../../config/types'; import formatErrorResponse, { ErrorResponse } from '../responses/formatError'; import { PayloadRequest } from '../types/payloadRequest'; import APIError from '../../errors/APIError'; -export type ErrorHandler = (err: Error, req: PayloadRequest, res: Response, next: NextFunction) => Promise> +export type ErrorHandler = (err: Error, req: PayloadRequest, res: Response) => Promise | void> // NextFunction must be passed for Express to use this middleware as error handler -const errorHandler = (config: Config, logger: Logger) => async (err: APIError, req: PayloadRequest, res: Response, next: NextFunction): Promise => { - const errorResponse = formatErrorResponse(err); - let response; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const errorHandler = (config: Config, logger: Logger) => async (err: APIError, req: PayloadRequest, res: Response): Promise => { + let response = formatErrorResponse(err); let status = err.status || httpStatus.INTERNAL_SERVER_ERROR; logger.error(err.stack); if (config.debug && config.debug === true) { - errorResponse.stack = err.stack; + response.stack = err.stack; + } + + if (req.collection && typeof req.collection.config.hooks.afterError === 'function') { + ({ response, status } = await req.collection.config.hooks.afterError(err, response) || { response, status }); } if (typeof config.hooks.afterError === 'function') { From 4ac08dac651bda4d009dad6419461d45c96758b0 Mon Sep 17 00:00:00 2001 From: Elliot DeNolf Date: Tue, 1 Dec 2020 16:38:27 -0500 Subject: [PATCH 3/3] type some collection operations --- demo/globals/BlocksGlobal.ts | 3 ++- demo/globals/GlobalWithStrictAccess.ts | 3 ++- demo/globals/NavigationArray.ts | 3 ++- package.json | 1 + src/collections/operations/create.ts | 3 ++- src/collections/operations/delete.ts | 3 ++- src/collections/operations/find.ts | 3 ++- src/collections/operations/findByID.ts | 5 ++++- src/fields/config/types.ts | 1 - 9 files changed, 17 insertions(+), 8 deletions(-) diff --git a/demo/globals/BlocksGlobal.ts b/demo/globals/BlocksGlobal.ts index 890bf43dc2..afbadfefa3 100644 --- a/demo/globals/BlocksGlobal.ts +++ b/demo/globals/BlocksGlobal.ts @@ -1,6 +1,7 @@ import checkRole from '../access/checkRole'; import Quote from '../blocks/Quote'; import CallToAction from '../blocks/CallToAction'; +import { PayloadGlobalConfig } from '../../src/globals/config/types'; export default { slug: 'blocks-global', @@ -18,4 +19,4 @@ export default { localized: true, }, ], -}; +} as PayloadGlobalConfig; diff --git a/demo/globals/GlobalWithStrictAccess.ts b/demo/globals/GlobalWithStrictAccess.ts index c002a26dc1..772abf3945 100644 --- a/demo/globals/GlobalWithStrictAccess.ts +++ b/demo/globals/GlobalWithStrictAccess.ts @@ -1,3 +1,4 @@ +import { PayloadGlobalConfig } from '../../src/globals/config/types'; import checkRole from '../access/checkRole'; export default { @@ -31,4 +32,4 @@ export default { required: true, }, ], -}; +} as PayloadGlobalConfig; diff --git a/demo/globals/NavigationArray.ts b/demo/globals/NavigationArray.ts index 7c097af286..bf17f132de 100644 --- a/demo/globals/NavigationArray.ts +++ b/demo/globals/NavigationArray.ts @@ -1,3 +1,4 @@ +import { PayloadGlobalConfig } from '../../src/globals/config/types'; import checkRole from '../access/checkRole'; export default { @@ -24,4 +25,4 @@ export default { }], }, ], -}; +} as PayloadGlobalConfig; diff --git a/package.json b/package.json index 69a649e9dc..193897937b 100644 --- a/package.json +++ b/package.json @@ -166,6 +166,7 @@ "@types/is-hotkey": "^0.1.2", "@types/isomorphic-fetch": "^0.0.35", "@types/jest": "^26.0.15", + "@types/joi": "^14.3.4", "@types/jsonwebtoken": "^8.5.0", "@types/method-override": "^0.0.31", "@types/mini-css-extract-plugin": "^1.2.1", diff --git a/src/collections/operations/create.ts b/src/collections/operations/create.ts index b1bd2e6a8e..44baca8edb 100644 --- a/src/collections/operations/create.ts +++ b/src/collections/operations/create.ts @@ -231,7 +231,7 @@ async function create(incomingArgs) { // Send verification email if applicable // ///////////////////////////////////// - if (collectionConfig.auth && collectionConfig.auth.verify && !disableVerificationEmail) { + if (collectionConfig.auth && collectionConfig.auth.verify) { sendVerificationEmail({ config: this.config, sendEmail: this.sendEmail, @@ -239,6 +239,7 @@ async function create(incomingArgs) { user: result, token: data._verificationToken, req, + disableEmail: disableVerificationEmail, }); } diff --git a/src/collections/operations/delete.ts b/src/collections/operations/delete.ts index 32955b2ab1..6476e6f3a7 100644 --- a/src/collections/operations/delete.ts +++ b/src/collections/operations/delete.ts @@ -6,6 +6,7 @@ import { NotFound, Forbidden, ErrorDeletingFile } from '../../errors'; import executeAccess from '../../auth/executeAccess'; import fileExists from '../../uploads/fileExists'; import { BeforeOperationHook } from '../config/types'; +import { Query } from './types'; async function deleteQuery(incomingArgs) { let args = incomingArgs; @@ -56,7 +57,7 @@ async function deleteQuery(incomingArgs) { // Retrieve document // ///////////////////////////////////// - const queryToBuild = { + const queryToBuild: Query = { where: { and: [ { diff --git a/src/collections/operations/find.ts b/src/collections/operations/find.ts index cdacf3075b..31fcf9f23f 100644 --- a/src/collections/operations/find.ts +++ b/src/collections/operations/find.ts @@ -1,6 +1,7 @@ import executeAccess from '../../auth/executeAccess'; import removeInternalFields from '../../utilities/removeInternalFields'; import { BeforeOperationHook, BeforeReadHook } from '../config/types'; +import { Query } from './types'; async function find(incomingArgs) { let args = incomingArgs; @@ -39,7 +40,7 @@ async function find(incomingArgs) { // Access // ///////////////////////////////////// - const queryToBuild = {}; + const queryToBuild: Query = {}; if (where) { let and = []; diff --git a/src/collections/operations/findByID.ts b/src/collections/operations/findByID.ts index f52b404f2b..390c7249d0 100644 --- a/src/collections/operations/findByID.ts +++ b/src/collections/operations/findByID.ts @@ -4,6 +4,7 @@ import { BeforeOperationHook } from '../config/types'; import removeInternalFields from '../../utilities/removeInternalFields'; import { Forbidden, NotFound } from '../../errors'; import executeAccess from '../../auth/executeAccess'; +import { Query } from './types'; async function findByID(incomingArgs) { let args = incomingArgs; @@ -45,7 +46,7 @@ async function findByID(incomingArgs) { const accessResults = !overrideAccess ? await executeAccess({ req, disableErrors, id }, collectionConfig.access.read) : true; const hasWhereAccess = typeof accessResults === 'object'; - const queryToBuild = { + const queryToBuild: Query = { where: { and: [ { @@ -76,6 +77,8 @@ async function findByID(incomingArgs) { req.findByID[collectionConfig.slug] = memoize(nonMemoizedFindByID, { isPromise: true, maxSize: 100, + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore This is straight from their docs, bad typings transformKey: JSON.stringify, }); } diff --git a/src/fields/config/types.ts b/src/fields/config/types.ts index 476fa61ba7..afa4004d0f 100644 --- a/src/fields/config/types.ts +++ b/src/fields/config/types.ts @@ -156,7 +156,6 @@ export type RadioField = FieldBase & { value: string; label: string; }[] | string[]; - hasMany?: boolean; } export type Block = {