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

fix(update): set CastError path to full path if casting update fails #14161

Merged
merged 7 commits into from
Dec 24, 2023

Conversation

vkarpov15
Copy link
Collaborator

Fix #14114

Summary

Currently, CastError only shows the path relative to parent, which is less than ideal in the case where casting update throws an error; in that case, Mongoose just throws the CastError, rather than wrapping the CastError in a ValidationError like how save() does. This PR makes CastError use $fullPath instead when casting updates

Examples

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.

Code looks good to me, though what about other types like uuid or Decimal128 or does this only apply to this.applySetters calls?

@vkarpov15 vkarpov15 added this to the 8.0.4 milestone Dec 7, 2023
@vkarpov15
Copy link
Collaborator Author

@hasezoey this change is just for castForQuery(). Decimal128 uses SchemaType base class's castForQuery, so that's covered. It looks like uuid isn't using applySetters() though, I'll check.

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.

Code looks good to me, though it seems deno does not like the UUID change

@vkarpov15
Copy link
Collaborator Author

@hasezoey the deno issue looks unrelated, it's an unfortunate quirk with how our tests are written: Test.findOneAndUpdate({ _id: doc._id }) is equivalent to Test.findOneAndUpdate({}, { _id: doc._id }) because findOneAndUpdate() with 1 arg means Mongoose treats the 1 arg as an update parameter, not a filter. Which means that, if there's any dangling data in the underlying collection, the test will fail. I'm not sure why there's dangling data in deno tests here, but either way we should improve the tests to make them more durable, which I did in 6f7b659

@vkarpov15 vkarpov15 merged commit 3bd67a8 into master Dec 24, 2023
34 checks passed
@hasezoey hasezoey deleted the vkarpov15/gh-14114 branch December 24, 2023 17:38
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.

CastError doesn't return the entire nested path
2 participants