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

More verbose error messages when failing to parse database results in… #741

Merged
merged 4 commits into from
Jan 17, 2018

Conversation

MaxGabriel
Copy link
Member

…to Persistent records

Closes #689

Previous error message, as reported in #689:

"get UserKey {unUserKey = SqlBackendKey {unSqlBackendKey = 1}}: field foo: int Expected Integer, received: PersistText "foo"

New error message:

Error when calling get with key: UserKey {unUserKey = SqlBackendKey {unSqlBackendKey = 1}}. Failed to create User because of error: Couldn't parse field foo from database results: Tried to parse Haskell type Int, expected integer from database, but received: PersistText "foo". Potential solution: Check that your database schema matches your Persistent model definitions. Potential solution: If this is a custom PersistField instance, check that it's correct.

This is maybe a little bit more verbose than ideal, imo, but I lean towards a verbose error message because you should only see this error message if your PersistField instances are incorrect, or your database schema doesn't match your persist model definitions (afaik). It's also kind of hard to make a clean error message because the error is constructed from several different locations in the code.

Specific improvements:

  1. In the message: field foo: int Expected Integer, received: PersistText "foo", it's totally unclear what int is, and calling it int doesn't suggest that it's a Haskell type (vs Int).
  2. Make recommendations about potential solutions. Afaik these are the only solutions to this problem?
  3. Highlight the function being called. I think because it's just get it's hard to realize that function is the root of the problem.

Behavior change: A handful of the PersistField parsing functions called error instead of returning Left Text. I think since the vast majority returned Left that it's fine to change this. I also can't imagine anyone is relying on the error vs left distinction, especially because higher level functions like get just call error themselves on a Left.

Copy link
Member

@gregwebs gregwebs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome, looks like just a doc conflict with master now.

(Avoids GHC version monoid import issues)
…Encoding.Error

This was necessary because of the following situation:

1. I wanted to use Data.Text.replace inside the instance for Natural, which is in an ifdef for GHC 7.8
2. I couldn't do T.replace, because two modules aliased as T exported a replace
3. I couldn't import replace directly from Data.Text without getting unused function errors on old GHCs, because the function was ifdefed out

Since it probably should be done anyway, I disambiguated the module imports as T, TE, and TERR, allowing me to use T.replace
@MaxGabriel MaxGabriel merged commit 453ef08 into master Jan 17, 2018
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.

None yet

2 participants