jspears / mers

Mongoose Express Rest Service
MIT License
342 stars 42 forks source link

Validation errors on PUT cause crash #21

Open cheesun opened 10 years ago

cheesun commented 10 years ago

I'm using version 0.7.1-0, though I can see the same problem will probably occur with the master branch.

Basically, I put a data change through that I know will not pass validations, MERS will bail out and crash the server.

Looking near line 145 of lib/routes.js - When the mongoose schema validations or pre save hooks return an err but no ret, the attempt to run ret.toJSON() will fail.

I believe the second part of the obj.save callback should be wrapped in an else statement, as we should only be trying to respond with the object if there is no error. If you agree, I will raise a pull request for this change.

I'm not yet sure why this doesn't happen for POST (it doesn't seem to hit the identical looking code in the router.post(reGet, ... callback

pos undefined
error saving  { message: 'Validation failed',
  name: 'ValidationError',
  errors:
   { type:
      { message: 'Validator "enum" failed for path type with value `blargh`',
        name: 'ValidatorError',
        path: 'type',
        type: 'enum',
        value: 'blargh' } } } ValidationError: Validator "enum" failed for path type with value `blargh`
    at model.Document.invalidate (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:979:32)
    at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:948:16
    at validate (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:558:7)
    at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:574:9
    at Array.forEach (native)
    at SchemaString.SchemaType.doValidate (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:562:19)
    at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:946:9
    at process._tickCallback (node.js:415:13)
127.0.0.1 - - [Mon, 25 Nov 2013 14:23:34 GMT] "PUT /rest/user/529352c7e94222833d000001 HTTP/1.1" 200 51 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/31.0.1650.57 Safari/537.36"

/Users/cchoong/jobtend-api/node_modules/mongoose/lib/utils.js:419
        throw err;
              ^
TypeError: Cannot call method 'toJSON' of undefined
    at /Users/cchoong/jobtend-api/node_modules/mers/lib/routes.js:145:31
    at handleError (/Users/cchoong/jobtend-api/node_modules/mongoose/node_modules/hooks/hooks.js:92:18)
    at _next (/Users/cchoong/jobtend-api/node_modules/mongoose/node_modules/hooks/hooks.js:34:22)
    at fnWrapper (/Users/cchoong/jobtend-api/node_modules/mongoose/node_modules/hooks/hooks.js:159:8)
    at complete (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:964:5)
    at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:955:20
    at validate (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:556:18)
    at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:571:11
    at Promise.<anonymous> (/Users/cchoong/jobtend-api/node_modules/mongoose-unique-validator/index.js:34:9)
    at Promise.<anonymous> (/Users/cchoong/jobtend-api/node_modules/mongoose/node_modules/mpromise/lib/promise.js:162:8)
jspears commented 10 years ago

Thanks for the patch I'll try to get it merged shortly. Seems resdonable.

Sent from my iPhone

On Nov 25, 2013, at 9:34 AM, cheesun notifications@github.com wrote:

I'm using version 0.7.1-0, though I can see the same problem will probably occur with the master branch.

Basically, I put a data change through that I know will not pass validations, MERS will bail out and crash the server.

Looking near line 145 of lib/routes.js - When the mongoose schema validations or pre save hooks return an err but no ret, the attempt to run ret.toJSON() will fail.

I believe the second part of the obj.save callback should be wrapped in an else statement, as we should only be trying to respond with the object if there is no error. If you agree, I will raise a pull request for this change.

I'm not yet sure why this doesn't happen for POST (it doesn't seem to hit the identical looking code in the router.post(reGet, ... callback

pos undefined error saving { message: 'Validation failed', name: 'ValidationError', errors: { type: { message: 'Validator "enum" failed for path type with value blargh', name: 'ValidatorError', path: 'type', type: 'enum', value: 'blargh' } } } ValidationError: Validator "enum" failed for path type with value blargh at model.Document.invalidate (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:979:32) at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:948:16 at validate (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:558:7) at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:574:9 at Array.forEach (native) at SchemaString.SchemaType.doValidate (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:562:19) at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:946:9 at process._tickCallback (node.js:415:13) 127.0.0.1 - - [Mon, 25 Nov 2013 14:23:34 GMT] "PUT /rest/user/529352c7e94222833d000001 HTTP/1.1" 200 51 "-" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/31.0.1650.57 Safari/537.36"

/Users/cchoong/jobtend-api/node_modules/mongoose/lib/utils.js:419 throw err; ^ TypeError: Cannot call method 'toJSON' of undefined at /Users/cchoong/jobtend-api/node_modules/mers/lib/routes.js:145:31 at handleError (/Users/cchoong/jobtend-api/node_modules/mongoose/node_modules/hooks/hooks.js:92:18) at _next (/Users/cchoong/jobtend-api/node_modules/mongoose/node_modules/hooks/hooks.js:34:22) at fnWrapper (/Users/cchoong/jobtend-api/node_modules/mongoose/node_modules/hooks/hooks.js:159:8) at complete (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:964:5) at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/document.js:955:20 at validate (/Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:556:18) at /Users/cchoong/jobtend-api/node_modules/mongoose/lib/schematype.js:571:11 at Promise. (/Users/cchoong/jobtend-api/node_modules/mongoose-unique-validator/index.js:34:9) at Promise. (/Users/cchoong/jobtend-api/node_modules/mongoose/node_modules/mpromise/lib/promise.js:162:8)

— Reply to this email directly or view it on GitHubhttps://github.com/jspears/mers/issues/21 .