Skip to content

fix(plugin-nested-docs): rethrow errors raised while re-saving children - #17701

Open
luantaraschi wants to merge 1 commit into
payloadcms:mainfrom
luantaraschi:fix/nested-docs-rethrow-child-error
Open

fix(plugin-nested-docs): rethrow errors raised while re-saving children#17701
luantaraschi wants to merge 1 commit into
payloadcms:mainfrom
luantaraschi:fix/nested-docs-rethrow-child-error

Conversation

@luantaraschi

Copy link
Copy Markdown

Fixes #17457.

What

resaveChildren swallows errors raised while re-saving a child. The parent update then resolves with a fresh updatedAt on a transaction the failed child update has already rolled back — the caller is told the write landed, and it did not.

@michael-spanier reported it and @sam9191 corroborated it at 3.80.0. Between them they identify three ways to reach the swallow; this closes all three with one line.

The catch

} catch (err) {
  req.payload.logger.error(`Nested Docs plugin encountered an error while re-saving a child document.`)
  req.payload.logger.error(err)

  if (err instanceof ValidationError && err.data?.errors?.length) {
    throw new APIError('Could not publish or save changes: One or more children are invalid.', 400)
  }
}

Two of the three paths need no bundling and no dual-module context:

  1. Anything that is not a ValidationError — a database error during the re-save — falls straight off the end of the catch. One level deep is enough.
  2. A cascade deeper than one level. The rethrow above does not rethrow the child's error, it converts it. So A → B → C, with C invalid: resaveChildren(B) turns C's ValidationError into an APIError, that propagates out of update(B) into resaveChildren(A), whose instanceof ValidationError check is now false by construction, and it is swallowed. Error information survives exactly one cascade level.

The third — instanceof failing across duplicate module copies in a bundled runtime — is the one the reporter observes in production and is the hardest to test; it lands in the same branch and is closed by the same line.

The fix

   if (err instanceof ValidationError && err.data?.errors?.length) {
     throw new APIError('Could not publish or save changes: One or more children are invalid.', 400)
   }
+
+  throw err
 }

The friendly APIError for the common one-level validation case is untouched, so nothing that works today changes shape. The issue also floats making the check structural (err?.name === 'ValidationError'); I left it alone deliberately, because @sam9191 is right that it does not help — the first level still converts to APIError, so the second level swallows regardless of how the check is written. Rethrowing is the part that matters.

I did not take the issue's other suggested direction — running the re-save with overrideValidation. That would hide the invalid child rather than report it, and it is a policy call about what a nested-docs re-save is allowed to write. This change stays inside a decision the plugin has already made: it has decided that an invalid child should fail the parent. It just fails to honour that one level down.

Verified by running it

Added error propagation to test/plugin-nested-docs/int.spec.ts — one test per non-bundled path. The fixture is a hidden breaksOnResave checkbox on Pages plus a validate and a beforeChange hook, both gated on request context so they fire only for the cascade a test drives and leave the other twelve tests alone.

MongoDB PostgreSQL
With the fix Tests 14 passed (14) Tests 14 passed (14)
Reverting only resaveChildren.ts Tests 2 failed | 12 passed Tests 2 failed | 12 passed

Both controls fail the same way, and the message is the bug itself rather than a proxy for it:

AssertionError: promise resolved "{ title: 'Cascade A Updated', …(9) }" instead of rejecting
AssertionError: promise resolved "{ title: 'Infra A Updated', …(9) }" instead of rejecting

I also confirmed the data-loss half separately, in a throwaway spec on the unfixed code, since the committed tests stop at the first assertion once the update resolves:

RESOLVED  title= Loss A Updated  updatedAt= 2026-08-06T23:45:53.107Z
STORED    title= Loss A          updatedAt= 2026-08-06T23:45:52.979Z

The resolved document and the database disagree — which is what the editor experiences as the documentModified modal on every subsequent save.

What this changes for users

Saves that report success today will start failing loudly. That is the point of the change, but it is a behavioural one: an installation whose child documents have gone stale has been silently discarding parent edits, and after this it gets an error instead. The parent write was never persisted either way.

What I did not verify

I ran plugin-nested-docs on MongoDB and PostgreSQL, not SQLite or Firestore. I did not build a bundled Next.js app to exercise the dual-module instanceof path the reporter sees in production — I am relying on their empirical evidence for that one, and the line closes it by construction rather than by type check. I did not run the e2e spec for this suite; the change is confined to an error path that the fixture opts into explicitly.

Checklist

  • Tests added
  • Documentation — no API change

resaveChildren only rethrows a child ValidationError, converting it to an
APIError. Everything else is logged and swallowed, so the parent update
resolves with a fresh updatedAt on a transaction the failed child update
has already rolled back — the caller is told the write landed and it did
not.

Two paths reach the swallow without any bundling or dual-module context:
a non-validation failure at any depth, and a cascade deeper than one
level, where the APIError produced above is no longer a ValidationError
by the time the next ancestor's catch inspects it.

Fixes payloadcms#17457

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant