Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encoding/json: include property name for marshal errors #25153

Open
dominicbarnes opened this issue Apr 29, 2018 · 3 comments
Open

encoding/json: include property name for marshal errors #25153

dominicbarnes opened this issue Apr 29, 2018 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@dominicbarnes
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.10 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/dominic/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/dominic/dev"
GORACE=""
GOROOT="/usr/lib/go-1.10"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go-1.10/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build203824871=/tmp/go-build -gno-record-gcc-switches"

What did you do?

When encountering an error using json.Marshal() (and related functions) the error returned gives no indication where the error was encountered. The only included metadata is about the type of the value that was being encoded, but that is usually insufficient to narrow down problems without the original input value. (which may not always be at hand)

What is being proposed?

The proposal would be to add the property name for marshal errors (using dot notation to signify hierarchy). For example:

json: error calling MarshalJSON for type time.Time: Time.MarshalJSON: year outside of range [0,9999]

would be updated to be more specific:

json: error calling MarshalJSON for timestamp (type time.Time): Time.MarshalJSON: year outside of range [0,9999]

and in the case of a more complex/nested data structure:

json: error calling MarshalJSON for events.[0].timestamp (type time.Time): Time.MarshalJSON: year outside of range [0,9999]

Why is this useful?

I use Go to do a lot of data processing, which is generally from 3rd-parties (aka: customers) so the structure of data is not usually known ahead of time, not to mention weird values being sent due to programming errors and a host of other problems.

Last week, I encountered the first error message above and needed to do some extra work just to determine which property was causing the error, when the encoder should be able to communicate that to the caller.

@dominicbarnes
Copy link
Author

I took this opportunity to dig into the go stdlib and I wanted this to be among my first contributions to the project. I have changes ready in a git branch that I can open a PR with, and I am definitely open to feedback on this proposal ahead of that step.

@mvdan
Copy link
Member

mvdan commented Apr 29, 2018

@dominicbarnes feel free to submit a patch, either on Gerrit or on GitHub. Even if the suggestion ends up being rejected, there's no harm in posting it.

@gopherbot
Copy link

Change https://golang.org/cl/110119 mentions this issue: encoding/json: include property name for marshal errors

@andybons andybons added this to the Unplanned milestone Apr 30, 2018
@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants