Why is findOneAndRemove sometimes passing null to doc in the callback?

Question

I am using the following function which is supposed to have a callback input: https://mongoosejs.com/docs/api.html#model_Model.findOneAndRemove

I have the following Mutation object that is supposed to delete a particular medicine from the database, and then remove all of the instances of its medicineId from the arrays of all of the Day entries that contain the id.

 deleteMedicine: {
            type: MedicineType,
            args: {
                id: { type: new GraphQLNonNull(GraphQLID) }
            },

            async resolve (parent, args) {

                let res = Medicine.findOneAndRemove({ _id: args.id }, async (err, doc) => {

                    if (err === null && doc === null || doc === null) {
                        return;
                    } else if (err) {
                        console.error(err);
                        return;
                    } else {
                        return await Promise.all(doc.dayNames.map(dayName => {
                            return DayOfWeek.findByIdAndUpdate(dayName, { $pull: { medicineIds: doc._id }})
                            .catch((error1) => console.error(error1));
                        })).catch((error2) => console.error(error2)); 
                    }
                });

                await res; 
                return res; 
            }
        }

findOneAndRemove successfully deletes the document that has args.id in the Medicine collection, but when It calls the callback function, sometimes it passes null to doc and so the callback function doesn't consistently execute properly.

Also I am getting an unhandled error warning even though I am catching all errors. \

I added logic to handle when err and doc are both null according to this post: https://github.com/Automattic/mongoose/issues/5022

Have you found any solution ? I have also same problem with `findByIdAndDelete()`. Document is removed from the database but callback returning null in the doc. — Kishan Bharda, Jan 18 '21 at 06:22

score 0 · Answer 1 · answered Mar 29 '20 at 11:10

You should only uses Promises, not callbacks, with GraphQL.js. If the library you're using doesn't support Promises, then you'll need to wrap each callback in a Promise. However, mongoose has great Promise support -- you just omit the callback from your method call and the call will return a Promise.

To avoid "callback hell" when using Promises and issues with properly chaining your Promises, it's generally preferable to utilize async/await syntax. Your cleaned up resolver would look something like:

async resolve (parent, args) {
  // Omit the callback to get a Promise and then await the result
  const medicine = await Medicine.findOneAndRemove({ _id: args.id })

  // If a medicine with that ID couldn't be found, we'll get a null
  if (medicine) {
    // Make sure we await the Promise returned by Promise.all 
    await Promise.all(doc.dayNames.map(dayName => DayOfWeek.findByIdAndUpdate(
      dayName,
      { $pull: { medicineIds: doc._id } }
    ))
  }

  // Make sure we return the appropriate result depending on the field's type
  return medicine
}

Typically when dealing with Promises, we use the catch method (or try/catch with async/await) in order to handle errors. However, this is only necessary inside a resolver if you want to mask or otherwise format your error -- otherwise, GraphQL will happily catch any errors for you and return them in the response. However, this only works if your Promises are correctly chained and the resolver returns a Promise. If you end up with an "orphan" Promise that's not awaited, you'll see unhandled error warnings.

Why is findOneAndRemove sometimes passing null to doc in the callback?

1 Answers1