{"action":"created","issue":{"url":"https://api.github.com/repos/golang/protobuf/issues/266","repository_url":"https://api.github.com/repos/golang/protobuf","labels_url":"https://api.github.com/repos/golang/protobuf/issues/266/labels{/name}","comments_url":"https://api.github.com/repos/golang/protobuf/issues/266/comments","events_url":"https://api.github.com/repos/golang/protobuf/issues/266/events","html_url":"https://github.com/golang/protobuf/issues/266","id":194001384,"number":266,"title":"jsonpb error reporting is poor, and here's how to fix it","user":{"login":"mwitkow","id":9655971,"avatar_url":"https://avatars2.githubusercontent.com/u/9655971?v=3","gravatar_id":"","url":"https://api.github.com/users/mwitkow","html_url":"https://github.com/mwitkow","followers_url":"https://api.github.com/users/mwitkow/followers","following_url":"https://api.github.com/users/mwitkow/following{/other_user}","gists_url":"https://api.github.com/users/mwitkow/gists{/gist_id}","starred_url":"https://api.github.com/users/mwitkow/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mwitkow/subscriptions","organizations_url":"https://api.github.com/users/mwitkow/orgs","repos_url":"https://api.github.com/users/mwitkow/repos","events_url":"https://api.github.com/users/mwitkow/events{/privacy}","received_events_url":"https://api.github.com/users/mwitkow/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":5,"created_at":"2016-12-07T09:42:31Z","updated_at":"2017-03-03T21:12:57Z","closed_at":null,"body":"JsonPB has very unhelpful error reporting. For example:\r\n\r\n```json: cannot unmarshal string into Go value of type []json.RawMessage```\r\n\r\nIts unclear what field this is about (and for multi nested messages it is a huge), and even the type `[]json.RawMessage` is incorrect (`[uint32`]). I forked the `jsonpb.Unmarshaller` at [go-nicejsonpb](https://github.com/mwitkow/go-nicejsonpb) and introduced few improvements:\r\n * Errors are are now prefixed with a \"stack\" path of fields that are returned in (including offsets in arrays, and keys in maps)\r\n * Poor \"cannot deserialize into `json.RawMessage\" errors now use proper types\r\n * Unknown fields now return a helpful message listing known fields :)\r\n\r\nFor a comparison of outputs, here's a test against normal `jsonpb` with the expected values being from `nicejsonpb`\r\n\r\n```\r\nnew: \"unparsable field SomeIntRep.[3]: json: cannot unmarshal string into Go value of type uint32\" \r\nold: \"json: cannot unmarshal string into Go value of type uint32\" \r\n\t\r\nnew: \"unparsable field SomeEmbedded.SomeValue: invalid character 's' looking for beginning of value while looking for an integer in a string\"\r\nold: \"invalid character 's' looking for beginning of value\"\r\n\r\nnew: \"unparsable field SomeEmbedded.Identifier: json: cannot unmarshal number into Go value of type string\" (expected)\r\nold: \"json: cannot unmarshal number into Go value of type string\"\r\n\r\nnew: \"unparsable field SomeIntRep: json: cannot unmarshal string into Go value of type []uint32\"\r\nold: \"json: cannot unmarshal string into Go value of type []json.RawMessage\"\r\n\t\r\nnew: \"unparsable field SomeEmbedded: fields [someUnknown anotherUnknown] do not exist in set of known fields [identifier someValue]\"\r\nold: \"unknown field \\\"someUnknown\\\" in validatortest.ValidatorMessage3_Embedded\"\r\n```\r\n\r\nI'd happily upstream it, but would like to know whether introducing a new `jsonpb.FieldError` is a viable option for upstream. @bcmills, thoughts?"},"comment":{"url":"https://api.github.com/repos/golang/protobuf/issues/comments/284071504","html_url":"https://github.com/golang/protobuf/issues/266#issuecomment-284071504","issue_url":"https://api.github.com/repos/golang/protobuf/issues/266","id":284071504,"user":{"login":"mwitkow","id":9655971,"avatar_url":"https://avatars2.githubusercontent.com/u/9655971?v=3","gravatar_id":"","url":"https://api.github.com/users/mwitkow","html_url":"https://github.com/mwitkow","followers_url":"https://api.github.com/users/mwitkow/followers","following_url":"https://api.github.com/users/mwitkow/following{/other_user}","gists_url":"https://api.github.com/users/mwitkow/gists{/gist_id}","starred_url":"https://api.github.com/users/mwitkow/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mwitkow/subscriptions","organizations_url":"https://api.github.com/users/mwitkow/orgs","repos_url":"https://api.github.com/users/mwitkow/repos","events_url":"https://api.github.com/users/mwitkow/events{/privacy}","received_events_url":"https://api.github.com/users/mwitkow/received_events","type":"User","site_admin":false},"created_at":"2017-03-03T21:12:57Z","updated_at":"2017-03-03T21:12:57Z","body":"Oh, of course. The field path construction can be moved to the error itself\nand avoid allocation. We use nicejsonpb to validate config files so there\nwas no need for performance considerations.\n\nI'll refactor the change into a proper PR once I'm back from vacation.\n\nOn Fri, 3 Mar 2017, 20:42 Bryan C. Mills, <notifications@github.com> wrote:\n\n> It's a bit hard to tell what the proposed changes are because the commit\n> history of go-nicejsonpb seems to be truncated, but I see a lot of\n> fmt.Sprintf calls in the error construction path which seem like they'll\n> add a lot of unnecessary allocations. Perhaps there's a more efficient way\n> to structure the errors to give a better balance of overhead and clarity?\n>\n> —\n> You are receiving this because you authored the thread.\n> Reply to this email directly, view it on GitHub\n> <https://github.com/golang/protobuf/issues/266#issuecomment-284064763>,\n> or mute the thread\n> <https://github.com/notifications/unsubscribe-auth/AJNWo_Ku2ViH7O1p5uqXagve85_wZkBBks5riHsFgaJpZM4LGYkX>\n> .\n>\n"}}
{"actor":{"display_login":"mwitkow"}}