golang / protobuf

Go support for Google's protocol buffers
BSD 3-Clause "New" or "Revised" License
9.74k stars 1.58k forks source link

encoding/protojson: provide error details #1534

Closed xxgreg closed 1 year ago

xxgreg commented 1 year ago

Is your feature request related to a problem? Please describe.

I'd like to return clear error messages to API users, this involves programatically determining the line number, column, and kind of error. I.e. "unknown field" vs invalid JSON due to a missing curly etc.

Currently protojson.Unmarshal(...) returns a go Error type with a string such as:

proto: (line 5:3): unknown field "nope"

Currently the only way to find the line number and the type of error is to parse the error message string. This message probably isn't guaranteed to stay the same across releases.

Describe the solution you'd like

Consider providing an API to provide this information to error handling code. One approach is to return a specific error type with fields describing the error. But I'm open to any API suggestions.

An ideal solution would also be able to recover from some kinds of errors and return multiple errors.

For example if a message has multiple unknown fields, an error would be returned for both, rather than stopping at the first error.

hoeppi-google commented 1 year ago

Hi, thanks for your feature request. This is one of many errors in decode.go. I assume you are suggesting to find a general solution to all of them. Could you please clarify and provide more information about use cases for an extended error API? What would really help here would be to have a concrete API proposal. To justify the maintenance burden, we would need to have convincing data that this would benefit a larger user base.

xxgreg commented 1 year ago

I'm using protojson to read and write JSON which are sometimes hand edited, or generated with from code with non-protobuf tool chains.

I'm also using JSON over HTTP APIs where the source of the message is not always built from a protobuf library.

I would like to provide clear error messages to users who choose to do this.

An example of a similar API is UnmarshalTypeError in the encoding/json library.

Understand the maintenance concerns. This feature is likely better experimented with in a fork of jsonpb.Unmarshal.

I thought it would be a good idea to raise this here to see if there is wider interest, or any previous discussion. Happy for this to be closed. I'm not planning on writing a detailed formal proposal.

stapelberg commented 1 year ago

I’m not aware of previous discussion, but note that the current team of maintainers only took over the library last year, so we don’t have all the state either…

The request for structured error types sounds reasonable, but we don’t have cycles for such work right now.

If you, or someone else, would like to send a change for review (via gerrit), we can take a look.

xxgreg commented 1 year ago

Fair enough. I'll close this issue for now, and report back once I get to this.