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: use the Error method to marshal an error value? #5161

Open
orofarne opened this issue Mar 30, 2013 · 12 comments
Open

encoding/json: use the Error method to marshal an error value? #5161

orofarne opened this issue Mar 30, 2013 · 12 comments
Labels
Thinking v2 An incompatible library change
Milestone

Comments

@orofarne
Copy link

What steps will reproduce the problem?
http://play.golang.org/p/epXpwk0s4s

What is the expected output?
Something like this:
http://play.golang.org/p/Dk0l6fTj01

What do you see instead?
{"bar":{},"foo":{}}

Which compiler are you using (5g, 6g, 8g, gccgo)?


Which operating system are you using?
Linux (Ubuntu 12.04, 3.2.0-38-generic)

Which version are you using?  (run 'go version')
go version go1.0.3
go version devel +5260abd6df41 Sat Mar 30 19:05:00 2013 +0800 linux/amd64

Please provide any additional information below.
I think it's not intuitive behaviour. fmt prints errors without problems, why json not?
@remyoudompheng
Copy link
Contributor

Comment 1:

See also issue #3353.

@gopherbot
Copy link
Contributor

Comment 2 by pongad:

I believe this is the expected behavior of json.Marshal(interfaceValue). According to
the documentation, if the argument is an interface value, it is encodes the value of
that interface.
The implementation of fmt.Errorf uses errors.errorString type. The type is a struct with
one unexported member 's' which holds the error. Since the member is unexported,
json.Marshal does not care to encode it.
A possible workaround is to create a new error type that both satisfies "error" and
"json.Marshaler" like 
http://play.golang.org/p/NrtoYsGzij

@adg
Copy link
Contributor

adg commented Jul 29, 2013

Comment 3:

Not sure what the right move is, here. Or even if we can fix it at all. There may be
code that relies on the JSON marhsaller *not* using the Error method to marshal a value.

Labels changed: added priority-later, removed priority-triage.

Status changed to Thinking.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2013

Comment 4:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 5:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

rsc commented Dec 4, 2013

Comment 6:

Labels changed: added repo-main.

@dimandzhi
Copy link

Hi! I hate to be the one who breaks the almost-a-decade silence, but after several years of using golang I've stumbled uppon a piece of code that uses JSON marshaling of an error interface value. Guess, it is a rare scenario. 😄 Well, result of {"err": {}} wasn't very informative. I realise, that errors interface puprpose is to provide a method to give stringified message of error (with according format for logging even), which doesn't mean it's meant to be marshaled into JSON the same way.
I think adding a configuration option (not default) to json.Encoder to enable using Error() method to encode value if there is no MarshalJSON() method will help everyone to achieve desired behavior without the need of creating a special type for that.

@micklove
Copy link

FYI - Had the same issue, expected the error string to be in the json output...
after various searches, ended up with this...
(would love to know if there's a simpler approach)

My original struct

type Result struct {
    Score   int
    Error   error
}

Implmented MarshalJSON on the Result struct

// MarshalJSON -
// create anonymous struct to generate a different representation of the Result, containing the Error() string, 
// as the error interface's string won't be marshaled by default
func (r Result) MarshalJSON() ([]byte, error) {
    var errorMsg string
    if r.Error != nil {
        errorMsg = r.Error.Error()
    }
    anon := struct {
        Score   int    `json:"score"`
        Error   string `json:"error"`
    }{
        Score:   r.Score,
        Error:   errorMsg,
    }
    return json.Marshal(anon)
}

JSON output

...
{
   "score": 25,
    "error": "did not qualify"
},
...

adrienthebo added a commit to gitpod-io/gitbot that referenced this issue Nov 3, 2022
per golang/go#5161 serializing an `error` object to json emits an empty object; this caused deployed-labeler to emit an empty object when erroring. This commit converts all errors to strings before returning to the user to provide more context.
Pothulapati pushed a commit to gitpod-io/gitbot that referenced this issue Dec 5, 2022
per golang/go#5161 serializing an `error` object to json emits an empty object; this caused deployed-labeler to emit an empty object when erroring. This commit converts all errors to strings before returning to the user to provide more context.

Signed-off-by: Tarun Pothulapati <tarun@gitpod.io>
roboquat pushed a commit to gitpod-io/gitbot that referenced this issue Dec 7, 2022
per golang/go#5161 serializing an `error` object to json emits an empty object; this caused deployed-labeler to emit an empty object when erroring. This commit converts all errors to strings before returning to the user to provide more context.

Signed-off-by: Tarun Pothulapati <tarun@gitpod.io>
@lrweck
Copy link

lrweck commented Jul 3, 2023

just stumbled on this too. I guess changing it now would be a breaking change

@mitar
Copy link
Contributor

mitar commented Aug 22, 2023

The gitlab.com/tozd/go/errors error package provides JSON marshal of errors which uses some standard interfaces to extract data from the error.

@mitar
Copy link
Contributor

mitar commented Aug 22, 2023

But I would suggest that any error marshaling in Go has at least this basic structure: {"error": "...error message"}.

@dsnet
Copy link
Member

dsnet commented Oct 6, 2023

Hi all, we kicked off a discussion for a possible "encoding/json/v2" package that addresses the spirit of this proposal.

I don't think we should provide native support for error types since the JSON representation is only unidirectional (i.e., we can marshal, but can't unmarshal). That said, with #5901, you should be able to accomplish to general gist of what's being asked for here. See this example.

marcovidonis added a commit to nimbleape/torrent that referenced this issue Jan 15, 2024
The main use for this status is to send update messages through the WS,
in the form of JSON. Marshalling a JSON field into an error object
doesn't work in Go, so it's better to send the error message as string.
See golang/go#5161
marcovidonis added a commit to nimbleape/torrent that referenced this issue Feb 9, 2024
* basic observer framework

* fleshing out the tracker status

* set up provision for onPeerConnUpdate function

* add provision for peer conn state update channel

* connects to a valid tracker

* added observer channel for announce status

* set up provision for onPeerConnUpdate function

* add provision for peer conn state update channel

* set up Peer Connection status Observers

* add missing Observer initialisation

* add PeerConn test: connection established

* add comment on torrent used for PeerConn testing

* merged PeerObserver commits, moved readChannelTimeout

* added failure case for AnnounceStatus observer

* added Observers factory method

* Added Event to AnnounceStatus, with embedded TrackerStatus

* AnnounceStatus details and error reporting

* state updates must be non-blocking

* json annotations and use matching infoHash string

* add unit tests on PeerConn Observer status reading

* refactor test by doing direct transfer locally

* add test and debug log on dropped connection

* add provision for test on PeerID

* remove unused OnPeerConnUpdate function

* change Err field in PeerStatus to string

The main use for this status is to send update messages through the WS,
in the form of JSON. Marshalling a JSON field into an error object
doesn't work in Go, so it's better to send the error message as string.
See golang/go#5161

* add PeerID check to test

* remove unused method

---------

Co-authored-by: Parker Whittle <pwhittle@medicom.us>
@seankhliao seankhliao added the v2 An incompatible library change label Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Thinking v2 An incompatible library change
Projects
None yet
Development

No branches or pull requests