[Bug]: ObjectId in meta transformed to empty object

[alice-telescoop]

🔎 Search Terms

ObjectId,mongo

The problem

When using winston-mongodband logging mongo documents, the _id (and I think any property typed ObjectId) is transformed to {}

I encounter this when logging authenticated user through express-winston.

I use mongodb driver version 4.3.0

What version of Winston presents the issue?

v5.1.1

What version of Node are you using?

v18.17.1

If this worked in a previous version of Winston, which was it?

No response

Minimum Working Example

No response

Additional information

While debugging the problem on my side, I figured that it is the helper cloneMeta that causes the issue : the test node instanceof ObjectID returns false whereas I guess it should return true

function cloneMeta(node, optParents) {
  if (!((node !== null && typeof node === 'object') || typeof node === 'function')
      || (node instanceof ObjectID) || (node instanceof Buffer)) {
    return node;
    }
    // ...
}

[curledUpSheep]

I think this is caused by the fact that multiple conflicting versions of the mongodb package are being used in your project, which leads to multiple conflicting versions of the bson package (which defines the ObjectId class) being installed.
Due to this, the ObjectId class used by your application's normal MongoDB interactions is different from the ObjectId class that is used by winston-mongodb to figure out whether something is an ObjectId or not.

The root cause of this seems to be that you are using v4.x.x of the mongodb package while winston-mongodb wants to use v3.x.x of the mongodb package (so winston-mongodb got its own separate version 3.7.4 of mongodb while the rest of your application is using version 4.17.0).

[alice-telescoop]

Indeed by looking into my node_modules and package-lock.json I find what you describe. If I understand correctly this means that my three choices are : workaround somehow, downgrade the project's mongodb version to 3.7.4, or wait that winston-mongodb gets compatible with newer mongodb driver's versions.

Anyway thanks for the explanation, it's good to at least understand what's going on

[curledUpSheep]

An alternative would be that winston-mongodb changes the implementation of the cloneMeta function so it only looks at the class name instead of doing an instanceof check.
Something like node.constructor.name === 'ObjectID instead of node instanceof ObjectID.

In theory this can lead to problems if someone decided to create their own class named "ObjectID" but I'm not sure if we need to care about that edge case in practice.
There are also some other edge cases that might need to be taken into account in the implementation, see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/name#telling_the_constructor_name_of_an_object.

The benefit of doing this would be that winston-mongodb would then work with an application using any version of the mongodb driver as far as I can tell, without having to change the mongodb driver version that winston-mongodb uses internally.

@yurijmikhalevich @DABH thoughts?

[SarkarKurdish]

until a better solution cames up I've fixed it like this:

1: Change this line (node instanceof ObjectID) to (node.constructor.name === 'ObjectId') in cloneMeta function at the \node_modules\winston-mongodb\lib\helpers.js

2: use npx patch-package winston-mongodb to patch the package locally

3: Add this script below to package.json so every time I install packages it will patch it automatically

"scripts": {
  "postinstall": "patch-package"
 }

[alice-telescoop]

I did not know of patch-package. I'll share that with the team. Thanks for the idea, either we do that or not at least I learnt something

[DABH]

@curledUpSheep @SarkarKurdish I like this idea (testing the class name as a string). Obviously there are edge cases like you mention, but given the limited bandwidth we have for maintaining the project, this sounds like it would overall make things more robust (we don't have to worry when different mongodb drivers come out, as much). If one of you makes a PR, I can approve/merge (I forget if I have npm permissions but I can try to do a release as well).

[DABH]

(A related PR: #254)