-
Notifications
You must be signed in to change notification settings - Fork 529
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
Improved serialization failure messages #1033
Conversation
81e0949
to
fa37d1e
Compare
fa37d1e
to
cb907d2
Compare
try do | ||
Type.Scalar.serialize(schema_node, value) | ||
rescue | ||
Absinthe.SerializationError -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not in love with how the choice to have serializers succeed or raise puts us in this fun position of having to raise / rescue / raise the same exception but I don't really want to go down the rabbit hole of changing how all of this works right now, and what we're doing here to improve the error message doesn't really box us in for any future changes.
@@ -1,5 +1,9 @@ | |||
# Changelog | |||
|
|||
## 1.6.1 | |||
|
|||
- Feature: [Improved serialization failure messages](https://github.com/absinthe-graphql/absinthe/pull/1033) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@binaryseed I'm somewhat inclined to push for PRs to add their own changelog entries when we think one is applicable. This ensures they actually happen and involves the PR author in the description of the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only tricky bit is the lack of a PR # before submitting :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is true, I had to add mine as a commit after opening the PR. Still, that doesn't seem like too painful of a requirement.
This changes the relatively unhelpful message we have today:
into something that actually tells you what field to go look for to fix the issue: