Skip to content

fix: report default paths in VersionError message because they can can cause VersionError #15464

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

Merged
merged 1 commit into from
Jun 5, 2025

Conversation

vkarpov15
Copy link
Collaborator

Summary

I ran into a VersionError: No matching document found for id "xyz" version 1 modifiedPaths "lastPaidAt, totalRevenue" error - that shouldn't happen because lastPaidAt and totalRevenue are both primitives, shouldn't trigger versioning.

The root cause was an array default was triggering versioning: an array value wasn't set in the db. Paths that are in default status can trigger versioning, but VersionError doesn't currently list those paths.

Examples

@vkarpov15 vkarpov15 added this to the 8.15.2 milestone Jun 4, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an issue where default paths were not being reported in a VersionError even though they could trigger version conflicts.

  • Updated the error generation logic in lib/model.js to include default state paths from activePaths.
  • Added a test in test/document.test.js to verify that a VersionError correctly reports the default path when using push() on an array.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
test/document.test.js Added a test case that triggers a VersionError by modifying array values to verify the fix.
lib/model.js Updated error generation in save methods to concatenate default paths with modified paths.
Comments suppressed due to low confidence (1)

lib/model.js:464

  • Consider adding a comment to explain why default state paths are concatenated with modifiedPaths, so future maintainers understand this intentional behavior.
this.$__.modifiedPaths = this.modifiedPaths().concat(Object.keys(this.$__.activePaths.getStatePaths('default')));

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vkarpov15 vkarpov15 merged commit 2dae080 into master Jun 5, 2025
71 checks passed
@hasezoey hasezoey deleted the vkarpov15/version-error-with-defaults branch June 5, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants