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

Ensure bulk create returns plain objects when lean is 'true' #186

Merged
merged 3 commits into from Aug 14, 2017

Conversation

DesignByOnyx
Copy link
Contributor

Summary

This closes #161

The tests for this are coming in a separate PR in feathers-service-tests.

@DesignByOnyx
Copy link
Contributor Author

This test is not failing for me locally. I did a fresh npm install and everything passes. Please advise.

@ekryski
Copy link
Member

ekryski commented May 3, 2017

@DesignByOnyx which version of Node are you running. It's failing on Node v7.9.0 when checking the ORM error.

Uncaught TypeError: Cannot convert undefined or null to object
      at Function.keys (<anonymous>)
      at ValidationError.toString (node_modules/mongoose/lib/error/validation.js:49:10)
      at Function.prepareStackTrace (node_modules/source-map-support/source-map-support.js:365:16)
      at Function.captureStackTrace (<anonymous>)
      at new MongooseError (node_modules/mongoose/lib/error.js:12:11)
      at ValidationError (node_modules/mongoose/lib/error/validation.js:17:19)
      at model.Document.invalidate (node_modules/mongoose/lib/document.js:1458:32)
      at node_modules/mongoose/lib/document.js:1334:17
      at validate (node_modules/mongoose/lib/schematype.js:705:7)
      at node_modules/mongoose/lib/schematype.js:742:9
      at Array.forEach (native)
      at SchemaString.SchemaType.doValidate (node_modules/mongoose/lib/schematype.js:710:19)
      at node_modules/mongoose/lib/document.js:1332:9
      at _combinedTickCallback (internal/process/next_tick.js:73:7)
      at process._tickCallback (internal/process/next_tick.js:104:9)

I'll see if I can reproduce locally.

@DesignByOnyx
Copy link
Contributor Author

DesignByOnyx commented May 3, 2017

I tested against Node v7.9.0 before posting my last comment. The test that is failing is a rather simple test, so this really boggles me.

It's worth noting that this is a "new" test - the test was written a long time ago but wasn't actually being executed prior to this PR.

After further investigation, there's a slightly different implementation of how errors are emitted between feathers-sequelize and feathers-mongoose. This is best explained by looking at code:

https://github.com/feathersjs/feathers-sequelize/blob/master/src/utils.js#L32
https://github.com/feathersjs/feathers-mongoose/blob/master/src/error-handler.js#L44

I attempted to make feathers-mongoose throw errors the same as feathers-sequelize and it fixed the error above but ended causing other errors. Hit me up in Slack so we can talk about this.

@daffl daffl merged commit 2f9e0b7 into feathersjs-ecosystem:master Aug 14, 2017
@daffl
Copy link
Member

daffl commented Aug 14, 2017

Merging with latest and re-running fixed it. Released as v5.1.2, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make sure create many works with lean:true. Follow up on #160
3 participants