Mongoose $push keeps adding two entries

Question

Here are my user and product schemas:

const productSchema = new Schema({
  //... 
  addedBy: {
    type: mongoose.Schema.Types.ObjectId,
    ref: "users"
  }
});

const userSchema = new Schema({   
  //...
  addedItems: [{
    type: mongoose.Schema.ObjectId,
    ref: "products"
  }]
});

mongoose.model("products", productSchema);
mongoose.model("users", userSchema);

In my Node back end route I do this query:

User.findOneAndUpdate(
  { _id: req.body.id },
  { $push: { addedItems: newProduct._id } },
  { upsert: true, new: true },
  function(err, doc) {
    console.log(err, doc);
  }
);

The console.log prints out this:

{
    //...
    addedItems: [ 5ab0223118599214f4dd7803 ]
}

Everything looks good. I go to actually look at the data using the front-end website for my mongo db; I'm using mlab.com, and this is what shows:

{
//...
"addedItems": [
        {
            "$oid": "5ab0223118599214f4dd7803"
        },
        {
            "$oid": "5ab0223118599214f4dd7803"
        }
    ]
}

Question: What the heck happened? Why does it add an additional entry into addedItems ?! Even though my console.log only showed one.

Note:

I tested to see if the backend route was being called more than once. It is not.

It seems to be a problem with $push because if I just have { addedItems: newProduct._id } then only one entry goes in, but it overwrites the entire array.

Edit:

Made a test project to produce the same results: https://github.com/philliprognerud/test-mcve-stackoverflow

Can anyone figure out what's going on?

@Johnny I'm pretty sure I'm not calling 'findOneAndUpdate' twice. There's not a lot of code I'm dealing with either. So i'm wondering how this could be happening. Also since I'm doing 'new: true' it should be returning the most up to date doc. And it only shows one. — Phil, Mar 19 '18 at 21:59
The only thing I'm doing after that update is calling res.send(...) and it goes back to my React front end. Keep in mind this is only happening with '$push', if I try to just do '{ addedItems: newProduct._id }' then only one entry goes in. But it overwrites everything obviously. — Phil, Mar 19 '18 at 22:02
@JohnnyHK https://github.com/philliprognerud/test-mcve-stackoverflow — Phil, Mar 19 '18 at 23:14

score 23 · Accepted Answer · answered Mar 20 '18 at 01:07

23

The problem is caused by your mixed used of promises (via async/await) and callbacks with the findOneAndUpdate call which ends up executing the command twice.

To fix the problem:

const updatedUser = await User.findOneAndUpdate(
  { id: userID },
  { $push: { addedItems: newProduct.id } },
  { upsert: true, new: true }
);

console.log(updatedUser);

Future readers note that the use of await isn't shown here in the question, but is in the MCVE.

answered Mar 20 '18 at 01:07

JohnnyHK

305,182
66
621
471

Thank you! Solution works, and reasoning makes sense. – Phil Mar 20 '18 at 01:16
Can you please explain why it happens? Why does the operation get executed twice if you pass a callback function? – lucifer63 Dec 02 '18 at 20:21
@lucifer63 I assume it's because of [this](https://stackoverflow.com/a/53970919/1259510). – JohnnyHK Nov 23 '19 at 14:06

score 2 · Answer 2 · answered Oct 21 '19 at 15:51

I am facing similar issue. Just landed to this page. I find that previous answer is not very descriptive. So posting this:

export const updateUserHandler = async (req, res) => {
    const request = req.body;    
 await  User.findOneAndUpdate(                  //<== remove await 
  { _id: request.id },
  { $push: { addedItems: newProduct._id } },
  { upsert: true, new: true },
  (findErr, findRes) => {
        if (findErr) {
          res.status(500).send({
            message: 'Failed: to update user',
            IsSuccess: false,
            result: findErr
          });
        } else {
          res.status(200).send({
            message: 'Success:  to update user',
            IsSuccess: true,
            result: findRes
          });

        }
      }
);
  }

Here there are two async calls one is the async and other is await. Because of this there are two entries in the document. Just remove await from await User.findOneAndUpdate. It will work perfectly. Thanks!!

score 2 · Answer 3 · edited Jun 14 '20 at 02:01

2

When you await Query you are using the promise-like, specifically, .then() and .catch(() of Query. Passing a callback as well will result in the behavior you're describing.

If you await Query and .then() of Query simultaneously, would make the query execute twice

use:

await Model.findOneAndUpdate(query, doc, options)

OR

Model.findOneAndUpdate(query, doc, options, callback)

edited Jun 14 '20 at 02:01

Jason Aller

3,541
28
38
38

answered Jun 14 '20 at 01:18

Johnsonsy

71
2

score 1 · Answer 4 · answered May 16 '20 at 18:00

This code $push keeps adding two entries: const ali={ "_id": "5eaa39a18e7719140e3f4430" };

//   return await customerModel.findOneAndUpdate(
//     ali,
//     {
//       "$push": {
//         "address": [objAdr],
//       },
//     },
//     function (error: any, success: any) {
//       if (error) {
//         console.log(error);
//       } else {
//         console.log(success);
//       }
//     }
//   );

My solutions working true:

return await customerModel
.findOneAndUpdate(
  { _id: ids },
  { $push: { "address": objAdr } }
)
.catch((err: string | undefined) => new Error(err));

Mongoose $push keeps adding two entries

4 Answers4