Skip to content

Commit

Permalink
fix: correct order for declaration
Browse files Browse the repository at this point in the history
  • Loading branch information
vkarpov15 committed Oct 14, 2019
1 parent cec9dda commit d9163f5
Showing 1 changed file with 2 additions and 3 deletions.
5 changes: 2 additions & 3 deletions lib/error/validation.js
Expand Up @@ -17,16 +17,15 @@ const util = require('util');

function ValidationError(instance) {
this.errors = {};
this.name = 'ValidationError';
this._message = '';

MongooseError.call(this, this._message);
if (instance && instance.constructor.name === 'model') {
this._message = instance.constructor.modelName + ' validation failed';
} else {
this._message = 'Validation failed';
}

MongooseError.call(this, this._message);
this.name = 'ValidationError';

if (Error.captureStackTrace) {
Error.captureStackTrace(this);
Expand Down

4 comments on commit d9163f5

@dskrvk
Copy link
Contributor

@dskrvk dskrvk commented on d9163f5 Feb 25, 2020

Choose a reason for hiding this comment

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

Due to this change we no longer set the super-class's message property (it just remains an empty string as set on line 20.

@vkarpov15
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dskrvk can you please elaborate on your comment? I don’t understand

@dskrvk
Copy link
Contributor

@dskrvk dskrvk commented on d9163f5 Feb 28, 2020

Choose a reason for hiding this comment

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

Sorry, I meant that because the "base constructor" is now called on line 22 before we set this._message, which feeds into the publicly-accessible message property in the base class, message is now always an empty string. It would be nice for it to contain something useful like before. I'm just trying to understand if this was intentional.

@vkarpov15
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dskrvk nope that sounds like an issue. .Can you please open up a new issue with code samples that demonstrate the behavior you're seeing?

Please sign in to comment.