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

Improve error handling #7215

Closed
pronebird opened this issue Nov 3, 2018 · 7 comments
Closed

Improve error handling #7215

pronebird opened this issue Nov 3, 2018 · 7 comments

Comments

@pronebird
Copy link

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

Feature

What is the current behavior?

Currently Mongoose does not provide any way to error handling. Consider the following example from documentation:

// Handler **must** take 3 parameters: the error that occurred, the document
// in question, and the `next()` function
schema.post('save', function(error, doc, next) {
  if (error.name === 'MongoError' && error.code === 11000) {
    next(new Error('There was a duplicate key error'));
  } else {
    next();
  }
});

The use of so many magic values is unacceptable 🤮. Perhaps some of them come from the underlying driver. Would be awesome to either re-export those values or define constants to improve the situation.

If the current behavior is a bug, please provide the steps to reproduce.

What is the expected behavior?

Please mention your node.js, mongoose and MongoDB version.

@arm22
Copy link

arm22 commented Nov 4, 2018

This is a contrived example, and one that appears in a handful closed issues. #3059, #2284

Normally Mongoose will throw one or an array of predefined Mongoose.Error objects.

In the case where a document passes the validator middleware, and attempts to be saved, MongoDB throws the above MongoError with code === 11000 because:

the unique Option Is Not a Validator
// Will error, but will not be a mongoose validation error, it will be
// a duplicate key error.

New users of Mongoose (myself included) expect that setting a field as unique on the schema will validate that any documents created will have been validated for their uniqueness before saving, which isn't the case.
https://mongoosejs.com/docs/api.html#schematype_SchemaType-unique

The example provided in the docs and referenced in this issue is a way to handle errors thrown outside of the scope of Mongoose, in this case by Mongo itself. Theres a nifty plugin to abstract this one specific instance of "magic values" out of your codebase. Under the hood this plugin just does the legwork of checking if any document exists with the given unique field and forgoes calling save() to return a Mongoose.ValidationError.

@vkarpov15
Copy link
Collaborator

Be wary of using mongoose-unique-validator because it comes with caveats. For a more robust solution, check out this plugin: https://www.npmjs.com/package/mongoose-beautiful-unique-validation

const assert = require('assert');
const mongoose = require('mongoose');
mongoose.set('debug', true);

const connectionString = `mongodb://localhost:27017/test`;
const { Schema } = mongoose;

run().then(() => console.log('done')).catch(error => console.error(error));

async function run() {
  await mongoose.connect(connectionString);
  await mongoose.connection.dropDatabase();

  const schema = new Schema({
    name: {
      type: String,
      unique: true
    }
  });

  schema.plugin(require('mongoose-beautiful-unique-validation'));

  const Model = mongoose.model('Test', schema);

  await Model.init();

  // Throws a validation error with 'ValidatorError: Path `name` (test) is not unique'
  await Model.create([{ name: 'test' }, { name: 'test' }]);
}

@bricekams
Copy link

bricekams commented Jul 23, 2023

This is still an issue to me. I think the error should be thrown as custom dedicated classes so that we can handle exceptions in a more concise way using instance of or @Catch<T> in NestJS. Instead of that, Mongoose throws errors as Object. It's still very difficult to catch errors globally in that way.

@Uzlopak
Copy link
Collaborator

Uzlopak commented Jul 24, 2023

@bricekams

MongooseErrors are inheriting from Error, so instanceof will work. Nestjs uses jest as testrunner, which is known to have issues with instanceof Error.

jestjs/jest#2549

@bricekams
Copy link

bricekams commented Jul 24, 2023

But i still think instanceof MongooseError should work to catch every error thrown by query at runtime.
For example:

schema.post('save', function(error, doc, next) {
  if (error.name === 'MongoError' && error.code === 11000) {
    next(new Error('There was a duplicate key error'));
  } else {
    next();
  }
});

This is still what I use to catch this type of erors. Even when I pass an invalid mongoId it just throw an object

@Uzlopak
Copy link
Collaborator

Uzlopak commented Jul 24, 2023

MongooseError and MongoError are extending from Error.

https://github.com/mongodb/node-mongodb-native/blob/52cd649caf2e64aef6d3984c5f2d24af03db4c51/src/error.ts#L122

So you claim that our thrown errors are just Objects is false.

Your code snippet seems wrong as you always expect error to be always not being undefined or null.

@bricekams
Copy link

Well I apologize, I'll just take a deep look into it

@Automattic Automattic locked and limited conversation to collaborators Jul 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants