fix(plugin-nested-docs): rethrow errors raised while re-saving children - #17701
Open
luantaraschi wants to merge 1 commit into
Open
fix(plugin-nested-docs): rethrow errors raised while re-saving children#17701luantaraschi wants to merge 1 commit into
luantaraschi wants to merge 1 commit into
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17457.
What
resaveChildrenswallows errors raised while re-saving a child. The parent update then resolves with a freshupdatedAton 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
Two of the three paths need no bundling and no dual-module context:
ValidationError— a database error during the re-save — falls straight off the end of the catch. One level deep is enough.resaveChildren(B)turns C'sValidationErrorinto anAPIError, that propagates out ofupdate(B)intoresaveChildren(A), whoseinstanceof ValidationErrorcheck is now false by construction, and it is swallowed. Error information survives exactly one cascade level.The third —
instanceoffailing 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
APIErrorfor 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 toAPIError, 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 propagationtotest/plugin-nested-docs/int.spec.ts— one test per non-bundled path. The fixture is a hiddenbreaksOnResavecheckbox onPagesplus avalidateand abeforeChangehook, both gated on request context so they fire only for the cascade a test drives and leave the other twelve tests alone.Tests 14 passed (14)Tests 14 passed (14)resaveChildren.tsTests 2 failed | 12 passedTests 2 failed | 12 passedBoth controls fail the same way, and the message is the bug itself rather than a proxy for it:
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:
The resolved document and the database disagree — which is what the editor experiences as the
documentModifiedmodal 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-docson MongoDB and PostgreSQL, not SQLite or Firestore. I did not build a bundled Next.js app to exercise the dual-moduleinstanceofpath 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