Support JSONPath in error messages
#1807
Conversation
JSON error messages can optionally include the JSON path so that you get a nicer error like `Error in $.foo.bar: rest of the message`. The trick is to parse into an `IResult` instead of a `Result` since `IResult` includes a `JSONPath`. This also updates the `requre*JsonBody` utilites to use these new `IResult`-based utilities, so anything that depends on them will automatically pick up improved error messages.
| eValue <- runConduit $ rawRequestBody .| runCatchC (sinkParser JP.value') | ||
| return $ case eValue of | ||
| Left e -> JT.IError [] $ show e | ||
| Right value -> JT.ifromJSON value |
There was a problem hiding this comment.
I see the symmetry with the parseInsecureJsonBody, but I must admit I'm curious about this implementation.
It seems like we have a two step process here - first piping the ConduitT _ ByteString _ () into a JSON parser to get an intermediate Value, and then calling ifromJSON on that value.
But eitherDecodeStrict works on lazy ByteString, and calls ifromJSON too:
-- | Like 'decodeStrict' but returns an error message when decoding fails.
eitherDecodeStrict :: (FromJSON a) => B.ByteString -> Either String a
eitherDecodeStrict =
eitherFormatError . eitherDecodeStrictWith jsonEOF ifromJSONIf we did rawRequestBody .| sinkLazy, then we'd get a Lazy.ByteString, which we could provide to eitherDecodeStrict and that would give us the JSON path error, albeit pre-formatted in the String error message. But that's what we end up doing anyway, so it may not be too bad.
But I'm also hesitant to change the implementation of a core bit of this without understanding why it is the way it is.
@snoyberg do you have any thouughts on this? tl;dr: this implementation requires an aeson >= 2.1 bound, and the eitherDecodeStrict implementation would work for our current bounds.
There was a problem hiding this comment.
I think adding a strict lower requiring aeson 2 of any kind may be a problem, I know of at least a few companies out there that haven't been able to migrate over yet. Would it be possible to keep backwards compat with CPP?
JSON error messages can optionally include the JSON path so that you get a nicer error like
Error in $.foo.bar: rest of the message. The trick is to parse into anIResultinstead of aResultsinceIResultincludes aJSONPath.This also updates the
requre*JsonBodyutilites to use these newIResult-based utilities, so anything that depends on them will automatically pick up improved error messages.Before submitting your PR, check that you've:
@sincedeclarations to the Haddocks for new, public APIsAfter submitting your PR: