Consider making the parallel validate() error a warning or make that an option

[taxilian]

Do you want to request a feature or report a bug?

I'm not sure where this lands, call it a suggested change in behavior =]

What is the current behavior?

As of mongoose 5.8.5 (I believe) there is a check in mongoose to protect against validate() being called multiple times in parallel; I completely agree that the check is needed as having spent a lot of time troubleshooting a recent bug I see all the ways the state could be in a weird condition in that case, however we have code which occasionally produces this error now that we've updated and we aren't sure why.

What is the expected behavior?

Clearly our code has an issue, but where before it was silently appearing to work now it has a fatal error which we can't get around. I want to know that the issue is happening, but it would be nice to be able to tell it to just warn me (since it could be a problem) rather than introducing a fatal error.

At this point I'm not 100% confident that the issue I'm seeing is a real issue and in our code rather than another bug similar to #8531. It's been very difficult to reproduce.

What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.

Not really applicable, but 5.8.9 + my recent PR #8532.

[vkarpov15]

Thanks for the suggestion, and thanks for your PR. I really appreciate the fact that you dug out why this error was happening, it revealed that we were double validating deeply nested subdocs.

But you're right that this issue is causing more headache than it should. Here's an idea: why don't we make it so that we avoid the parallel validate check for subdocuments, but keep it for top-level documents? The reason why we introduced this error is that if you run validate() followed by save() in the same tick, that causes a race condition that may cause invalid data to get saved to the database, and we don't have a good way to fix that issue while retaining support for Document#invalidate(). If this error only happens for top-level documents, that should be fine.

[taxilian]

That would probably be even better. I am still seeing this issue somewhere in production even with my fix on the other PR; it was relatively easy to reproduce (but only on our production db) with a patched 5.8.9 but I can't repro it and only see it in logs with 5.8.10, so I think it's probably a timing issue and the performance improvements in 5.8.10 made it less likely to happen.

I have a branch that I have added code to track the async stack trace (when a schema option is set) through the validation tree to make it possible to see where the issue came from so that if you actually were calling it in parallel it's easier to track down; it shouldn't add any overhead when not enabled and is pretty inexpensive when enabled. I'll submit it as a PR as soon as I can get back to it (another pressing issue to fix first on something else) and you can decide if it's worth pulling in. I don't think it'll be needed often, but it was a pain in the neck tracing through all those nextTick callbacks.

[vkarpov15]

@taxilian thanks, I'm looking forward to that PR. I'll wait for that before I make changes to not throw this error on subdocuments, but I'm thinking I'll make that change in v5.8.11.

[taxilian]

I've been able to reproduce my issue and discovered that there are still child documents with validators being called more than once even with my earlier PR; is there a faster way to communicate with you than submitting issues for when I'm working on something like this? Often I think a 5 minute question to you might save me hours of trial and error =] There is just too much going on in here for me to change things without a lot of research or inside knowledge.

[taxilian]

you can reach me at rbateman at gradecam dot com

[taxilian]

#8548 -- I think this should be the complete fix this time

[vkarpov15]

Thanks @taxilian , I really appreciate your help 👍

[vkarpov15]

Also @taxilian , we're putting together a "Built with Mongoose" page (#8540). I'd love to put Gradecam up there 👍