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

Post validate error handler does not fire #4885

Closed
sshymko opened this issue Jan 10, 2017 · 13 comments
Closed

Post validate error handler does not fire #4885

sshymko opened this issue Jan 10, 2017 · 13 comments
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@sshymko
Copy link

sshymko commented Jan 10, 2017

Environment:

  • Node.js 6.7.0
  • MongoDB 3.2.10
  • Mongoose 4.7.2

Steps to reproduce:

  1. Execute the following test script:
let PaymentSchema = new db.Schema({
  currency: {type: String, required: true, enum: ['USD'], default: 'USD'},
  amount: {type: Number, required: true},
});

PaymentSchema.post('validate', function(error, doc, next) {
  next(new Error('Invalid payment information'));
});

let PaymentModel = db.model('Payment', PaymentSchema);

let payment = new PaymentModel({
  currency: 'EUR',
  amount: 10,
});

payment.validate()
  .catch((error) => {
    console.log(error.message);
  });

let error = payment.validateSync();
if (error) {
  console.log(error.message);
}

Expected result:

  • Post-validate error handler catches & converts all validation errors
  • Model validation methods validate and validateSync return converted error

Actual result:

  • Post-validate error handler does NOT get called upon validation failure
  • Model validation methods return original error
@sobafuchs
Copy link
Contributor

Thanks for the repro script.

Looks like it also doesn't run when you actually make a db operation:

const mongoose = require('mongoose');
mongoose.Promise = global.Promise;

const GITHUB_ISSUE = `gh-4885`;

mongoose.connect(`mongodb://localhost:27017/${ GITHUB_ISSUE }`);
let PaymentSchema = new mongoose.Schema({
  currency: { type: String, required: true, enum: ['USD'], default: 'USD' },
  amount: { type: Number, required: true },
});

PaymentSchema.post('validate', function(error, doc, next) {
  console.log(`Error in post validate hook ${error}`)
  next(new Error('Invalid payment information'));
});

let PaymentModel = mongoose.model('Payment', PaymentSchema);

try {
  let payment = new PaymentModel({
    currency: 'EUR',
    amount: 10,
  });

  console.log('saving...');
  payment.save()
    .then(doc => {
      console.log('saved', doc);
      process.exit(0);
    })
    .catch(error => {
      console.error(`Error in catch handler ${error}\n${error.stack}`);
      process.exit(2);
    })

} catch (error) {
  console.error(error);
  process.exit(2);
}

@sobafuchs sobafuchs added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. bug? and removed confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. labels Jan 12, 2017
@sobafuchs sobafuchs added this to the 4.7.8 milestone Jan 12, 2017
@sshymko
Copy link
Author

sshymko commented Jan 17, 2017

Looks like post save error handler does not fire either. Do error handlers ever fire at all?

@sshymko
Copy link
Author

sshymko commented Jan 17, 2017

@varunjayaraman
You confirmed the issue, didn't you? Why has the label changed "confirmed-bug" -> "bug?" than?

@sobafuchs
Copy link
Contributor

yep, sorry about that, not sure what happened

@sobafuchs sobafuchs added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed bug? labels Jan 17, 2017
vkarpov15 added a commit that referenced this issue Jan 22, 2017
@sshymko
Copy link
Author

sshymko commented Jan 22, 2017

@vkarpov15
Thanks for the fix, I'm yet to verify it.
Does it also resolve the post save error hander not intercepting errors?
The issue does seem to extend beyond the post validate to error handlers in general.

@vkarpov15
Copy link
Collaborator

What do you mean by "post save error handler not intercepting errors"? I'm not aware of such an issue.

@sshymko
Copy link
Author

sshymko commented Jan 24, 2017

@vkarpov15
Thanks for getting back to me.

The test case is the same, but schema.post('save', errorHandler) instead of schema.post('validate', errorHandler). Error handler won't fire upon save failure. In my case save failure was due to the VersionError, but perhaps DB error of any nature will reproduce too.

@vkarpov15
Copy link
Collaborator

@sshymko works just fine:

const mongoose = require('mongoose');
mongoose.Promise = global.Promise;

const GITHUB_ISSUE = `gh-4885`;

mongoose.connect(`mongodb://localhost:27017/${ GITHUB_ISSUE }`);
let PaymentSchema = new mongoose.Schema({
  currency: { type: String, required: true, enum: ['USD'], default: 'USD' },
  amount: { type: Number, required: true },
});

PaymentSchema.post('save', function(error, doc, next) {
  console.log(`Error in post validate hook ${error}`)
  next(new Error('Invalid payment information'));
});

let PaymentModel = mongoose.model('Payment', PaymentSchema);

try {
  let payment = new PaymentModel({
    currency: 'EUR',
    amount: 10,
  });

  console.log('saving...');
  payment.save()
    .then(doc => {
      console.log('saved', doc);
      process.exit(0);
    })
    .catch(error => {
      console.error(`Error in catch handler ${error}\n${error.stack}`);
      process.exit(2);
    })

} catch (error) {
  console.error(error);
  process.exit(2);
}

Output

$ node gh-4885.js 
saving...
Error in post validate hook ValidationError: `EUR` is not a valid enum value for path `currency`.
Error in catch handler Error: Invalid payment information
Error: Invalid payment information
    at model.<anonymous> (/home/val/Workspace/10gen/troubleshoot-mongoose/gh-4885.js:14:8)
    at next (/home/val/Workspace/10gen/mongoose/node_modules/kareem/index.js:141:14)
    at Kareem.execPost (/home/val/Workspace/10gen/mongoose/node_modules/kareem/index.js:189:3)
    at /home/val/Workspace/10gen/mongoose/lib/schema.js:199:43
    at handleError (/home/val/Workspace/10gen/mongoose/node_modules/hooks-fixed/hooks.js:40:22)
    at next_ (/home/val/Workspace/10gen/mongoose/node_modules/hooks-fixed/hooks.js:75:26)
    at fnWrapper (/home/val/Workspace/10gen/mongoose/node_modules/hooks-fixed/hooks.js:186:8)
    at /home/val/Workspace/10gen/mongoose/lib/document.js:1370:9
    at /home/val/Workspace/10gen/mongoose/node_modules/kareem/index.js:127:16
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)
    at Module.runMain (module.js:606:11)
    at run (bootstrap_node.js:394:7)
    at startup (bootstrap_node.js:149:9)
    at bootstrap_node.js:509:3
$ 

Which version of mongoose are you using?

@sshymko
Copy link
Author

sshymko commented Feb 20, 2017

@vkarpov15
I'll test schema.post('save', errorHandler) once again and file a separate issue, if it reproduces.

@sshymko
Copy link
Author

sshymko commented Feb 20, 2017

@vkarpov15
Confirmed the error transformation by the handler schema.post('validate', handler) does affect doc.validate() as of Mongoose 4.7.8.
However, doc.validateSync() still returns the original error object unaffected by the handler.
Cannot tell whether it's because of not invoking the handler or ignoring its result.
Tested on the latest Mongoose 4.8.3.

@vkarpov15
Copy link
Collaborator

@sshymko can you please clarify with a code sample?

@jayadrathamondal
Copy link

This error is still there in mongoose v5.9.5.
Post validate hook doesn't get executed if I call doc.validateSync function but it get executed if I call doc.validate().

const mongoose = require('mongoose');

const PaymentSchema = new mongoose.Schema({
  currency: { type: String, required: true, enum: ['USD'], default: 'USD' },
  amount: { type: Number, required: true },
});

PaymentSchema.post('validate', (error, doc, next) => {
  next(new Error('POST VALIDATE FUNCTION EXECUTED'));
});

const PaymentModel = mongoose.model('Payment', PaymentSchema);

const payment = new PaymentModel({
  currency: 'EUR',
  amount: 10,
});

payment.validate((error) => {
  console.log(`validate: ${error.message}`);
});

const error = payment.validateSync();
if (error) {
  console.log(`validateSync: ${error.message}`);
}

Check the outputs of both console.log. They are different.
In post validate hook I'm changing the error message but its only reflecting in payment.validate() function not in payment.validateSync().

@vkarpov15
Copy link
Collaborator

@jayadrathamondal we unfortunately don't have a way to run error handling middleware on validateSync() because error handling middleware always takes 3 arguments. Please follow #8703 for updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
Development

No branches or pull requests

4 participants