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

Improved serialization failure messages #1033

Merged
merged 3 commits into from Jan 20, 2021
Merged

Conversation

benwilson512
Copy link
Contributor

@benwilson512 benwilson512 commented Jan 20, 2021

This changes the relatively unhelpful message we have today:

** (Absinthe.SerializationError) Value 1.0 is not a valid integer

into something that actually tells you what field to go look for to fix the issue:

** (Absinthe.SerializationError) Could not serialize term 1.0 as type Int
When serializing the field:
RootQueryType.bad_integer (/path/to/schema/module.ex:8)

@benwilson512 benwilson512 changed the title nicer-serialization-errors Improved serialization failure messages Jan 20, 2021
try do
Type.Scalar.serialize(schema_node, value)
rescue
Absinthe.SerializationError ->
Copy link
Contributor Author

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)
Copy link
Contributor Author

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.

Copy link
Contributor

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 :)

Copy link
Contributor Author

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.

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