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

_id field bug introduced with v5.4.11 #7524

Closed
hanMarr opened this issue Feb 15, 2019 · 4 comments
Closed

_id field bug introduced with v5.4.11 #7524

hanMarr opened this issue Feb 15, 2019 · 4 comments
Labels
has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue
Milestone

Comments

@hanMarr
Copy link

hanMarr commented Feb 15, 2019

On Friday, Feb 8th, we started experiencing a failure on our integration tests (that spin up a in memory version of mongodb, inserts docs, and runs tests). There were a series of failures but it started with this one:

The _id field cannot be changed from {_id: ObjectId('5c648eb8ec93cd5888f9c9de')} to {_id: "5c648eb8ec93cd5888f9c9de"}.

This error happened when we were trying to write to mongo with the ".replaceOne()" method.

After testing out many versions of mongodb and mongodb-memory-server, we finally nailed down the issue to the mongoose library. We were using mongoose 5.4.12.

To reproduce:
use mongodb v 3.1.13 and mongodb-memory-server v3.1.1 and mongoose v5.4.11 (this is the version where the problem started but it was carried through to 5.4.12). node v10.6.0

The solution to our issue was to revert back to mongoose 5.4.10 (using the same mongodb libraries as above) still using the same version of the other libraries listed above.

@rvillane
Copy link

we are experiencing a very similar issue, like if a recent change on Mongoose is messing up with the type of "_id" on existing documents. Seems that is trying to cast from ObjectId to String. As you pointed out 5.4.11 is the version where this behavior was introduced.

Any ideas ?

@ahce
Copy link

ahce commented Feb 15, 2019

@vkarpov15 Please check this.

The bug is introduced in this commit

exampleBreak is broken because _id false from otherProps causes the _id deletion of the schema. It only happens when otherProps is used as an object.

const mongoose = require('mongoose');

const otherProps = {
  _id: false,
  prop: { type: String }
};

const exampleBreak = new mongoose.Schema(
  {
    name: {
      type: String
    },
    data: otherProps
  },
  {
    timestamps: true
  }
);

const exampleWorks = new mongoose.Schema(
  {
    name: {
      type: String
    },
    data: [otherProps]
  },
  {
    timestamps: true
  }
);

const ExampleBreak = mongoose.model('exampleBreak', exampleBreak);
const ExampleWorks = mongoose.model('exampleWorks', exampleWorks);

async function run() {
  console.log('run');
  const { connection } = await mongoose.connect('YOUR_URI', {
    dbName: 'YOUR_DB_NAME',
    useNewUrlParser: true
  });
  const objWorks = {
    name: 'test',
    data: [{ prop: 'prop' }]
  };
  const objBreak = {
    name: 'test',
    data: { prop: 'prop' }
  };

  await connection.dropDatabase();

  try {
    const doc = await ExampleWorks.create(objWorks);

    console.log(doc);
  } catch (err) {
    console.log(err);
  }

  try {
    const doc = await ExampleBreak.create(objBreak);

    console.log(doc);
  } catch (err) {
    console.log(err);
  }
}

run();

@cosminn777
Copy link

We also moved back to mongoose 5.4.10 (some find queries were not working after the update).
Mongodb 4.0.6

ahce referenced this issue Feb 19, 2019
@vkarpov15 vkarpov15 added this to the 5.4.16 milestone Feb 22, 2019
@vkarpov15 vkarpov15 added the has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue label Feb 22, 2019
vkarpov15 added a commit that referenced this issue Feb 24, 2019
@vkarpov15
Copy link
Collaborator

@ahce we fixed this in master, but your schema doesn't do what you expect it'll do. Your schema:

const otherProps = {
  _id: false,
  prop: { type: String }
};

const exampleBreak = new mongoose.Schema(
  {
    name: {
      type: String
    },
    data: otherProps
  },
  {
    timestamps: true
  }
);

Does not remove a nested._id path. It instead creates a data._id path with arbitrary type. That data._id path would not be there if you didn't put _id: false. Please just remove that _id: false from your schema.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue
Projects
None yet
Development

No branches or pull requests

5 participants