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

Error: Converting circular structure to EJSON: #74

Open
helipilot50 opened this issue Aug 6, 2021 · 9 comments
Open

Error: Converting circular structure to EJSON: #74

helipilot50 opened this issue Aug 6, 2021 · 9 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@helipilot50
Copy link

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",

@arielboitas
Copy link

same here

@jonasmeier1212
Copy link

jonasmeier1212 commented Sep 2, 2021

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.

@lorensr
Copy link
Member

lorensr commented Sep 4, 2021

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.

@lorensr lorensr added bug Something isn't working help wanted Extra attention is needed labels Sep 4, 2021
@leonardootoni
Copy link

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.

@lorensr
Copy link
Member

lorensr commented Aug 11, 2022

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

@leonardootoni
Copy link

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.

@lorensr
Copy link
Member

lorensr commented Aug 11, 2022

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.

@jonasmeier1212
Copy link

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.

@leonardootoni
Copy link

leonardootoni commented Aug 11, 2022 via email

drinkataco pushed a commit to drinkataco/location-events-api that referenced this issue Oct 10, 2022
Currently apollo-datasource-mongodb semi-supports mongoose. This is the
second patch required to allow caching to work correctly - otherwise
bjson throws circular reference errors due to how mongoose hydrates
objects. By changing the mongoose integration to always return lean we
can fix the conversion.

Known Issue: GraphQLGuide/apollo-datasource-mongodb#74
Mongoose Lean: https://mongoosejs.com/docs/tutorials/lean.html
drinkataco pushed a commit to drinkataco/location-events-api that referenced this issue Oct 10, 2022
Currently apollo-datasource-mongodb semi-supports mongoose. This is the
second patch required to allow caching to work correctly - otherwise
bjson throws circular reference errors due to how mongoose hydrates
objects. By changing the mongoose integration to always return lean we
can fix the conversion.

Known Issue: GraphQLGuide/apollo-datasource-mongodb#74
Mongoose Lean: https://mongoosejs.com/docs/tutorials/lean.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

5 participants