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

Improve error message when marshalling fails #846

Merged
merged 4 commits into from
Jan 27, 2019

Conversation

danbroooks
Copy link
Contributor

@danbroooks danbroooks commented Jan 14, 2019

Alters the error message that appears when marshalling DB values to include the name of the table. I have found it useful to know the table when this error rears its head with generic field names (lastAccessed for example), allowing me to go straight to the offending table.


I'm not sure which of these I need to complete for this change (bumping version number seems drastic, and I've not changed any public facing API):

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)

@psibi
Copy link
Member

psibi commented Jan 26, 2019

@danbroooks Thanks for the PR! I like this. :-) I think some part of the code change in the PR are just formatting. Can we avoid that or keep that in a separate commit ( I would like to avoid it if the code isn't changed for easier review and easier git-blame for future references) ?

Also, can you give a sample example of the previous and the current error message with this PR ?

@danbroooks danbroooks force-pushed the better-errors branch 3 times, most recently from fceefe5 to 7a82d3c Compare January 26, 2019 16:15
@danbroooks
Copy link
Contributor Author

danbroooks commented Jan 26, 2019

@psibi I have modified the commits so the re-arranging of stuff is separated from the changing of the error message. The change to the message simply adds the relevant table name to the error message as can be seen here:

8b1fa1d#diff-26b0e2f04e1da957f51921997c4ca77dR1016

@psibi psibi merged commit 8418e8a into yesodweb:master Jan 27, 2019
@psibi
Copy link
Member

psibi commented Jan 27, 2019

@danbroooks Thanks!

@psibi
Copy link
Member

psibi commented Jan 27, 2019

Also, a new version has been released to Hackage with the changes. Thanks!

@danbroooks danbroooks deleted the better-errors branch January 27, 2019 21:23
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