Skip to content

fix(query): replace {MODEL} in cast error messages from update and bulkWrite() casting - #16529

Open
SulimanAbdulrazzaq wants to merge 1 commit into
Automattic:masterfrom
SulimanAbdulrazzaq:fix/update-cast-error-model
Open

SulimanAbdulrazzaq wants to merge 1 commit into
Automattic:masterfrom
SulimanAbdulrazzaq:fix/update-cast-error-model

Conversation

@SulimanAbdulrazzaq

Copy link
Copy Markdown

Summary

{MODEL} in a custom cast error message is replaced when casting query filters, on document validation (#16480) and in castObject() (#16502), but not when casting updates. updateOne(), updateMany() and findOneAndUpdate() report the literal {MODEL}, and the default cast error message for updates leaves out the for model "..." suffix that filter cast errors have. bulkWrite() has the same gap for both its update and filter casting, since it calls castUpdate() and cast() directly instead of going through Query.

This PR:

  • sets the model on cast errors in castUpdate()'s _appendError(), which every update cast error goes through. It's done before the error is thrown or added to the multipleCastError ValidationError, so the aggregated error message is also correct. The context there is the Query, or the model itself for bulkWrite().
  • casts bulkWrite() filters through a small helper that sets the model on cast errors, like Query.prototype._castConditions() does.

Examples

const schema = new Schema({
  age: { type: Number, cast: '{VALUE} is not a valid number for model {MODEL}' }
});
const Test = mongoose.model('Test', schema);

await Test.updateOne({}, { age: 'twenty' });
// Before: CastError: "twenty" is not a valid number for model {MODEL}
// After:  CastError: "twenty" is not a valid number for model Test

Added a test in test/schema.test.js next to the other {MODEL} tests. It covers updateOne(), updateMany(), findOneAndUpdate(), multipleCastError, and bulkWrite() updates and filters.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Some update paths bypass the new handling, and discriminator updates can report the wrong model.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Adds model-aware formatting to update and bulkWrite() cast errors.

Changes:

  • Assigns models to update cast errors.
  • Adds model-aware bulk-write filter casting.
  • Tests query and bulk-write error messages.
File Description
lib/​helpers/​query/​castUpdate.js Sets models on collected update cast errors.
lib/​helpers/​model/​castBulkWrite.js Wraps bulk-write filter casting.
test/​schema.test.js Tests {MODEL} substitution.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 501 to +505
function _appendError(error, query, key, aggregatedError) {
// Set the model on cast errors so `{MODEL}` gets replaced, like it is for
// query filter casting and document validation (gh-8300). `query` is a
// Query, or the model itself when casting a `bulkWrite()` operation.
const model = query?.modelName != null ? query : query?.model;

This branch has not been deployed

No deployments
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