apollo-datasource-mongodb

apollo-datasource-mongodb copied to clipboard

Hello,

I get the following error when I specify a TTL on findOneById, findByFields, etc

Error: Converting circular structure to EJSON:

I'm sure that it is caused by something weird in my Mongoose schema, Do you have ideas on how to debug the issue

Version: "apollo-datasource-mongodb": "^0.4.6",

same here

I also have this issue, and I think it is caused by the way Mongoose models are represented in the memory. They always have circular structures. My solution for this is for now to just rely on lean Mongoose documents. They don't have any references to parents or something else it works again. In my usecase it is totally fine and also good for performance, but I'm not sure if there is something that speaks against this approach in general. @lorensr What do you think about using lean documents by default? If we can decide on a solution for this problem I will create a PR for this.

Or maybe add a flag to enable lean documents and then state in the documentation that the TTL option will only work in combination with lean documents when using Mongoose models.

I'm sure that it is caused by something weird in my Mongoose schema, Do you have ideas on how to debug the issue

There are a number of places in the code where EJSON.stringify is used. I recommend adding a test case to https://github.com/GraphQLGuide/apollo-datasource-mongodb/blob/master/src/tests/cache.test.js that reproduces the error, and then see which line the error is coming from, and we can think about how it can be modified.

@lorensr What do you think about using lean documents by default?

I'm not familiar with them, but I'm glad we at least have a workaround.

So, the problem happens using Mongoose because this library tries to serialize a Mongoose “Document” instance, which contains a bunch of methods such as getters/setters, validation, virtuals, etc. Depending on how complex/nested the mongoose model is, extra references(inner Mongoose stuff) should exist, and the serialization through bson.EJSON.stringfy() seems to get a bit confused traversing it and causes that error.

There are a few approaches I’ve tested that solve the problem:

1 – Fetch all mongoose models using .lean() to avoid Mongoose hydration. That substantially reduces the object’s size and complexity as well as speed-up the queries since it returns a plain JS object. Thus, simple to be serialized/deserialized without any problems. Here is a comprehensive explanation from Mongoose about it. It seems to be the best choice, at least for most of the use cases where a mongoose document instance is not necessary for further operations like save, update, delete;

2 – Change the serialization process. It is unclear why bson.EJSON.stringify is in place for that once Mongoose/Mongo Nodejs driver does not return BSON document, but JS Objects instead. On my tests, the infamous JSON.stringify() seems to offer better serialization performance than bson.EJSON(.stringify()/.parse()) and bson(.serialize(), deserialize()). Not to say that it also can serialize those hydrated mongoose documents without errors. There are also plenty of other options like avro-js and protobuf-js for faster serialization, but they should require additional tests.

@lorensr, we can discuss a pull request for that if you want/need.

It is unclear why bson.EJSON.stringify is in place

Might the JS objects have things like int32s that don't survive JSON.parse(JSON.stringify(o))?

https://github.com/mongodb/js-bson#ejsonstringifyvalue-replacer-space-options

AFAIK, the Nodejs Mongo driver is the responsible layer for parsing/translating BSON to native JS objects and prop values during CRUD operations through it.

Yeah, they're no longer binary BSON, but I think the object can have types that aren't supported by JSON (hence EJSON).

Would be happy to review a PR for approach 1.

We at B42 have done approach 1 on our fork: https://github.com/B42-Training/apollo-datasource-mongodb I will create a PR out of it, but it has been a time since we have done it, so maybe it needs a bit of cleaning up. Currently, I don't have the time for doing this, maybe I can do it in two weeks.

It's done.✌

On Wed, Aug 10, 2022 at 11:23 PM Loren ☺️ @.***> wrote:

Yeah, they're no longer binary BSON, but I think the object can have types that aren't supported by JSON (hence EJSON).

Would be happy to review a PR for approach 1.

— Reply to this email directly, view it on GitHub https://github.com/GraphQLGuide/apollo-datasource-mongodb/issues/74#issuecomment-1211514459, or unsubscribe https://github.com/notifications/unsubscribe-auth/AIQZSETPXVVDHUUV7DPVZSTVYRW4NANCNFSM5BWAJTKQ . You are receiving this because you commented.Message ID: @.***>

--

Best regards,

Leonardo Otoni de Assis