trailsjs / trailpack-sequelize

:package: Sequelize.js Trailpack http://sequelizejs.com
MIT License
5 stars 9 forks source link

Throw SequelizeUniqueConstraintError: E_VALIDATION #23

Closed mautematico closed 7 years ago

mautematico commented 8 years ago

When your insertion throws a SequelizeUniqueConstraintError, trails should throw an E_VALIDATION error that reachs the api's JSON response, as it does (currently only with Express) when there is a SequelizeValidationError.

At least on hapi, current behaivor is to display an 500, internal server error on the api's JSON response.

jaumard commented 8 years ago

Thanks for this @mautematico is there any other sequelize errors that can be useful to send back to the users ? Yeah currently hapi trailpack doesn't do this but I think it should :D

mautematico commented 8 years ago

Sure there are! I am just starting with sequelize (and trails), and I'll create a new PR when I found them. :D

jaumard commented 8 years ago

Hum I just think about the SequelizeUniqueConstraintError and I don't think this kind of errors should be return to the user because it was a SQL errors. Validation errors are useful to the users for form validations but database errors just should return a 500 in my opinion.

@cesar2064 @konstantinzolotarev @tjwebb can I have your advice on this ?

mautematico commented 8 years ago

I think this is a Validation error. But even sequelize https://github.com/sequelize/sequelize/blob/3e5b8772ef75169685fc96024366bca9958fee63/lib/errors.js#L133 seems to think it is a DatabaseError

I really would like to distinguiss between an 'Internal server error' and an 'hey dude! you're trying to duplicate something', that way I can take one aproach if, let's say, the record I am trying to insert already exists, or if my input is wrong format. But these are only my tougths.

cesar2064 commented 8 years ago

@jaumard I agree that is a server error and is a sql error, but I think is very helpful to display the error to the user, because it allowing quickly corrections to errors, when the user found them. For example this is what spring hibernate returns by default when there was a sql validation error:

{"timestamp":1467120285976,"status":500,"error":"Internal Server Error","exception":"org.springframework.dao.DataIntegrityViolationException","message":"could not execute statement; SQL [n/a]; constraint [null]; nested exception is org.hibernate.exception.ConstraintViolationException: could not execute statement","path":"/xxxx/xxxx"}

jaumard commented 8 years ago

@cesar2064 but it's sending a 500 not a 400 :) also I think spring only send this in dev mode not in production mode, can you confirm ? Because for me returning database errors like this is a security risk (it give architecture informations to hackers).

cesar2064 commented 8 years ago

@jaumard you are right is only in dev mode, because sending this message to the user is a bad practice, in prod mode those kind of errors are stored in a log file.

jaumard commented 8 years ago

Thanks @cesar2064 So I'm Ok to send all errors on dev mode too (with a 500 not 400) if it's really needed. And log the error whatever the mode if it's not a ValidationError :) it can be useful @mautematico can you update your PR in this way ?

jaumard commented 7 years ago

Closing as no update since a while