tabshift/payloadcms
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

feat!: improve afterError hook to accept array of functions, change to object args (#8389)

Browse Source 
Changes the `afterError` hook structure, adds tests / more docs.
Ensures that the `req.responseHeaders` property is respected in the
error handler.

**Breaking**
`afterError` now accepts an array of functions instead of a single
function:
```diff
- afterError: () => {...}
+ afterError: [() => {...}]
```

The args are changed to accept an object with the following properties:
| Argument | Description |
| ------------------- |
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
|
| **`error`** | The error that occurred. |
| **`context`** | Custom context passed between Hooks. [More
details](./context). |
| **`graphqlResult`** | The GraphQL result object, available if the hook
is executed within a GraphQL context. |
| **`req`** | The
[Request](https://developer.mozilla.org/en-US/docs/Web/API/Request)
object containing the currently authenticated `user` |
| **`collection`** | The [Collection](../configuration/collections) in
which this Hook is running against. This will be `undefined` if the hook
is executed from a non-collection endpoint or GraphQL. |
| **`result`** | The formatted error result object, available if the
hook is executed from a REST context. |
This commit is contained in:
Sasha
2024-09-24 20:29:53 +03:00
committed by GitHub
parent 6da4f06205
commit 28ea0c59e8
12 changed files with 238 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
docs/hooks/collections.mdx
View File

@@ -46,6 +46,7 @@ export const CollectionWithHooks: CollectionConfig = {
afterRead: [(args) => {...}],
afterDelete: [(args) => {...}],
afterOperation: [(args) => {...}],
afterError: [(args) => {....}],
// Auth-enabled Hooks
beforeLogin: [(args) => {...}],
@@ -289,6 +290,30 @@ The following arguments are provided to the `afterOperation` hook:
| **`operation`** | The name of the operation that this hook is running within. |
| **`result`** | The result of the operation, before modifications. |
### afterError
The `afterError` Hook is triggered when an error occurs in the Payload application. This can be useful for logging errors to a third-party service, sending an email to the development team, logging the error to Sentry or DataDog, etc. The output can be used to transform the result object / status code.
```ts
import type { CollectionAfterErrorHook } from 'payload';
const afterDeleteHook: CollectionAfterErrorHook = async ({
req,
id,
doc,
}) => {...}
```
The following arguments are provided to the `afterError` Hook:
| Argument | Description |
| ------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| **`error`** | The error that occurred. |
| **`context`** | Custom context passed between Hooks. [More details](./context). |
| **`graphqlResult`** | The GraphQL result object, available if the hook is executed within a GraphQL context. |
| **`req`** | The `PayloadRequest` object that extends [Web Request](https://developer.mozilla.org/en-US/docs/Web/API/Request). Contains currently authenticated `user` and the Local API instance `payload`. |
| **`collection`** | The [Collection](../configuration/collections) in which this Hook is running against. |
| **`result`** | The formatted error result object, available if the hook is executed from a REST context. |
### beforeLogin
For [Auth-enabled Collections](../authentication/overview), this hook runs during `login` operations where a user with the provided credentials exist, but before a token is generated and added to the response. You can optionally modify the user that is returned, or throw an error in order to deny the login operation.

21
docs/hooks/overview.mdx
View File

@@ -43,7 +43,7 @@ export default buildConfig({
// ...
// highlight-start
hooks: {
afterError: () => {...}
afterError:[() => {...}]
},
// highlight-end
})
@@ -57,7 +57,7 @@ The following options are available:
### afterError
The `afterError` Hook is triggered when an error occurs in the Payload application. This can be useful for logging errors to a third-party service, sending an email to the development team, logging the error to Sentry or DataDog, etc.
The `afterError` Hook is triggered when an error occurs in the Payload application. This can be useful for logging errors to a third-party service, sending an email to the development team, logging the error to Sentry or DataDog, etc. The output can be used to transform the result object / status code.
```ts
import { buildConfig } from 'payload'
@@ -65,20 +65,23 @@ import { buildConfig } from 'payload'
export default buildConfig({
// ...
hooks: {
afterError: async ({ error }) => {
afterError: [async ({ error }) => {
// Do something
}
}]
},
})
```
The following arguments are provided to the `afterError` Hook:
| Argument | Description |
|----------|-----------------------------------------------------------------------------------------------|
| **`error`** | The error that occurred. |
| **`context`** | Custom context passed between Hooks. [More details](./context). |
| Argument | Description |
| ------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| **`error`** | The error that occurred. |
| **`context`** | Custom context passed between Hooks. [More details](./context). |
| **`graphqlResult`** | The GraphQL result object, available if the hook is executed within a GraphQL context. |
| **`req`** | The `PayloadRequest` object that extends [Web Request](https://developer.mozilla.org/en-US/docs/Web/API/Request). Contains currently authenticated `user` and the Local API instance `payload`. |
| **`collection`** | The [Collection](../configuration/collections) in which this Hook is running against. This will be `undefined` if the hook is executed from a non-collection endpoint or GraphQL. |
| **`result`** | The formatted error result object, available if the hook is executed from a REST context. |
## Async vs. Synchronous
All Hooks can be written as either synchronous or asynchronous functions. Choosing the right type depends on your use case, but switching between the two is as simple as adding or removing the `async` keyword.

48
packages/next/src/routes/graphql/handler.ts
View File

@@ -1,5 +1,5 @@
import type { GraphQLError, GraphQLFormattedError } from 'graphql'
import type { CollectionAfterErrorHook, Payload, SanitizedConfig } from 'payload'
import type { APIError, Payload, PayloadRequest, SanitizedConfig } from 'payload'
import { configToSchema } from '@payloadcms/graphql'
import { createHandler } from 'graphql-http/lib/use/fetch'
@@ -11,28 +11,30 @@ import { createPayloadRequest } from '../../utilities/createPayloadRequest.js'
import { headersWithCors } from '../../utilities/headersWithCors.js'
import { mergeHeaders } from '../../utilities/mergeHeaders.js'
const handleError = async (
payload: Payload,
err: any,
debug: boolean,
afterErrorHook: CollectionAfterErrorHook,
// eslint-disable-next-line @typescript-eslint/require-await
): Promise<GraphQLFormattedError> => {
const status = err.originalError.status || httpStatus.INTERNAL_SERVER_ERROR
const handleError = async ({
err,
payload,
req,
}: {
err: GraphQLError
payload: Payload
req: PayloadRequest
}): Promise<GraphQLFormattedError> => {
const status = (err.originalError as APIError).status || httpStatus.INTERNAL_SERVER_ERROR
let errorMessage = err.message
payload.logger.error(err.stack)
// Internal server errors can contain anything, including potentially sensitive data.
// Therefore, error details will be hidden from the response unless `config.debug` is `true`
if (!debug && status === httpStatus.INTERNAL_SERVER_ERROR) {
if (!payload.config.debug && status === httpStatus.INTERNAL_SERVER_ERROR) {
errorMessage = 'Something went wrong.'
}
let response: GraphQLFormattedError = {
extensions: {
name: err?.originalError?.name || undefined,
data: (err && err.originalError && err.originalError.data) || undefined,
stack: debug ? err.stack : undefined,
data: (err && err.originalError && (err.originalError as APIError).data) || undefined,
stack: payload.config.debug ? err.stack : undefined,
statusCode: status,
},
locations: err.locations,
@@ -40,9 +42,20 @@ const handleError = async (
path: err.path,
}
if (afterErrorHook) {
;({ response } = afterErrorHook(err, response, null, null) || { response })
}
await payload.config.hooks.afterError?.reduce(async (promise, hook) => {
await promise
const result = await hook({
context: req.context,
error: err,
graphqlResult: response,
req,
})
if (result) {
response = result.graphqlResult || response
}
}, Promise.resolve())
return response
}
@@ -95,9 +108,6 @@ export const POST =
const { payload } = req
const afterErrorHook =
typeof payload.config.hooks.afterError === 'function' ? payload.config.hooks.afterError : null
const headers = {}
const apiResponse = await createHandler({
context: { headers, req },
@@ -113,7 +123,7 @@ export const POST =
if (response.errors) {
const errors = (await Promise.all(
result.errors.map((error) => {
return handleError(payload, error, payload.config.debug, afterErrorHook)
return handleError({ err: error, payload, req })
}),
)) as GraphQLError[]
// errors type should be FormattedGraphQLError[] but onOperation has a return type of ExecutionResult instead of FormattedExecutionResult

69
packages/next/src/routes/rest/routeError.ts
View File

@@ -1,14 +1,13 @@
import type { Collection, PayloadRequest, SanitizedConfig } from 'payload'
import type { Collection, ErrorResult, PayloadRequest, SanitizedConfig } from 'payload'
import httpStatus from 'http-status'
import { APIError, APIErrorName, ValidationErrorName } from 'payload'
import { getPayloadHMR } from '../../utilities/getPayloadHMR.js'
import { headersWithCors } from '../../utilities/headersWithCors.js'
import { mergeHeaders } from '../../utilities/mergeHeaders.js'
export type ErrorResponse = { data?: any; errors: unknown[]; stack?: string }
const formatErrors = (incoming: { [key: string]: unknown } | APIError): ErrorResponse => {
const formatErrors = (incoming: { [key: string]: unknown } | APIError): ErrorResult => {
if (incoming) {
// Cannot use `instanceof` to check error type: https://github.com/microsoft/TypeScript/issues/13965
// Instead, get the prototype of the incoming error and check its constructor name
@@ -73,14 +72,14 @@ export const routeError = async ({
collection,
config: configArg,
err,
req,
req: incomingReq,
}: {
collection?: Collection
config: Promise<SanitizedConfig> | SanitizedConfig
err: APIError
req: Partial<PayloadRequest>
req: PayloadRequest | Request
}) => {
let payload = req?.payload
let payload = 'payload' in incomingReq && incomingReq?.payload
if (!payload) {
try {
@@ -95,6 +94,8 @@ export const routeError = async ({
}
}
const req = incomingReq as PayloadRequest
req.payload = payload
const headers = headersWithCors({
headers: new Headers(),
@@ -119,26 +120,44 @@ export const routeError = async ({
response.stack = err.stack
}
if (collection && typeof collection.config.hooks.afterError === 'function') {
;({ response, status } = collection.config.hooks.afterError(
err,
response,
req?.context,
collection.config,
) || { response, status })
if (collection) {
await collection.config.hooks.afterError?.reduce(async (promise, hook) => {
await promise
const result = await hook({
collection: collection.config,
context: req.context,
error: err,
req,
result: response,
})
if (result) {
response = (result.response as ErrorResult) || response
status = result.status || status
}
}, Promise.resolve())
}
if (typeof config.hooks.afterError === 'function') {
;({ response, status } = config.hooks.afterError(
err,
response,
req?.context,
collection?.config,
) || {
response,
status,
await config.hooks.afterError?.reduce(async (promise, hook) => {
await promise
const result = await hook({
collection: collection?.config,
context: req.context,
error: err,
req,
result: response,
})
}
return Response.json(response, { headers, status })
if (result) {
response = (result.response as ErrorResult) || response
status = result.status || status
}
}, Promise.resolve())
return Response.json(response, {
headers: req.responseHeaders ? mergeHeaders(req.responseHeaders, headers) : headers,
status,
})
}

16
packages/payload/src/collections/config/types.ts
View File

@@ -16,6 +16,8 @@ import type {
import type { Auth, ClientUser, IncomingAuthType } from '../../auth/types.js'
import type {
Access,
AfterErrorHookArgs,
AfterErrorResult,
CustomComponent,
EditConfig,
Endpoint,
@@ -178,14 +180,6 @@ export type AfterOperationHook<TOperationGeneric extends CollectionSlug = string
>
>
export type AfterErrorHook = (
err: Error,
res: unknown,
context: RequestContext,
/** The collection which this hook is being run on. This is null if the AfterError hook was be added to the payload-wide config */
collection: null | SanitizedCollectionConfig,
) => { response: any; status: number } | void
export type BeforeLoginHook<T extends TypeWithID = any> = (args: {
/** The collection which this hook is being run on */
collection: SanitizedCollectionConfig
@@ -237,6 +231,10 @@ export type AfterRefreshHook<T extends TypeWithID = any> = (args: {
token: string
}) => any
export type AfterErrorHook = (
args: { collection: SanitizedCollectionConfig } & AfterErrorHookArgs,
) => AfterErrorResult | Promise<AfterErrorResult>
export type AfterForgotPasswordHook = (args: {
args?: any
/** The collection which this hook is being run on */
@@ -402,7 +400,7 @@ export type CollectionConfig<TSlug extends CollectionSlug = any> = {
hooks?: {
afterChange?: AfterChangeHook[]
afterDelete?: AfterDeleteHook[]
afterError?: AfterErrorHook
afterError?: AfterErrorHook[]
afterForgotPassword?: AfterForgotPasswordHook[]
afterLogin?: AfterLoginHook[]
afterLogout?: AfterLogoutHook[]

33
packages/payload/src/config/types.ts
View File

@@ -6,6 +6,7 @@ import type {
} from '@payloadcms/translations'
import type { BusboyConfig } from 'busboy'
import type GraphQL from 'graphql'
import type { GraphQLFormattedError } from 'graphql'
import type { JSONSchema4 } from 'json-schema'
import type { DestinationStream, pino } from 'pino'
import type React from 'react'
@@ -23,7 +24,6 @@ import type {
InternalImportMap,
} from '../bin/generateImportMap/index.js'
import type {
AfterErrorHook,
Collection,
CollectionConfig,
SanitizedCollectionConfig,
@@ -31,7 +31,7 @@ import type {
import type { DatabaseAdapterResult } from '../database/types.js'
import type { EmailAdapter, SendEmailOptions } from '../email/types.js'
import type { GlobalConfig, Globals, SanitizedGlobalConfig } from '../globals/config/types.js'
import type { Payload, TypedUser } from '../index.js'
import type { Payload, RequestContext, TypedUser } from '../index.js'
import type { PayloadRequest, Where } from '../types/index.js'
import type { PayloadLogger } from '../utilities/logger.js'
@@ -621,6 +621,33 @@ export type FetchAPIFileUploadOptions = {
useTempFiles?: boolean | undefined
} & Partial<BusboyConfig>
export type ErrorResult = { data?: any; errors: unknown[]; stack?: string }
export type AfterErrorResult = {
graphqlResult?: GraphQLFormattedError
response?: Partial<ErrorResult> & Record<string, unknown>
status?: number
} | void
export type AfterErrorHookArgs = {
/** The Collection that the hook is operating on. This will be undefined if the hook is executed from a non-collection endpoint or GraphQL. */
collection?: SanitizedCollectionConfig
/** Custom context passed between hooks */
context: RequestContext
/** The error that occurred. */
error: Error
/** The GraphQL result object, available if the hook is executed within a GraphQL context. */
graphqlResult?: GraphQLFormattedError
/** The Request object containing the currently authenticated user. */
req: PayloadRequest
/** The formatted error result object, available if the hook is executed from a REST context. */
result?: ErrorResult
}
export type AfterErrorHook = (
args: AfterErrorHookArgs,
) => AfterErrorResult | Promise<AfterErrorResult>
/**
* This is the central configuration
*
@@ -895,7 +922,7 @@ export type Config = {
* @see https://payloadcms.com/docs/hooks/overview
*/
hooks?: {
afterError?: AfterErrorHook
afterError?: AfterErrorHook[]
}
/** i18n config settings */
// eslint-disable-next-line @typescript-eslint/no-empty-object-type

5
packages/payload/src/index.ts
View File

@@ -24,6 +24,7 @@ import type {
DataFromCollectionSlug,
TypeWithID,
} from './collections/config/types.js'
export type * from './admin/types.js'
import type { Options as CountOptions } from './collections/operations/local/count.js'
import type { Options as CreateOptions } from './collections/operations/local/create.js'
import type {
@@ -31,6 +32,7 @@ import type {
ManyOptions as DeleteManyOptions,
Options as DeleteOptions,
} from './collections/operations/local/delete.js'
export type { MappedView } from './admin/views/types.js'
import type { Options as DuplicateOptions } from './collections/operations/local/duplicate.js'
import type { Options as FindOptions } from './collections/operations/local/find.js'
import type { Options as FindByIDOptions } from './collections/operations/local/findByID.js'
@@ -654,8 +656,6 @@ interface RequestContext {
// eslint-disable-next-line @typescript-eslint/no-empty-object-type
export interface DatabaseAdapter extends BaseDatabaseAdapter {}
export type { Payload, RequestContext }
export type * from './admin/types.js'
export type { MappedView } from './admin/views/types.js'
export { default as executeAccess } from './auth/executeAccess.js'
export { executeAuthStrategies } from './auth/executeAuthStrategies.js'
export { getAccessResults } from './auth/getAccessResults.js'
@@ -687,7 +687,6 @@ export type {
VerifyConfig,
} from './auth/types.js'
export { generateImportMap } from './bin/generateImportMap/index.js'
export type { ImportMap } from './bin/generateImportMap/index.js'
export { genImportMapIterateFields } from './bin/generateImportMap/iterateFields.js'
export type { ClientCollectionConfig } from './collections/config/client.js'

10
packages/plugin-sentry/src/plugin.ts
View File

@@ -16,10 +16,12 @@ export const sentryPlugin =
config.hooks = {
...(incomingConfig.hooks || {}),
// eslint-disable-next-line @typescript-eslint/no-explicit-any
afterError: (err: any) => {
captureException(err)
},
afterError: [
({ error }) => {
captureException(error)
},
],
}
config.onInit = async (payload) => {

1
test/_community/payload-types.ts
View File

@@ -186,7 +186,6 @@ export interface PayloadLockedDocument {
relationTo: 'users';
value: string | User;
} | null);
editedAt?: string | null;
globalSlug?: string | null;
user: {
relationTo: 'users';

10
test/collections-rest/config.ts
View File

@@ -2,7 +2,7 @@ import { fileURLToPath } from 'node:url'
import path from 'path'
const filename = fileURLToPath(import.meta.url)
const dirname = path.dirname(filename)
import type { CollectionConfig } from 'payload'
import { APIError, type CollectionConfig } from 'payload'
import { buildConfigWithDefaults } from '../buildConfigWithDefaults.js'
import { devUser } from '../credentials.js'
@@ -288,6 +288,14 @@ export default buildConfigWithDefaults({
method: 'get',
path: '/internal-error-here',
},
{
handler: () => {
// Throwing an internal error with potentially sensitive data
throw new APIError('Connected to the Pentagon. Secret data: ******')
},
method: 'get',
path: '/api-error-here',
},
],
onInit: async (payload) => {
await payload.create({

73
test/collections-rest/int.spec.ts
View File

@@ -1,6 +1,8 @@
import type { Payload, SanitizedCollectionConfig } from 'payload'
import { randomBytes, randomUUID } from 'crypto'
import path from 'path'
import { NotFound, type Payload } from 'payload'
import { APIError, NotFound } from 'payload'
import { fileURLToPath } from 'url'
import type { NextRESTClient } from '../helpers/NextRESTClient.js'
@@ -1533,6 +1535,75 @@ describe('collections-rest', () => {
expect(Array.isArray(result.errors)).toEqual(true)
expect(result.errors[0].message).toStrictEqual('Something went wrong.')
})
it('should execute afterError hook on root level and modify result/status', async () => {
let err: unknown
let errResult: any
payload.config.hooks.afterError = [
({ error, result }) => {
err = error
errResult = result
return { status: 400, response: { modified: true } }
},
]
const response = await restClient.GET(`/api-error-here`)
expect(response.status).toBe(400)
expect(err).toBeInstanceOf(APIError)
expect(errResult).toStrictEqual({
errors: [
{
message: 'Something went wrong.',
},
],
})
const result = await response.json()
expect(result.modified).toBe(true)
payload.config.hooks.afterError = []
})
it('should execute afterError hook on collection level and modify result', async () => {
let err: unknown
let errResult: any
let collection: SanitizedCollectionConfig
payload.collections.posts.config.hooks.afterError = [
({ error, result, collection: incomingCollection }) => {
err = error
errResult = result
collection = incomingCollection
return { response: { modified: true } }
},
]
const post = await createPost({})
const response = await restClient.GET(
`/${slug}/${typeof post.id === 'number' ? 1000 : randomUUID()}`,
)
expect(response.status).toBe(404)
expect(collection.slug).toBe(slug)
expect(err).toBeInstanceOf(NotFound)
expect(errResult).toStrictEqual({
errors: [
{
message: 'Not Found',
},
],
})
const result = await response.json()
expect(result.modified).toBe(true)
payload.collections.posts.config.hooks.afterError = []
})
})
describe('Local', () => {

4
test/hooks/config.ts
View File

@@ -45,14 +45,14 @@ export const HooksConfig: Promise<SanitizedConfig> = buildConfigWithDefaults({
},
],
hooks: {
afterError: () => console.log('Running afterError hook'),
afterError: [() => console.log('Running afterError hook')],
},
onInit: async (payload) => {
await seedHooksUsers(payload)
await payload.create({
collection: hooksSlug,
data: {
check: 'update',
check: true,
fieldBeforeValidate: false,
collectionBeforeValidate: false,
fieldBeforeChange: false,