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

Jump to bottom

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
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@ianlancetaylor
Copy link
Contributor

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 the NeedsFix The path to resolution is known, but the work has not been done. label Jan 15, 2019
@ianlancetaylor ianlancetaylor added this to the Go1.13 milestone Jan 15, 2019
@augustoroman
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link

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

@gopherbot
Copy link

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
@golang golang locked and limited conversation to collaborators Oct 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@rsc @andybons @augustoroman @ianlancetaylor @gopherbot