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

Fixes #637; Preserve Err values when using Json.Decode.customDecoder #639

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@noahzgordon
Contributor

noahzgordon commented Jun 3, 2016

See issue: https://github.com/elm-lang/core/issues/637

This commit allows Err values returned by the second argument of Json.Decode.customDecoder to be preserved when the decoder fails. This way all user-defined error messages are not swallowed by the decoder.

jvoigtlaender referenced this pull request Jul 15, 2016

Improve error message for Json.Decode.customDecode
Thanks to @rtfeldman for showing me the bad message.
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jul 15, 2016

Member

I just ran into this independently and did a fix in 7f6e518. Someone pointed me here, which is very similar. My fix includes context information, so if you run into a customDecode nine levels into a JSON object, the error message will tell you about that context.

That said, it's probably still a good idea to test that the error message produced contains the Err str string as a subset. Are you willing to adapt your tests to the alternate implementation?

Member

evancz commented Jul 15, 2016

I just ran into this independently and did a fix in 7f6e518. Someone pointed me here, which is very similar. My fix includes context information, so if you run into a customDecode nine levels into a JSON object, the error message will tell you about that context.

That said, it's probably still a good idea to test that the error message produced contains the Err str string as a subset. Are you willing to adapt your tests to the alternate implementation?

@evancz evancz closed this Jul 15, 2016

@noahzgordon

This comment has been minimized.

Show comment
Hide comment
@noahzgordon

noahzgordon Jul 15, 2016

Contributor

@evancz Gladly! I will open a new PR shortly.

Contributor

noahzgordon commented Jul 15, 2016

@evancz Gladly! I will open a new PR shortly.

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