diff --git a/packages/plugin-nested-docs/src/hooks/resaveChildren.ts b/packages/plugin-nested-docs/src/hooks/resaveChildren.ts index fc007f65e94..ac45ec5027d 100644 --- a/packages/plugin-nested-docs/src/hooks/resaveChildren.ts +++ b/packages/plugin-nested-docs/src/hooks/resaveChildren.ts @@ -97,6 +97,16 @@ export const resaveChildren = 400, ) } + + // Anything else has to propagate too. A failed child update has already + // killed the transaction the whole cascade shares, so swallowing here + // does not salvage the parent write — it only stops the caller from + // being told the write was discarded. Two ways to land in this branch: + // a non-validation failure (a database error), and a cascade deeper + // than one level, where the throw above has already turned the + // grandchild's ValidationError into an APIError that the next + // ancestor's `instanceof` check no longer recognises. + throw err } } diff --git a/test/plugin-nested-docs/collections/Pages.ts b/test/plugin-nested-docs/collections/Pages.ts index f5ca5847726..39613312949 100644 --- a/test/plugin-nested-docs/collections/Pages.ts +++ b/test/plugin-nested-docs/collections/Pages.ts @@ -1,7 +1,17 @@ -import type { CollectionConfig } from 'payload' +import type { CollectionConfig, PayloadRequest } from 'payload' import { populateFullTitle } from './populateFullTitle.js' +type Simulation = { + /** Make the child re-save throw something that is not a `ValidationError`. */ + infraFailure?: boolean + /** Make the child re-save throw a `ValidationError`. */ + staleReference?: boolean +} + +const getSimulation = (req: PayloadRequest): Simulation => + (req?.context?.simulate as Simulation) || {} + export const Pages: CollectionConfig = { slug: 'pages', labels: { @@ -19,6 +29,20 @@ export const Pages: CollectionConfig = { access: { read: () => true, }, + hooks: { + beforeChange: [ + ({ data, req }) => { + // Stands in for a non-validation failure during the re-save — the + // reporter names a database error. Anything of this shape is swallowed + // by the plugin's catch even one level deep. + if (data?.breaksOnResave && getSimulation(req).infraFailure) { + throw new Error('Simulated database failure while re-saving a child.') + } + + return data + }, + ], + }, fields: [ { name: 'title', @@ -32,6 +56,21 @@ export const Pages: CollectionConfig = { type: 'text', required: true, }, + { + // Marks a document that should fail when the plugin re-saves it. Both + // failure modes are gated on request context as well, so they fire only + // for the one cascade a test drives and leave the rest of the suite alone. + name: 'breaksOnResave', + type: 'checkbox', + admin: { + hidden: true, + }, + // Stands in for the reported cause: a document that is valid as stored and + // only fails when something re-validates it later — there, a required + // upload field whose media document had since been deleted. + validate: (value, { req }) => + value && getSimulation(req).staleReference ? 'This field is invalid.' : true, + }, { name: 'fullTitle', type: 'text', diff --git a/test/plugin-nested-docs/int.spec.ts b/test/plugin-nested-docs/int.spec.ts index 3d7661eb224..a54049b510b 100644 --- a/test/plugin-nested-docs/int.spec.ts +++ b/test/plugin-nested-docs/int.spec.ts @@ -497,4 +497,75 @@ describe('@payloadcms/plugin-nested-docs', () => { expect(grandchild.categorization[2].label).toStrictEqual('grandchild') }) }) + + describe('error propagation', () => { + const createPage = async (data: Record) => + payload.create({ + collection: 'pages', + data: { _status: 'published', ...data } as Page, + }) + + it('should surface a validation error raised two levels down the cascade', async () => { + const grandparent = await createPage({ title: 'Cascade A', slug: 'cascade-a' }) + const parent = await createPage({ + title: 'Cascade B', + slug: 'cascade-b', + parent: grandparent.id, + }) + await createPage({ + title: 'Cascade C', + slug: 'cascade-c', + breaksOnResave: true, + parent: parent.id, + }) + + // Updating A re-saves B, and B's own resaveChildren re-saves C. C's + // ValidationError is converted to an APIError one level down, so A's + // catch no longer recognises it and lets it go — while the failed update + // has already killed the transaction the three of them share. + await expect( + payload.update({ + id: grandparent.id, + collection: 'pages', + context: { simulate: { staleReference: true } }, + data: { title: 'Cascade A Updated', _status: 'published' }, + }), + ).rejects.toThrow() + + // What the caller was told and what the database holds now agree. + const stored = await payload.findByID({ + id: grandparent.id, + collection: 'pages', + draft: false, + }) + + expect(stored.title).toBe('Cascade A') + }) + + it('should surface a non-validation error raised during a child re-save', async () => { + const parent = await createPage({ title: 'Infra A', slug: 'infra-a' }) + await createPage({ + title: 'Infra B', + slug: 'infra-b', + breaksOnResave: true, + parent: parent.id, + }) + + // This one needs no cascade depth at all: the catch only ever rethrows + // ValidationErrors, so a database error during the child re-save is + // swallowed on the first level. + await expect( + payload.update({ + id: parent.id, + collection: 'pages', + context: { simulate: { infraFailure: true } }, + data: { title: 'Infra A Updated', _status: 'published' }, + }), + ).rejects.toThrow() + + const stored = await payload.findByID({ id: parent.id, collection: 'pages', draft: false }) + + expect(stored.title).toBe('Infra A') + }) + }) }) diff --git a/test/plugin-nested-docs/payload-types.ts b/test/plugin-nested-docs/payload-types.ts index 7dfbffb38c7..d051d1d6711 100644 --- a/test/plugin-nested-docs/payload-types.ts +++ b/test/plugin-nested-docs/payload-types.ts @@ -127,6 +127,7 @@ export interface Page { id: string; title: string; slug: string; + breaksOnResave?: boolean | null; fullTitle?: string | null; parent?: (string | null) | Page; breadcrumbs?: @@ -277,6 +278,7 @@ export interface PayloadMigration { export interface PagesSelect { title?: T; slug?: T; + breaksOnResave?: T; fullTitle?: T; parent?: T; breadcrumbs?: