feat!: show detailed validation errors in console (#6551)

BREAKING: `ValidationError` now requires the `global` or `collection`
slug, as well as an `errors` property. The actual errors are no longer
at the top-level.
This commit is contained in:
Alessio Gravili
2024-06-28 16:35:35 -04:00
committed by GitHub
parent 559c0646fa
commit 8f346dfb62
15 changed files with 101 additions and 44 deletions

View File

@@ -2,7 +2,7 @@ import type { Create, Document, PayloadRequestWithData } from 'payload'
import type { MongooseAdapter } from './index.js' import type { MongooseAdapter } from './index.js'
import handleError from './utilities/handleError.js' import { handleError } from './utilities/handleError.js'
import { withSession } from './withSession.js' import { withSession } from './withSession.js'
export const create: Create = async function create( export const create: Create = async function create(
@@ -15,7 +15,7 @@ export const create: Create = async function create(
try { try {
;[doc] = await Model.create([data], options) ;[doc] = await Model.create([data], options)
} catch (error) { } catch (error) {
handleError(error, req) handleError({ collection, error, req })
} }
// doc.toJSON does not do stuff like converting ObjectIds to string, or date strings to date objects. That's why we use JSON.parse/stringify here // doc.toJSON does not do stuff like converting ObjectIds to string, or date strings to date objects. That's why we use JSON.parse/stringify here

View File

@@ -2,7 +2,7 @@ import type { PayloadRequestWithData, UpdateOne } from 'payload'
import type { MongooseAdapter } from './index.js' import type { MongooseAdapter } from './index.js'
import handleError from './utilities/handleError.js' import { handleError } from './utilities/handleError.js'
import sanitizeInternalFields from './utilities/sanitizeInternalFields.js' import sanitizeInternalFields from './utilities/sanitizeInternalFields.js'
import { withSession } from './withSession.js' import { withSession } from './withSession.js'
@@ -29,7 +29,7 @@ export const updateOne: UpdateOne = async function updateOne(
try { try {
result = await Model.findOneAndUpdate(query, data, options) result = await Model.findOneAndUpdate(query, data, options)
} catch (error) { } catch (error) {
handleError(error, req) handleError({ collection, error, req })
} }
result = JSON.parse(JSON.stringify(result)) result = JSON.parse(JSON.stringify(result))

View File

@@ -1,16 +1,30 @@
import httpStatus from 'http-status' import httpStatus from 'http-status'
import { APIError, ValidationError } from 'payload' import { APIError, ValidationError } from 'payload'
const handleError = (error, req) => { export const handleError = ({
collection,
error,
global,
req,
}: {
collection?: string
error
global?: string
req
}) => {
// Handle uniqueness error from MongoDB // Handle uniqueness error from MongoDB
if (error.code === 11000 && error.keyValue) { if (error.code === 11000 && error.keyValue) {
throw new ValidationError( throw new ValidationError(
[ {
{ collection,
field: Object.keys(error.keyValue)[0], errors: [
message: req.t('error:valueMustBeUnique'), {
}, field: Object.keys(error.keyValue)[0],
], message: req.t('error:valueMustBeUnique'),
},
],
global,
},
req.t, req.t,
) )
} else if (error.code === 11000) { } else if (error.code === 11000) {
@@ -19,5 +33,3 @@ const handleError = (error, req) => {
throw error throw error
} }
} }
export default handleError

View File

@@ -313,12 +313,14 @@ export const upsertRow = async <T extends Record<string, unknown> | TypeWithID>(
} catch (error) { } catch (error) {
throw error.code === '23505' throw error.code === '23505'
? new ValidationError( ? new ValidationError(
[ {
{ errors: [
field: adapter.fieldConstraints[tableName][error.constraint], {
message: req.t('error:valueMustBeUnique'), field: adapter.fieldConstraints[tableName][error.constraint],
}, message: req.t('error:valueMustBeUnique'),
], },
],
},
req.t, req.t,
) )
: error : error

View File

@@ -83,10 +83,16 @@ export const loginOperation = async <TSlug extends CollectionSlug>(
const { email: unsanitizedEmail, password } = data const { email: unsanitizedEmail, password } = data
if (typeof unsanitizedEmail !== 'string' || unsanitizedEmail.trim() === '') { if (typeof unsanitizedEmail !== 'string' || unsanitizedEmail.trim() === '') {
throw new ValidationError([{ field: 'email', message: req.i18n.t('validation:required') }]) throw new ValidationError({
collection: collectionConfig.slug,
errors: [{ field: 'email', message: req.i18n.t('validation:required') }],
})
} }
if (typeof password !== 'string' || password.trim() === '') { if (typeof password !== 'string' || password.trim() === '') {
throw new ValidationError([{ field: 'password', message: req.i18n.t('validation:required') }]) throw new ValidationError({
collection: collectionConfig.slug,
errors: [{ field: 'password', message: req.i18n.t('validation:required') }],
})
} }
const email = unsanitizedEmail ? unsanitizedEmail.toLowerCase().trim() : null const email = unsanitizedEmail ? unsanitizedEmail.toLowerCase().trim() : null

View File

@@ -67,7 +67,10 @@ export const resetPasswordOperation = async (args: Arguments): Promise<Result> =
if (!user) throw new APIError('Token is either invalid or has expired.', httpStatus.FORBIDDEN) if (!user) throw new APIError('Token is either invalid or has expired.', httpStatus.FORBIDDEN)
// TODO: replace this method // TODO: replace this method
const { hash, salt } = await generatePasswordSaltHash({ password: data.password }) const { hash, salt } = await generatePasswordSaltHash({
collection: collectionConfig,
password: data.password,
})
user.salt = salt user.salt = salt
user.hash = hash user.hash = hash

View File

@@ -1,5 +1,7 @@
import crypto from 'crypto' import crypto from 'crypto'
import type { SanitizedCollectionConfig } from '../../../collections/config/types.js'
import { ValidationError } from '../../../errors/index.js' import { ValidationError } from '../../../errors/index.js'
const defaultPasswordValidator = (password: string): string | true => { const defaultPasswordValidator = (password: string): string | true => {
@@ -24,16 +26,21 @@ function pbkdf2Promisified(password: string, salt: string): Promise<Buffer> {
} }
type Args = { type Args = {
collection: SanitizedCollectionConfig
password: string password: string
} }
export const generatePasswordSaltHash = async ({ export const generatePasswordSaltHash = async ({
collection,
password, password,
}: Args): Promise<{ hash: string; salt: string }> => { }: Args): Promise<{ hash: string; salt: string }> => {
const validationResult = defaultPasswordValidator(password) const validationResult = defaultPasswordValidator(password)
if (typeof validationResult === 'string') { if (typeof validationResult === 'string') {
throw new ValidationError([{ field: 'password', message: validationResult }]) throw new ValidationError({
collection: collection?.slug,
errors: [{ field: 'password', message: validationResult }],
})
} }
const saltBuffer = await randomBytes() const saltBuffer = await randomBytes()

View File

@@ -34,12 +34,13 @@ export const registerLocalStrategy = async ({
}) })
if (existingUser.docs.length > 0) { if (existingUser.docs.length > 0) {
throw new ValidationError([ throw new ValidationError({
{ field: 'email', message: req.t('error:userEmailAlreadyRegistered') }, collection: collection.slug,
]) errors: [{ field: 'email', message: req.t('error:userEmailAlreadyRegistered') }],
})
} }
const { hash, salt } = await generatePasswordSaltHash({ password }) const { hash, salt } = await generatePasswordSaltHash({ collection, password })
const sanitizedDoc = { ...doc } const sanitizedDoc = { ...doc }
if (sanitizedDoc.password) delete sanitizedDoc.password if (sanitizedDoc.password) delete sanitizedDoc.password

View File

@@ -260,7 +260,10 @@ export const updateByIDOperation = async <TSlug extends CollectionSlug>(
const dataToUpdate: Record<string, unknown> = { ...result } const dataToUpdate: Record<string, unknown> = { ...result }
if (shouldSavePassword && typeof password === 'string') { if (shouldSavePassword && typeof password === 'string') {
const { hash, salt } = await generatePasswordSaltHash({ password }) const { hash, salt } = await generatePasswordSaltHash({
collection: collectionConfig,
password,
})
dataToUpdate.salt = salt dataToUpdate.salt = salt
dataToUpdate.hash = hash dataToUpdate.hash = hash
delete dataToUpdate.password delete dataToUpdate.password

View File

@@ -11,7 +11,10 @@ class ExtendableError<TData extends object = { [key: string]: unknown }> extends
status: number status: number
constructor(message: string, status: number, data: TData, isPublic: boolean) { constructor(message: string, status: number, data: TData, isPublic: boolean) {
super(message) super(message, {
// show data in cause
cause: data,
})
this.name = this.constructor.name this.name = this.constructor.name
this.message = message this.message = message
this.status = status this.status = status

View File

@@ -5,14 +5,25 @@ import httpStatus from 'http-status'
import { APIError } from './APIError.js' import { APIError } from './APIError.js'
export class ValidationError extends APIError<{ field: string; message: string }[]> { export class ValidationError extends APIError<{
constructor(results: { field: string; message: string }[], t?: TFunction) { collection?: string
errors: { field: string; message: string }[]
global?: string
}> {
constructor(
results: { collection?: string; errors: { field: string; message: string }[]; global?: string },
t?: TFunction,
) {
const message = t const message = t
? t('error:followingFieldsInvalid', { count: results.length }) ? t('error:followingFieldsInvalid', { count: results.errors.length })
: results.length === 1 : results.errors.length === 1
? en.translations.error.followingFieldsInvalid_one ? en.translations.error.followingFieldsInvalid_one
: en.translations.error.followingFieldsInvalid_other : en.translations.error.followingFieldsInvalid_other
super(`${message} ${results.map((f) => f.field).join(', ')}`, httpStatus.BAD_REQUEST, results) super(
`${message} ${results.errors.map((f) => f.field).join(', ')}`,
httpStatus.BAD_REQUEST,
results,
)
} }
} }

View File

@@ -69,7 +69,14 @@ export const beforeChange = async <T extends Record<string, unknown>>({
}) })
if (errors.length > 0) { if (errors.length > 0) {
throw new ValidationError(errors, req.t) throw new ValidationError(
{
collection: collection?.slug,
errors,
global: global?.slug,
},
req.t,
)
} }
await mergeLocaleActions.reduce(async (priorAction, action) => { await mergeLocaleActions.reduce(async (priorAction, action) => {

View File

@@ -7,7 +7,7 @@ const pinoPretty = (pinoPrettyImport.default ||
export type PayloadLogger = pinoImport.default.Logger export type PayloadLogger = pinoImport.default.Logger
const prettyOptions = { const prettyOptions: pinoPrettyImport.PrettyOptions = {
colorize: true, colorize: true,
ignore: 'pid,hostname', ignore: 'pid,hostname',
translateTime: 'SYS:HH:MM:ss', translateTime: 'SYS:HH:MM:ss',

View File

@@ -339,8 +339,8 @@ export const Form: React.FC<FormProps> = (props) => {
newNonFieldErrs.push(err) newNonFieldErrs.push(err)
} }
if (Array.isArray(err?.data)) { if (Array.isArray(err?.data?.errors)) {
err.data.forEach((dataError) => { err.data?.errors.forEach((dataError) => {
if (dataError?.field) { if (dataError?.field) {
newFieldErrs.push(dataError) newFieldErrs.push(dataError)
} else { } else {

View File

@@ -1185,24 +1185,26 @@ describe('collections-graphql', () => {
expect(errors[0].message).toEqual('The following field is invalid: password') expect(errors[0].message).toEqual('The following field is invalid: password')
expect(errors[0].path[0]).toEqual('test2') expect(errors[0].path[0]).toEqual('test2')
expect(errors[0].extensions.name).toEqual('ValidationError') expect(errors[0].extensions.name).toEqual('ValidationError')
expect(errors[0].extensions.data[0].message).toEqual('No password was given') expect(errors[0].extensions.data.errors[0].message).toEqual('No password was given')
expect(errors[0].extensions.data[0].field).toEqual('password') expect(errors[0].extensions.data.errors[0].field).toEqual('password')
expect(Array.isArray(errors[1].locations)).toEqual(true) expect(Array.isArray(errors[1].locations)).toEqual(true)
expect(errors[1].message).toEqual('The following field is invalid: email') expect(errors[1].message).toEqual('The following field is invalid: email')
expect(errors[1].path[0]).toEqual('test3') expect(errors[1].path[0]).toEqual('test3')
expect(errors[1].extensions.name).toEqual('ValidationError') expect(errors[1].extensions.name).toEqual('ValidationError')
expect(errors[1].extensions.data[0].message).toEqual( expect(errors[1].extensions.data.errors[0].message).toEqual(
'A user with the given email is already registered.', 'A user with the given email is already registered.',
) )
expect(errors[1].extensions.data[0].field).toEqual('email') expect(errors[1].extensions.data.errors[0].field).toEqual('email')
expect(Array.isArray(errors[2].locations)).toEqual(true) expect(Array.isArray(errors[2].locations)).toEqual(true)
expect(errors[2].message).toEqual('The following field is invalid: email') expect(errors[2].message).toEqual('The following field is invalid: email')
expect(errors[2].path[0]).toEqual('test4') expect(errors[2].path[0]).toEqual('test4')
expect(errors[2].extensions.name).toEqual('ValidationError') expect(errors[2].extensions.name).toEqual('ValidationError')
expect(errors[2].extensions.data[0].message).toEqual('Please enter a valid email address.') expect(errors[2].extensions.data.errors[0].message).toEqual(
expect(errors[2].extensions.data[0].field).toEqual('email') 'Please enter a valid email address.',
)
expect(errors[2].extensions.data.errors[0].field).toEqual('email')
}) })
it('should return the minimum allowed information about internal errors', async () => { it('should return the minimum allowed information about internal errors', async () => {