Better invalid json message (issue #558)

[JoeMShanahan]

See #558.

First pass. Since these shows all errors for parses named with <?> it leads to some other jankiness:

> eitherDecode "{ 3" :: Either String Value
Left "Error in $: Failed reading: satisfy, valid json, 34"

The "34" refers to a double quote character and comes from https://hackage.haskell.org/package/attoparsec-0.13.1.0/docs/src/Data-Attoparsec-ByteString-Internal.html#word8

Suggestions welcome. I feel like this is slightly better but this could potentially be even more confusing.

[Lysxia]

I think it should read in this order "valid json, 34, satisfy", or the other way around, the reason being that the error comes from value' calling word8 calling satisfy.

The 34 and satisfy parts of the message suggest some improvements to be made in attoparsec rather than in aeson: word8 could be more descriptive, saying instead something like "expected character 34 (")", and satisfy (or a new version of it) could require an error string (which word8 would provide, thus removing the (<?>) wrapper) instead of providing an uninformative "satisfy" error.

[Lysxia]

"valid json" is indeed misleading, since the error is that the JSON is in fact invalid, but that's easy to change of course. Another issue is that annotating this recursive parser is going to result in large stacks of duplicated "valid json" strings when parsing nested structures like "[[[[[[[[[[[[[[[[[[[[[[[". Instead, you can insert this error string just in eitherDecodeWith, or perhaps add more descriptive information here, such as the current position, that may become quite heavy. It can be helpful as well, for instance by indicating where the potential mismatched brackets are, but there may also be a more lightweight solution.

[Lysxia]

Maybe also add the first component of Fail (position?) in the error message.

[JoeMShanahan]

👍

[bergmark]

Don't mind the benchmark failure, it's unrelated. I tried to fix it yesterday but travis was confusing me.

It would be nice to have some tests that show what the error messages will look like now for simple cases and some more complex ones. Like @Lysxia mentioned it might have some unwanted consequences.