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

BSONError thrown when calling _id.toString() after using R.clone after mongoose bump #14353

Closed
2 tasks done
Axosoft-Ashley opened this issue Feb 14, 2024 · 6 comments
Closed
2 tasks done
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Milestone

Comments

@Axosoft-Ashley
Copy link

Axosoft-Ashley commented Feb 14, 2024

Prerequisites

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

Mongoose version

8.1.2

Node.js version

18

MongoDB server version

6.0

Typescript version (if applicable)

No response

Description

After bumping from v5.13.20 to 8.1.2, when R.clone() is called on an array of mongoose documents a BSON error is thrown, this did not happen on the older version of mongoose.
throw new BSONError("Cannot create Buffer from ${String(potentialBuffer)}");

Ramda version ^0.29.1 was used for both old and new mongoose test

Steps to Reproduce

schema setup

const userSchema = new mongoose.Schema({
  _id: { type: String, default: () => uuid.v4() },
});
const UserModel = mongoose.model('User', userSchema);

const organizationUser = {
  user: { type: String, ref: 'User' }
};

const organizationSchema = new mongoose.Schema({
  members: [organizationUser],
}, { usePushEach: true });

crash code:

    const R = require('ramda');
    const mongoose = require('mongoose');

    let foundOrgModel = await OrganizationModel.findOne({});
    if (!foundOrgModel) {
        // create user
        const user = await UserModel.create({})
        // create org
       foundOrgModel = await OrganizationModel.create({ members: [{ user: user._id }] })
    }

    foundOrgModel = await OrganizationModel.findOne({})
    const members = R.clone(foundOrgModel.members);
    console.log(members[0]._id.toString()) // crashes

Expected Behavior

it should not throw but instead just log the id as a string like it did in v5.13.20 of mongoose

@gitkrakel
Copy link

I am also able to reproduce with Document.prototype.$clone instead of Ramda's clone by modifying an existing unit-test to use a subdocument array:

  it.only('modification of test "$clone() (gh-11849)"', async function() {
    const schema = new mongoose.Schema({
      subDocArray: [{
        name: {
          type: String,
          validate: {
            validator: (v) => v !== 'Invalid'
          }
        }
      }]
    });
    const Test = db.model('Test', schema);

    const item = await Test.create({ subDocArray: [{ name: 'Test' }] });

    const doc = await Test.findById(item._id);
    const clonedDoc = doc.$clone();

    // This assertion fails
    assert.deepEqual(clonedDoc.subDocArray[0], doc.subDocArray[0]);
  });

@vkarpov15 vkarpov15 added this to the 8.1.4 milestone Feb 19, 2024
@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 19, 2024
@IslandRhythms IslandRhythms added confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. and removed has repro script There is a repro script, the Mongoose devs need to confirm that it reproduces the issue labels Feb 26, 2024
@IslandRhythms
Copy link
Collaborator

const mongoose = require('mongoose');
const assert = require('assert');

const testSchema = new mongoose.Schema({
  subDocArray: [{
    name: {
      type: String,
      validate: {
        validator: (v) => v !== 'Invalid'
      }
    }
  }]
});

const Test = mongoose.model('Test', testSchema);

async function run() {
  await mongoose.connect('mongodb://localhost:27017');
  await mongoose.connection.dropDatabase();

  await Test.create({ subDocArray: [{ name: 'Test' }] });
  const doc = await Test.findOne();
  const cloneTrooper = doc.$clone();

  assert.deepEqual(cloneTrooper.subDocArray[0], doc.subDocArray[0]);
  console.log('done');
}

run();

@vkarpov15
Copy link
Collaborator

I repro-ed OP's post using the following:

    const R = require('ramda');
    const mongoose = require('mongoose');

const OrganizationModel = mongoose.model('Organization', mongoose.Schema({ members: [{ user: 'ObjectId' }] }));
const UserModel = mongoose.model('User', mongoose.Schema({ members: ['ObjectId'] }));

void async function main() {
    await mongoose.connect('mongodb://127.0.0.1:27017/mongoose_test');
    let foundOrgModel = await OrganizationModel.findOne({});
    if (!foundOrgModel) {
        // create user
        const user = await UserModel.create({})
        // create org
       foundOrgModel = await OrganizationModel.create({ members: [{ user: user._id }] })
    }

    foundOrgModel = await OrganizationModel.findOne({})
    const members = R.clone(foundOrgModel.members);
    console.log(members[0]._id.toString()) // crashes
}();

@gitkrakel your issue looks unrelated to OP's, but we'll take a look at that as well

@vkarpov15
Copy link
Collaborator

@Axosoft-Ashley the R.clone() issue comes down to the fact that, in Mongoose 5.x, the ObjectId class from the bson library stored its data in an id property. In Mongoose 8.x, the ObjectId class from the bson library stores its data in a Symbol('kId') property. And, internally, R.clone() uses a for/in loop to iterate the object to clone, which doesn't loop over symbols.

I'll do some more digging to see if there's a way we can reconcile these two. But given that R.clone() isn't extensible, it looks like our only options are to either 1) update the bson library to make the raw data associated with an ObjectId stored in an own non-symbol property on the ObjectId instance, 2) update Ramda to clone symbols. For example, updating Ramda's _clone() function to use the following seems to work:

      for (var key of Object.keys(value).concat(Object.getOwnPropertySymbols(value))) {
        if (Object.prototype.hasOwnProperty.call(value, key)) {
          copiedValue[key] = deep ? _clone(value[key], true, map) : value[key];
        }
      }

@vkarpov15
Copy link
Collaborator

Actually, it looks like bson@6.4 (just released today) fixes this issue, so R.clone() works fine now, following script outputs the ObjectId as a string if Mongoose installs bson@6.4:

    const R = require('ramda');
    const mongoose = require('mongoose');

const OrganizationModel = mongoose.model('Organization', mongoose.Schema({ members: [{ user: 'ObjectId' }] }));
const UserModel = mongoose.model('User', mongoose.Schema({ members: ['ObjectId'] }));

void async function main() {
    await mongoose.connect('mongodb://127.0.0.1:27017/mongoose_test');
    let foundOrgModel = await OrganizationModel.findOne({});
    if (!foundOrgModel) {
        // create user
        const user = await UserModel.create({})
        // create org
       foundOrgModel = await OrganizationModel.create({ members: [{ user: user._id }] })
    }

    foundOrgModel = await OrganizationModel.findOne({})
    const members = R.clone(foundOrgModel.members);
    console.log(members[0]._id.toString()) // crashes
}();

Related issue here: mongodb/js-bson#643. So @Axosoft-Ashley please re-run npm install, and update your package-lock.json to use bson@6.4 if you have a package-lock.json.

@Axosoft-Ashley
Copy link
Author

@vkarpov15 I tested in my playground and it did appear to fix the issue with 6.4.0 of bson! Thank you 😄

vkarpov15 added a commit that referenced this issue Mar 4, 2024
fix(document): make `$clone` avoid converting subdocs into POJOs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug We've confirmed this is a bug in Mongoose and will fix it.
Projects
None yet
Development

No branches or pull requests

4 participants