1

I can't seem to find similar codes handling errors properly in bulkwrite so I just want to ask if I am doing it right.

What I am trying to do is just have one single try/catch and when there are other promises, I would just check for instanceof Error and throw that in the try/catch. Is this right or there is a better/cleaner way of doing this? Thanks a lot. 

try {
  // other logic here

  const bulkWriteResult = await new Promise((resolve, reject) => {
    try {
      Student.collection.bulkWrite(
        bulkUpdateOps,
        { ordered: true, w: 1 },
        (err, result) => {
          if (err) reject(err);
          resolve(result);
        },
      );
    } catch (err) {
      reject(err);
    }
  });

  if (bulkWriteResult instanceof Error) {
    const error = new Error('Unable to batch update students');
    error.code = 500;
    throw error;
  }
} catch (err) {
  // handle all thrown error
}
Woppi
  • 5,303
  • 11
  • 57
  • 81
  • Meaning what exactly? An exception gets thrown and you catch it. There isn't much more to it unless you want to do different actions for "specific types of errors". Or otherwise you have a 'specific problem' which you have omitted from the question? I don't see a question here right now other than "is my code okay?", and you're on the wrong site for that question. Please go to [codereview.stackexchange.com](https://codereview.stackexchange.com) or decide what question you are actually asking. – Neil Lunn Jun 02 '18 at 11:40
  • If you mean `if (bulkWriteResult instanceof Error) {` does this catch the error? Then no it does not. That is what the `catch (err)` block is for and where the exception goes. – Neil Lunn Jun 02 '18 at 11:41
  • @NeilLunn do you mean even if bulkWriteResult is a value of promise it will instantly go to catch and I don't even have to do `if (bulkWriteResult instanceof Error)`? I'm confused. – Woppi Jun 02 '18 at 12:18
  • 1
    If you're confused, read the referenced answer as the explanations are quite good. The inner `try.catch` you have is not doing anything because it's a "callback" and not an `await` resolved promise. Also the core driver and mongoose support promises natively, so there is no need wrap a callback with a Promise. And `Student.bulkWrite()` has been a mongoose model method for some time. – Neil Lunn Jun 02 '18 at 12:21
  • @NeilLunn thanks for your patience and tips. You were right... I could simply just do `const bulkWriteResult = await Student.bulkWrite( bulkUpdateOps, { ordered: true, w: 1 }, (err) => { if (err) throw (err); }, );` because Mongoose returns promises. What I did before was straight out of Mongo db.collection.bulkWrite, upon reading mongo documentation there is no mention of returning a promise. Upon checking, it is working :) – Woppi Jun 04 '18 at 08:49

0 Answers0