davidmarkclements / fast-safe-stringify

Safely and quickly serialize JavaScript objects
MIT License
348 stars 27 forks source link

this.$__path is not a function error when using with Mongoose #43

Open filetvignon opened 5 years ago

filetvignon commented 5 years ago

Whenever I try to use fast-safe-stringify with a Mongoose Object, I get the error "this.$__path is not a function". I tried investigating exactly what is it in mongoose documents that cause this but couldn't find anything.

JSON.stringify() works with mongoose objects just fine.

I'm using the newest version of both packages: fast-safe-stringify: 2.0.6, mongoose: 8.6.8, node: 10.16.1

davidmarkclements commented 5 years ago

can you please add a full stack trace?

filetvignon commented 5 years ago
TypeError: this.$__path is not a function
at String.Document.get (/app-root-folder/node_modules/mongoose/lib/document.js:1348:23)
at model.get [as _id] (/app-root-folder/node_modules/mongoose/lib/helpers/document/compile.js:145:32)
at model.idGetter (/app-root-folder/node_modules/mongoose/lib/plugins/idGetter.js:23:12)
at VirtualType.applyGetters (/app-root-folder/node_modules/mongoose/lib/virtualtype.js:137:25)
at model.Document.get (/app-root-folder/node_modules/mongoose/lib/document.js:1356:19)
at applyGetters (/app-root-folder/node_modules/mongoose/lib/document.js:2986:22)
at model.Document.$toObject (/app-root-folder/node_modules/mongoose/lib/document.js:2750:5)
at model.Document.toJSON (/app-root-folder/node_modules/mongoose/lib/document.js:3061:15)
at JSON.stringify (<anonymous>)
at stringify (/app-root-folder/node_modules/fast-safe-stringify/index.js:11:18)
at NativeConnection.db.once (/app-root-folder/test.js:36:17)
at process._tickCallback (internal/process/next_tick.js:68:7)
filetvignon commented 5 years ago

I realize now this doesn't happen with just ANY mongoose document, I'll try to narrow down what the culprits could be.

filetvignon commented 5 years ago

Ok, I was able to figure out how to reproduce the issue. This error only happens when there are nested objects in schema AND option toJSON.virtuals is set to true. Also, using safeStringify.stableStringify doesn't throw an error.

Here's the code:

const mongoose = require('mongoose');
const db = mongoose.connection;
const SafeStringify = require('fast-safe-stringify');

mongoose.connect('mongodb://localhost:27017/local');

const schema = new mongoose.Schema(
  {
    key: {
      nestedKey: String,
    },
  },
  {
    toJSON: {
      virtuals: true,
    },
  }
);

const M = mongoose.model('M', schema);

db.once('open', async () => {
  try {
    const doc = new M();
    console.log('Works before saving document', SafeStringify(doc));
    await doc.save();
    console.log('Works using stableStringify', SafeStringify.stableStringify(doc));
    console.log('This will throw an error', SafeStringify(doc));
  } catch (err) {
    console.error(err);
  }
  db.close();
});

And here are the logs:

Works before saving document {"_id":"5d4c6c44d21e3504dabef3f1","id":"5d4c6c44d21e3504dabef3f1"}
Works using stableStringify {"_id":"5d4c6c44d21e3504dabef3f1","__v":0,"id":"5d4c6c44d21e3504dabef3f1"}
TypeError: this.$__path is not a function
    at String.Document.get (/app-root-folder/node_modules/mongoose/lib/document.js:1349:21)
    at model.get [as _id] (/app-root-folder/node_modules/mongoose/lib/helpers/document/compile.js:145:32)
    at model.idGetter (/app-root-folder/node_modules/mongoose/lib/plugins/idGetter.js:23:12)
    at VirtualType.applyGetters (/app-root-folder/node_modules/mongoose/lib/virtualtype.js:137:25)
    at model.Document.get (/app-root-folder/node_modules/mongoose/lib/document.js:1366:19)
    at applyGetters (/app-root-folder/node_modules/mongoose/lib/document.js:3012:22)
    at model.Document.$toObject (/app-root-folder/node_modules/mongoose/lib/document.js:2776:5)
    at model.Document.toJSON (/app-root-folder/node_modules/mongoose/lib/document.js:3087:15)
    at JSON.stringify (<anonymous>)
    at stringify (/app-root-folder/node_modules/fast-safe-stringify/index.js:11:18)
    at NativeConnection.db.once (/app-root-folder/test.js:28:45)
    at process._tickCallback (internal/process/next_tick.js:68:7)

It's funny though, if we also set option toJSON.id to false, error doesn't happen.

const schema = new mongoose.Schema(
  {
    key: {
      nestedKey: String,
    },
  },
  {
    toJSON: {
      virtuals: true,
    },
    id: false,
  }
);

const M = mongoose.model('M', schema);

db.once('open', async () => {
  try {
    const doc = new M();
    await doc.save();
    console.log('This will now work!', SafeStringify(doc));
  } catch (err) {
    console.error(err);
  }
  db.close();
});
filetvignon commented 5 years ago

So, the problem seems to be when mongoose tries to get the "id" virtual of a subdocument. Declaring this schema, which is equivalent to the one above, will also work as it will make the subdocument NOT have an _id field (and, consequently, not an "id" virtual).

Sorry, should I be bringing this issue to mongoose instead? It seems to be very specific.

const schema = new mongoose.Schema(
  {
    key: {
      type: {
        nestedKey: String,
      },
      _id: false,
    },
  },
  {
    toJSON: {
      virtuals: true,
    },
  }
);
BridgeAR commented 5 years ago

It seems the decycling breaks the mongoose toJSON function. It could be that the function was replaced in case it's cyclic. But that should not break mongoose, since JSON.stringify works for you on the very same object. It's an interesting edge case that should be fixed here.

@filetvignon would you be so kind and check if https://github.com/BridgeAR/safe-stable-stringify works for you? I guess that might work as the decycling works different.

filetvignon commented 5 years ago

Hey @BridgeAR, I have checked it and it does work! =] I'm just a little apprehensive in using it as it's said here to be experimental and all, but I'm willing to make the switch.

BridgeAR commented 5 years ago

@filetvignon great!

It is out there for quite some time and it should be very stable. I would actually call it safer to use since it has less special handling than this implementation.

Le-Stagiaire commented 4 years ago

Do we have any update on this ? Other than using a fork

mcollina commented 4 years ago

I think this would need a a volunteer to write a PR to fix it - we are not mongoose users, and it's not really a priority for us atm.