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

Foreign languages used in the db might crash the app #3567

Merged
merged 1 commit into from Apr 21, 2015

Conversation

2 participants
@BridgeAR
Contributor

BridgeAR commented Apr 20, 2015

The error messages are not parsed in a multi language friendly way. If the database in use is not english, error messages are checked with a match that might fail. This fixes the app crash.

This is definitly not a very good solution but more a kind of a hotfix. After all some time should be invested to check if there's a better solution. I couldn't come up with a good regression test for this either. Probably the best way to write a regression test for this is to fake each error with a foreign language. I do not have the time for that right now though.

janmeier added a commit that referenced this pull request Apr 21, 2015

Merge pull request #3567 from BridgeAR/fix-error-msg
Foreign languages used in the db might crash the app

@janmeier janmeier merged commit 41a9e50 into sequelize:master Apr 21, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@janmeier

This comment has been minimized.

Member

janmeier commented Apr 21, 2015

+1 for hotfix :)

I'm pretty sure there is no better solution for this, at least not at the sequelize level. Maybe if the DB actually returned the information we need, instead of just strings, but then some handling would need to be implemented at the driver level as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment