MongoDB Node.js driver's ObjectId is an empty object on client

[baryshok]

Summary

While trying to update our app from Meteor 2.5.6 to 2.7.1 I've noticed the following issue: If a document have a field of type ObjectId (MongoDB Node.js driver's one, not Meteor's Mongo.OjbectID) and we send this document to the client via Meteor.methods then on the client that field arrives as an empty object though previously it has arrived as an object with _bsontype and id fields which we tend to use on the client.

Steps to reproduce (Meteor 2.7.1)

1. git clone [email protected]:baryshok/meteor-test-objectid-271.git
2. meteor npm i
3. meteor
4. Open http://localhost:3000 in browser
5. Click on "Click Me" button
6. Check the browser's console – oid field should be an empty object:

How it used to be (Meteor 2.5.6)

1. git clone [email protected]:baryshok/meteor-test-objectid-256.git
2. meteor npm i
3. meteor
4. Open http://localhost:3000 in browser
5. Click on "Click Me" button
6. Check the browser's console – oid field should be an object containing _bsontype and id fields:

Operating system: MacOS 12.3.1

I'm sure that this issue is directly related to the update of MongoDB Node.js driver in Meteor 2.6 and I hope that it's possible to get back the _bsontype and id fields.

This bug report should include:

• [x] A short, but descriptive title. The title doesn't need "Meteor" in it.
• [x] The version of Meteor showing the problem.
• [x] The last version of Meteor where the problem did not occur, if applicable.
• [x] The operating system you're running Meteor on.
• [x] The expected behavior.
• [x] The actual behavior.
• [x] A simple reproduction! (Must include the Github repository and steps to reproduce the issue on it.)

If you don't include a reproduction the issue is probably going to be closed.

Independent packages

Please ensure your issue is opened in the appropriate repository:

• Feature Requests: https://github.com/meteor/meteor/discussions
• Blaze: https://github.com/meteor/blaze/
• Galaxy Guide: https://github.com/meteor/galaxy-docs/ -->

[denihs]

Hi @baryshok, I did some investigating work on this and concluded this isn't a Meteor issue. But you're right when you said is issue is related to the update of the MongoDB Node.js driver.

Just for context, MongoDB driver uses a package called bson to create the ObjectId.

In Meteor 2.5.6, we were using the MongoDB driver in version 3.6.10 which used bson in version 1.1.4. Then we updated the driver to version 4.3.1, which uses bson in version 4.6.1. At this version, the ObjectID does not return the values you were getting before. I couldn't distinguish from their history where they changed things, but based on all this, I can say this isn't coming from Meteor.

You could open an issue on their repository. But if you're willing to change your code, what you would need to do is change your code to new MongoInternals.NpmModule.ObjectID().toString() and update the client side to expect a string instead of an object.

[baryshok]

Thank you for the investigation, @denihs! I also did an investigation today based on your results and here's what I've found:

In bson package 1.1.4, _bsontype and id were enumerable own properties of the ObjectID:

On the other hand, in bson package 4.6.1, _bsontype and id properties are still present but they are now defined on the ObjectId.prototype so they are not own properties of the ObjectId and they are non-enumerable either:

Object.defineProperty(ObjectId.prototype, '_bsontype', { value: 'ObjectID' });

Object.defineProperty(ObjectId.prototype, "id", {
    /**
     * The ObjectId bytes
     * @readonly
     */
    get: function () {
        return this[kId];
    },
    set: function (value) {
        this[kId] = value;
        if (ObjectId.cacheHexString) {
            this.__id = value.toString('hex');
        }
    },
    enumerable: false,
    configurable: true
});

(This is the code from objectid.js module which is compiled from the original objectid.ts one)

To transfer an ObjectId value using DDP protocol we stringify it with DDPCommon.stringifyDDP method (source) and the first step of this method is EJSON.clone(msg) and that's exactly where an instance of an ObjectId class becomes an empty object because the cloning algorithm treats ObjectId as a plain object and tries to copy only its own enumerable properties (source) which the ObjectId does not have anymore:

I really think that DDP should not change ObjectId into an empty object while stringifying it because ObjectId has everything we need for stringification: toJSON, toString, toHexString methods.

So, how about adding special handling of an ObjectId to EJSON.clone method? For instance, we may return the ObjectId value as is (unfortunately, we cannot really clone the ObjectId by instantiating a new ObjectId from the original one on the client-side) and then in the DDPCommon.stringifyDDP method when we call JSON.stringify, the ObjectId will be converted into a hex string automatically since it has a dedicated toJSON method for it.

What do you think about this solution?

[radekmie]

Maybe ObjectId should be registered as an EJSON type (EJSON.addType)?

[plb666]

We had the same problem when invoking methods that return objects to the client from native calls to mongo (eg aggregate). As a workaround, we have overridden the EJSON.clone function as:

((fn) => {
    EJSON.clone = function (obj) {
        if(obj?.constructor?.name === 'ObjectId')
            return EJSON.clone( new Mongo.ObjectID(obj.toString()));

        return fn(obj);
    };
})(EJSON.clone)

[baryshok]

Maybe ObjectId should be registered as an EJSON type (EJSON.addType)?

I've checked that idea in the following repo: https://github.com/baryshok/meteor-test-add-type-for-object-id

In this repo I use local package ejson-objectid to add EJSON.addType("ObjectId", ...) and then test a number of cases: At least my implementation of EJSON.addType approach works only for cases when we construct ObjectId in the app's code (both on the client and on the server) because we import ObjectId from the meteor/ejson-objectid package where ObjectId from bson package is extended with the properties required for EJSON.addType to work (typeName, toJSONValue, and clone).

But if an ObjectId value comes from the MongoDB Node.js driver (on the server) then it doesn't have those properties (typeName, toJSONValue, and clone) and then EJSON._isCustomType inside EJSON.clone (source) doesn't recognize an ObjectId value and it's being treated like a plain object resulting in an empty object value after cloning, just like before.

So, unless I'm missing something, EJSON.addType approach doesn't help fixing the issue of ObjectIds being transformed into empty objects after cloning

[radekmie]

And what if you’d extend the ObjectId‘s prototype? I know it’s a hack + potentially a small performance impact, but it should do the trick.

Edit: My bad, I haven’t checked the code before. It is already doing that. Edit 2: I don’t understand why it’s not working. It shouldn’t matter where these are coming from - if the method is on the prototype, all instances should have it. Is there another, internal ObjectId? Maybe you should use the one from MongoInternals?

[baryshok]

Is there another, internal ObjectId?

Yes, MongoDB Node.js driver imports ObjectId directly from bson npm package (source)

Maybe you should use the one from MongoInternals?

Yes, that's the way to go for the server-side. I've made a PR where:

• Still use ObjectId from bson npm package on the client-side.
• Use MongoInternals.NpmModule.ObjectID from mongo atmosphere package on the server-side.

We cannot use MongoInternals.NpmModule.ObjectID on the client-side because MongoInternals is exported by mongo package for the server-side only (source) which is caused by NpmModuleMongodb being exported by npm-mongo package for the server-side only (source)

Now, the resulting ejson-objectid package in the demo repo seems to me like a solid solution for deserializing ObjectId into a native implementation on both server and client sides when working with rawCollections

[radekmie]

OK, so we'd have to do it differently on the client and on the server - makes sense. Now I'm just thinking whether it should not be covered in the mongo-id package. That'd make it available through Mongo.ObjectId, making it easier for everyone.

What do you think?

[radekmie]

@baryshok what do you think about it? We could handle it in the core packages to make life easier for everyone.