patch doesn't throw correct object

[shrynx]

Using auth for facebook generated by feathers cli, patch function fails , the issue is with the data parameter in service.js on line 164.

For more verbose explanation

  feathers-authentication:oauth2 Updating user: 57b71962784bec9f311e4462 +821ms
  feathers:rest Error in REST handler: `Invalid atomic update value for $__original_remo
ve. Expected an object, received function` +11ms
  feathers-authentication:middleware An authentication error occurred. +2ms Error: Inval
id atomic update value for $__original_remove. Expected an object, received function
    at Query._castUpdate (/home/shriyans/Projects/test-app/node_modules/mongoose/lib/que
ry.js:2378:13)
    at Query.update (/home/shriyans/Projects/test-app/node_modules/mongoose/lib/query.js
:2161:22)
    at Function.update (/home/shriyans/Projects/test-app/node_modules/mongoose/lib/model
.js:2056:13)
    at Object.patch (/home/shriyans/Projects/test-app/node_modules/feathers-mongoose/lib
/service.js:255:27)
  at /home/shriyans/Projects/test-app/node_modules/feathers-hooks/lib/hooks.js:74:31
    at new Promise (/home/shriyans/Projects/test-app/node_modules/core-js/modules/es6.pr
omise.js:191:7)
    at /home/shriyans/Projects/test-app/node_modules/feathers-hooks/lib/hooks.js:58:16
    at run (/home/shriyans/Projects/test-app/node_modules/core-js/modules/es6.promise.js:87:22)
    at /home/shriyans/Projects/test-app/node_modules/core-js/modules/es6.promise.js:100:28
    at flush (/home/shriyans/Projects/test-app/node_modules/core-js/modules/_microtask.j
s:18:9)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

[shrynx]

this is already reported as a bug in feathers-authentication library
feathersjs-ecosystem/authentication#244

2 solutions were suggested
1.> editing data in service.js on line 164. to data._doc (little dirty)
2.> globally setting {lean : true} for all queries

but these doesn't solve the real problem

[ekryski]

Thanks for the report @madmantalking! Fix incoming 😄

[shrynx]

@ekryski I am still able to reproduce the same error, and again am being able to solve it by changing line 215 in src/service.js

return this.Model
     .update(query, data._doc, options)

the problem is still with toObject() i guess.

[shrynx]

@ekryski To reproduce this bug i simply did,
1> Generate a new app with feathers generate
2> Choose authentication (i chose facebook , but i guess same for others too)
3> Db as Mongo DB.

Now when i first access /auth/facebook , i get a token successfully , (this is the create method)
If i access the url(/auth/facebook) again , i get login failed (this being the patch method).
with console error using DEBUG=feathers-authentication*

An authentication error occurred. +10ms Error: Invalid atomic update value for $__original_remove. Expected an object, received function
    at Query._castUpdate (/Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/mongoose/lib/query.js:2389:13)
    at Query.update (/Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/mongoose/lib/query.js:2161:22)
    at Function.update (/Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/mongoose/lib/model.js:2059:13)
    at Object.patch (/Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/feathers-mongoose/lib/service.js:282:27)
    at /Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/feathers-hooks/lib/hooks.js:74:31
    at new Promise (/Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/core-js/modules/es6.promise.js:191:7)
    at /Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/feathers-hooks/lib/hooks.js:58:16
    at run (/Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/core-js/modules/es6.promise.js:87:22)
    at /Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/core-js/modules/es6.promise.js:100:28
    at flush (/Users/shriyans.bhatnagar/Projects/feathers-app/node_modules/core-js/modules/_microtask.js:18:9)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)

Hope this helps!

[ekryski]

@madmantalking what is your feathers-mongoose version with the generated app?

[shrynx]

it is 3.5.2

[shrynx]

{
"dependencies": {
    "body-parser": "^1.15.2",
    "compression": "^1.6.2",
    "cors": "^2.8.0",
    "feathers": "^2.0.1",
    "feathers-authentication": "^0.7.9",
    "feathers-configuration": "^0.2.3",
    "feathers-errors": "^2.4.0",
    "feathers-hooks": "^1.5.7",
    "feathers-mongoose": "^3.5.2",
    "feathers-rest": "^1.4.4",
    "feathers-socketio": "^1.4.1",
    "mongoose": "^4.5.10",
    "passport": "^0.3.2",
    "passport-facebook": "^2.1.1",
    "passport-facebook-token": "^3.3.0",
    "serve-favicon": "^2.3.0",
    "winston": "^2.2.0"
  },
  "devDependencies": {
    "jshint": "^2.9.3",
    "mocha": "^3.0.2",
    "request": "^2.74.0"
  }
}

[frimoldi]

Same thing happening here, feathers-mongoose version 3.5.2

[daffl]

How is this reproducible? https://github.com/feathersjs/feathers-mongoose/pull/112/files#diff-0fd0e07cf6d02bf7cf00f18cebb8e6eaR152 is the tests for the original fix.

[frimoldi]

See @madmantalking comment above. Steps to reproduce the error are there.

[frimoldi]

Any news on this ?

[lewisblackwood]

I also encountered this issue with github authentication and mongoDB database.

Thanks @madmantalking for the fix above - would be great to see a general fix soon! 👍

[ekryski]

I'll try to take look this evening or tomorrow as part of the auth updates I am doing.

[MariMax]

How it goes? same bug for me

[laxgoalie392]

I've had this issue as well for a bit now.

I tried following @madmantalking's advice but:
fix 1: I didn't want to be updating imported libraries.
fix 2: I tried googling how to set lean globally and couldn't get it to work.

As I'm just playing around with the framework right now I found a not so bad approach to at least continue testing it.

In the generated code file src/services/user/index.js I added lean: true to the options passed into the service:

  const options = {
    Model: user,
    paginate: {
      default: 5,
      max: 25
    },
    lean: true,
  };

  // Initialize our service with any options it requires
  app.use('/users', service(options));

I'm not even sure what the lean option does but at least I can play around with featherjs more now! Maybe that will help someone play around as well until a fix comes out.

[daffl]

The lean option does not convert the data into a Mongoose model. A lot of issues (especially related to performance) seem to be fixed by setting this option. It makes it effectively the same as the plain MongoDB adapter with some validation. I'm not entirely sure why that makes a difference since update and patch already convert the data into a plain object now (which does not seem to be the same for some reason).

[ekryski]

@daffl hmm. I thought it changed the model as well but good to know. Maybe we should make it a default option then??

[shrynx]

@ekryski @daffl I also didn't know well about how mongoose works (I still don't know , Db is my weakest tool) , but if lean gives us performance and also solves core issues , then i think it should be part of cli tool (as it would help beginers like me).
I have been busy for a long while but next week onward i'll try to figure out the core issue.
Also, i just want to say , you guys are REALLY doing an awesome job. I have been using feathers for a project in my company for a while. And it is AWESOME !!!
Before starting feathers, i had little to no clue over service based architecture and discovered feathers because meteor wasn't giving me enough freedom ( too much "magic" happening , if i put it nicely).
Thanks a lot.

[ekryski]

Closing in favour of tracking in #132.