Add field name to error when failing to parse a field of type Message #402
Conversation
github-vincent-baret
force-pushed
the
add_fullname_message_in_json_format_exception
branch
4 times, most recently
from
2b87551 to
1315cf6
Compare
github-vincent-baret
changed the title
github-vincent-baret
force-pushed
the
add_fullname_message_in_json_format_exception
branch
from
1315cf6 to
3c49e78
Compare
|
Hi @thesamet Do you think this PR is ok for review? |
| } catch { | ||
| case ex: JsonFormatException => | ||
| throw new JsonFormatException( | ||
| s"Failed parsing field ${fd.name}: ${ex.getMessage}", |
There was a problem hiding this comment.
With nested messages, wouldn't this lead to something that looks like:
"Failed parsing field x: failed parsing field y: failed parsing field z: reason"? Can this be changed somehow so the full path appears once?
There was a problem hiding this comment.
Hi @thesamet , sorry the person who opened that PR left my company so didn't follow up.
Indeed for this input """{"optMessage": { "optMessage": 39} }""" the error message would be
Failed parsing field opt_message: Failed parsing field opt_message: Expected an object for jsontest.MyTest, found JInt(39)
I don't know well this code base and I'm not seeing any trivial way to change it to a full path instead. I guess that might require subtyping JsonFormatException to accumulate a path for failure in nested objects, and I have no idea if adding that kind of complexity would be worth it or acceptable for the project.
Current implementation may not be the cleanest message, but it's simple and at least it's giving a helpful message, while without this implementation the message would be instead
Expected an object for jsontest.MyTest, found JInt(39)
Leaving the user clueless about where this is happening, especially for another type of error like "expected string and got number".
We are missing a way to understand what is failing when converting json to proto. This PR updates the error with the proto field name for the field that cannot be parsed, and the descriptor full name of the message type that is expected.