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: mapEncoder.encode should not use MarshalerError #29753

Closed
ianlancetaylor opened this issue Jan 15, 2019 · 4 comments
Closed

encoding/json: mapEncoder.encode should not use MarshalerError #29753

ianlancetaylor opened this issue Jan 15, 2019 · 4 comments
Labels
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Jan 15, 2019

As of CL 20356, if there is an error in encoding/json.mapEncoder.encode, it returns an error of type MarshalerError. But the message printed by MarshalerError's Error method refers to MarshalJSON, and that method was never called. Every other use of MarshalerError is in the context of a call to MarshalJSON or MarshalText. I think that mapEncoder.encode should use a different error type; I note that stringEncoder simply calls fmt.Errorf.

Also we should probably change MarshalerError's message to refer to MarshalText as well as MarshalJSON.

CC @augustoroman

@ianlancetaylor ianlancetaylor added this to the Go1.13 milestone Jan 15, 2019
@augustoroman

This comment has been minimized.

Copy link
Contributor

@augustoroman augustoroman commented Jan 15, 2019

Would it violate the compatibility guarantee to change the error type from MarshalerError to a fmt.Errorf? Perhaps only updating the MarshalerError's message would be sufficient?

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor Author

@ianlancetaylor ianlancetaylor commented Jan 15, 2019

This is permitted by the Go 1 compatibility guarantee. We shouldn't change it if we think that people are relying the returned type, but that doesn't seem particularly likely to me.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 28, 2019

Change https://golang.org/cl/184119 mentions this issue: encoding/json: correct caller's name in MarshalerError

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jun 28, 2019

Change https://golang.org/cl/184120 mentions this issue: encoding/json: mapEncoder.encode does not use MarshalerError

@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gopherbot gopherbot closed this in 02196d3 Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.