Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: show detailed validation errors in error.cause #6551

Merged
merged 7 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/db-mongodb/src/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { Create, Document, PayloadRequestWithData } from 'payload'

import type { MongooseAdapter } from './index.js'

import handleError from './utilities/handleError.js'
import { handleError } from './utilities/handleError.js'
import { withSession } from './withSession.js'

export const create: Create = async function create(
Expand All @@ -15,7 +15,7 @@ export const create: Create = async function create(
try {
;[doc] = await Model.create([data], options)
} 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
Expand Down
4 changes: 2 additions & 2 deletions packages/db-mongodb/src/updateOne.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { PayloadRequestWithData, UpdateOne } from 'payload'

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 { withSession } from './withSession.js'

Expand All @@ -29,7 +29,7 @@ export const updateOne: UpdateOne = async function updateOne(
try {
result = await Model.findOneAndUpdate(query, data, options)
} catch (error) {
handleError(error, req)
handleError({ collection, error, req })
}

result = JSON.parse(JSON.stringify(result))
Expand Down
30 changes: 21 additions & 9 deletions packages/db-mongodb/src/utilities/handleError.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,30 @@
import httpStatus from 'http-status'
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
if (error.code === 11000 && error.keyValue) {
throw new ValidationError(
[
{
field: Object.keys(error.keyValue)[0],
message: req.t('error:valueMustBeUnique'),
},
],
{
collection,
errors: [
{
field: Object.keys(error.keyValue)[0],
message: req.t('error:valueMustBeUnique'),
},
],
global,
},
req.t,
)
} else if (error.code === 11000) {
Expand All @@ -19,5 +33,3 @@ const handleError = (error, req) => {
throw error
}
}

export default handleError
14 changes: 8 additions & 6 deletions packages/db-postgres/src/upsertRow/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,12 +313,14 @@ export const upsertRow = async <T extends Record<string, unknown> | TypeWithID>(
} catch (error) {
throw error.code === '23505'
? new ValidationError(
[
{
field: adapter.fieldConstraints[tableName][error.constraint],
message: req.t('error:valueMustBeUnique'),
},
],
{
errors: [
{
field: adapter.fieldConstraints[tableName][error.constraint],
message: req.t('error:valueMustBeUnique'),
},
],
},
req.t,
)
: error
Expand Down
10 changes: 8 additions & 2 deletions packages/payload/src/auth/operations/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,16 @@ export const loginOperation = async <TSlug extends CollectionSlug>(
const { email: unsanitizedEmail, password } = data

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() === '') {
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
Expand Down
5 changes: 4 additions & 1 deletion packages/payload/src/auth/operations/resetPassword.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

// 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.hash = hash
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import crypto from 'crypto'

import type { SanitizedCollectionConfig } from '../../../collections/config/types.js'

import { ValidationError } from '../../../errors/index.js'

const defaultPasswordValidator = (password: string): string | true => {
Expand All @@ -24,16 +26,21 @@ function pbkdf2Promisified(password: string, salt: string): Promise<Buffer> {
}

type Args = {
collection: SanitizedCollectionConfig
password: string
}

export const generatePasswordSaltHash = async ({
collection,
password,
}: Args): Promise<{ hash: string; salt: string }> => {
const validationResult = defaultPasswordValidator(password)

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()
Expand Down
9 changes: 5 additions & 4 deletions packages/payload/src/auth/strategies/local/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ export const registerLocalStrategy = async ({
})

if (existingUser.docs.length > 0) {
throw new ValidationError([
{ field: 'email', message: req.t('error:userEmailAlreadyRegistered') },
])
throw new ValidationError({
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 }
if (sanitizedDoc.password) delete sanitizedDoc.password
Expand Down
5 changes: 4 additions & 1 deletion packages/payload/src/collections/operations/updateByID.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,10 @@ export const updateByIDOperation = async <TSlug extends CollectionSlug>(
const dataToUpdate: Record<string, unknown> = { ...result }

if (shouldSavePassword && typeof password === 'string') {
const { hash, salt } = await generatePasswordSaltHash({ password })
const { hash, salt } = await generatePasswordSaltHash({
collection: collectionConfig,
password,
})
dataToUpdate.salt = salt
dataToUpdate.hash = hash
delete dataToUpdate.password
Expand Down
5 changes: 4 additions & 1 deletion packages/payload/src/errors/APIError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ class ExtendableError<TData extends object = { [key: string]: unknown }> extends
status: number

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.message = message
this.status = status
Expand Down
21 changes: 16 additions & 5 deletions packages/payload/src/errors/ValidationError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,25 @@ import httpStatus from 'http-status'

import { APIError } from './APIError.js'

export class ValidationError extends APIError<{ field: string; message: string }[]> {
constructor(results: { field: string; message: string }[], t?: TFunction) {
export class ValidationError extends APIError<{
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
? t('error:followingFieldsInvalid', { count: results.length })
: results.length === 1
? t('error:followingFieldsInvalid', { count: results.errors.length })
: results.errors.length === 1
? en.translations.error.followingFieldsInvalid_one
: 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,
)
}
}
9 changes: 8 additions & 1 deletion packages/payload/src/fields/hooks/beforeChange/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,14 @@ export const beforeChange = async <T extends Record<string, unknown>>({
})

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) => {
Expand Down
2 changes: 1 addition & 1 deletion packages/payload/src/utilities/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const pinoPretty = (pinoPrettyImport.default ||

export type PayloadLogger = pinoImport.default.Logger

const prettyOptions = {
const prettyOptions: pinoPrettyImport.PrettyOptions = {
colorize: true,
ignore: 'pid,hostname',
translateTime: 'SYS:HH:MM:ss',
Expand Down
4 changes: 2 additions & 2 deletions packages/ui/src/forms/Form/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,8 @@ export const Form: React.FC<FormProps> = (props) => {
newNonFieldErrs.push(err)
}

if (Array.isArray(err?.data)) {
err.data.forEach((dataError) => {
if (Array.isArray(err?.data?.errors)) {
err.data?.errors.forEach((dataError) => {
if (dataError?.field) {
newFieldErrs.push(dataError)
} else {
Expand Down
14 changes: 8 additions & 6 deletions test/collections-graphql/int.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1185,24 +1185,26 @@ describe('collections-graphql', () => {
expect(errors[0].message).toEqual('The following field is invalid: password')
expect(errors[0].path[0]).toEqual('test2')
expect(errors[0].extensions.name).toEqual('ValidationError')
expect(errors[0].extensions.data[0].message).toEqual('No password was given')
expect(errors[0].extensions.data[0].field).toEqual('password')
expect(errors[0].extensions.data.errors[0].message).toEqual('No password was given')
expect(errors[0].extensions.data.errors[0].field).toEqual('password')

expect(Array.isArray(errors[1].locations)).toEqual(true)
expect(errors[1].message).toEqual('The following field is invalid: email')
expect(errors[1].path[0]).toEqual('test3')
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.',
)
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(errors[2].message).toEqual('The following field is invalid: email')
expect(errors[2].path[0]).toEqual('test4')
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[0].field).toEqual('email')
expect(errors[2].extensions.data.errors[0].message).toEqual(
'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 () => {
Expand Down