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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure MongooseError is exported in TypeScript #13387

Closed
2 tasks done
kspshnik opened this issue May 8, 2023 · 6 comments 路 Fixed by #13403
Closed
2 tasks done

Make sure MongooseError is exported in TypeScript #13387

kspshnik opened this issue May 8, 2023 · 6 comments 路 Fixed by #13403
Labels
typescript Types or Types-test related issue / Pull Request
Milestone

Comments

@kspshnik
Copy link

kspshnik commented May 8, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

馃殌 Feature Proposal

A tool to detect if error is MongooseError.
The most simple approach is to check if (error instanceof MongooseError). But unfortunately this class is not exported, so one need to check if error is instance of all the exported descendants of it.
The idea is to export a very simple member:

const ifMongooseError = (error: Error) : boolean => error instanceof MongooseError;

This will allow to find out if it's Mongoose's error and do not lead to leaking of private MongooseError class.

Motivation

Sometimes one need to transform thrown by Mongoose and caught in .catch()/catch or passed to errors handler(s) by .catch(next) (if using Express). It would be very nice to have a tool to detect if it's a Mongoose error, since several sources of error might exist in the same controller, like celebrate does (celebrate.isCelebrateError() method).
At present, I use a helper that check against all 16 types of errors. It would be way more refined and elegant to have a built-in Mongoose method.

Example

// ...somewhere in errors handler

elseif (mongoose.isMongooseError(error)) {
 /* Process, transform and/or send to client error */
}
@kspshnik kspshnik added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature new feature This change adds new functionality, like a new method or class labels May 8, 2023
@vkarpov15 vkarpov15 added this to the 7.3.0 milestone May 8, 2023
@vkarpov15 vkarpov15 changed the title A helper to detect if caught error is a MongooseError Make sure MongooseError is exported in TypeScript May 16, 2023
@vkarpov15 vkarpov15 added typescript Types or Types-test related issue / Pull Request and removed new feature This change adds new functionality, like a new method or class enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature labels May 16, 2023
@vkarpov15 vkarpov15 modified the milestones: 7.3.0, 7.1.3 May 16, 2023
@ramos-ph
Copy link
Contributor

Hey, I was skimming through the code to make sure it was exported, and I found out that it's actually exported as mongoose.Error. Here's a little snippet I made to test things:

// main.ts
import mongoose from 'mongoose';

const UserSchema = new mongoose.Schema({ name: String });
const User = mongoose.model('user', UserSchema);

async function main() {
  try {
    await User.create({ name: 'John Doe' }); // MongooseError: Operation `users.insertOne()` buffering timed out after 10000ms
  } catch (err) {
    console.log(err instanceof mongoose.Error); // true
    console.log((err as Error).name); // MongooseError
  }
}

main();

I ended up finding the definition on those files:

module.exports = exports = MongooseError;

Mongoose.prototype.Error = require('./error/index');

Making the following export makes err instanceof MongooseError work, but it doesn't seem to have any difference rather than the class name.

// lib/index.js#1143
Mongoose.prototype.MongooseError = require('./error/mongooseError');

I'm just wondering if Error and MongooseError are different in any way. If so, pardon me for the lack of knowledge.

@kspshnik
Copy link
Author

I'm just wondering if Error and MongooseError are different in any way. If so, pardon me for the lack of knowledge.

Isn't it actually a Mongo error in your snippet? They're different. I meant Mongoose errors descrbed in docs here: https://mongoosejs.com/docs/api/error.html#Error()

And its not a TS solution anyway %(

@ramos-ph
Copy link
Contributor

I see. I changed my snippet using the example of the docs:

import mongoose from 'mongoose';

const Model = mongoose.model('model', new mongoose.Schema({ answer: Number }));

async function main() {
  const doc = new Model({ answer: 'not a number' });
  const err = doc.validateSync();

  console.log(err instanceof mongoose.Error); // true
  console.log(err instanceof mongoose.MongooseError); // true
  console.log(err instanceof mongoose.Error.ValidationError); // true
}

main();

I'm exporting MongooseError the same way, but I didn't noticed any TypeScript changes when I changed errors.d.ts. I'll update my PR.

@kspshnik
Copy link
Author

I'm exporting MongooseError the same way, but I didn't noticed any TypeScript changes when I changed errors.d.ts. I'll update my PR.

It seems to me to be more useful to have an exported helper function in the same way they did it in Celebrate. I mean something like isMoongoseError( error: Error ) : boolean that performs all the required underneath.

@ramos-ph
Copy link
Contributor

It seems to me to be more useful to have an exported helper function in the same way they did it in Celebrate. I mean something like isMoongoseError( error: Error ) : boolean that performs all the required underneath.

It's something that @vkarpov15 suggested on my PR. I originally had implemented a isMongooseError util.
Here's the comment for reference: #13403 (review)

@kspshnik
Copy link
Author

kspshnik commented May 16, 2023

It's something that @vkarpov15 suggested on my PR. I originally had implemented a isMongooseError util.

OK, I see.

Thank you!

@vkarpov15 vkarpov15 modified the milestones: 7.1.3, 7.2.0 May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Types or Types-test related issue / Pull Request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants