codepunkt / mongoose-patch-history

Mongoose plugin that saves a history of JSON patch operations for all documents belonging to a schema in an associated 'patches' collection
MIT License
96 stars 21 forks source link

Timeout upon saving a new document #74

Open hk7math opened 3 years ago

hk7math commented 3 years ago

I'm new to document versioning and have been trialing on a few similar plugins. It may be my mis-configuration, but I have really no idea what's going wrong after attempting for a whole day...

When I comment out the plugin-related script, Mongoose works as expected. But if the plugin is applied, new document cannot be saved and results in timeout...

Debug: handler, error
    Error: Operation `issue_patches.insertOne()` buffering timed out after 10000ms        
    at Toolkit.badRequest (...\microservice-issue\lib\boom.js:43:34)
    at handler (...\microservice-issue\lib\controllers\issue.js:95:16)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)

Below is my models/issue.js.

'use strict'

const Mongoose = require('mongoose')
// const History = require('mongoose-patch-history').default

const issueSchema = new Mongoose.Schema({
  trelloId: String,
  trelloUrl: String,
  parcelNo: String,
  warehouse: String,
  courier: String,
  issueReason: Number,
  photo: [{
    url: String,
    remark: String
  }],
  status: {
    type: String,
    default: 'To Do'
  },
  updatedBy: String
})

// issueSchema.plugin(History, {
//   mongoose: Mongoose,
//   name: 'issuePatches'
// })

module.exports = {
  name: 'Issue',
  schema: issueSchema
}

Versions: node: 12.18.4 mongodb: 4.2.11 mongoose: 5.11.4 mongoose-patch-history: 2.0.0

Appreciate any help. Thank you!

codepunkt commented 3 years ago

Sounds strange. Can you provide a simplified example repository that allows to reproduce this and versioning information for everything that's not versioned through your repo like mongodb itself?

vinerich commented 3 years ago

Hey @hk7math, this could possibly be related to #73. Could you try out the code from this branch or else downgrade your mongoose version to <= 5.9.6 as stated here https://github.com/codepunkt/mongoose-patch-history/issues/71#issuecomment-699845276.

hk7math commented 3 years ago

Sounds strange. Can you provide a simplified example repository that allows to reproduce this and versioning information for everything that's not versioned through your repo like mongodb itself?

I remake an example repo for my case. It used hpal to boilerplate a hapi server, and I added some routes with mongodb connection (4.2.11) Mongo related scripts are here:

this could possibly be related to #73. Could you try out the code from this branch or else downgrade your mongoose version to <= 5.9.6 as stated here #71 (comment).

I tried downgrading mongoose version with the following npm list mongoose result. The timeout still happens with the plugin but POST goes properly without it.

npm list mongoose
paldo-riddles@1.0.0 [masked project path]
+-- mongoose@5.9.6
`-- mongoose-patch-history@2.0.0
  `-- mongoose@5.9.6  deduped

Thank you very much!

hk7math commented 3 years ago

For reference, it seems other people encounter similar error on Mongoose Repo However, my example runs fine when the patch plugin is not used.. ><

vinerich commented 3 years ago

I can say that the issue discussed here is somehow related to the saving of the patch.

In mongoose there are 2 options (probably) to save a new document. Either with schema.create() or with new model(doc).save(). Both calls result in timeouts, if called on our Patches Schema, but work fine if called on your Riddle Schema.

I may have time this evening to do some further debugging. However I don't know if this is related to the mongoose issue you pointed at, since this is relatively new and our issue arises even with older versions of mongoose.

This may be related to how the mongoose connection is defined or how hapi manages these connections. I will probably know more in the evening.

Thanks for adding additional info @hk7math!

vinerich commented 3 years ago

Summary

The timeout error origins in an undefined mongoose connection for our mongoose-patch-history plugin.

Quick workaround: Change Mongoose.createConnection() to Mongoose.connect() in lib/index.js. This uses the default mongoose connection instead of creating a new one.

Details

Background Knowledge

The mongoose-patch-history plugin retrieves the used mongoose connection (instance) from the options as stated in the documentation (first parameter).

Usually one just imports/requires the mongoose module and give that as a parameter. Happening under the hood is the following:

  1. Mongoose maintains an internal connections array which is pre-initialized with a default connection (state = disconnected)
  2. Plugin uses options.mongoose.model(PatchSchema) to create the model used for saving patches to the db
  3. mongoose.model uses the connection assigned to the mongoose-instance which, in this case is the default mongoose connection which is defined as mongoose.connections[0], the pre-initialized connection mentioned in 1.

Specific error

In your sample repo you pass the default imported mongoose instance as done in our usage example.

This default mongoose instance however has no connection assigned to it, since the connection is created with Mongoose.createConnection. createConnection() adds a new connection to the connections array maintained by the mongoose instance.

Since a new connection is added, the default connection (connections[0]) is still uninitialized. Hence our plugin has no connection to work with.

Solutions

  1. Exchange createConnection() with connect() (example-repo-ref) to use the default mongoose connection instead of creating a new one.
  2. Predefine the (uninitialized) connection with const db = mongoose.createConnection(); and pass that to the mongoose-patch-history-plugin options. After open the connection with db.openUri(...). (mongoose-doc-ref)

@codepunkt I think a PR is needed where the connection state is examined before each patch operation. If the connection is not in state connected we can either throw an error or wait for a timeout (maybe add an option for this) and then throw an error. I'd see this error as a user error but nevertheless we can improve on user experience with a good error msg.

I'd of course volunteer to tackle this but only if you have the time to review and merge the PR, or else we are talking about the maintainer role again.

hk7math commented 3 years ago

@vinerich Thank you so much!

I have updated the hapi example accordingly and it works fine now=)