C.f. #8778 and #5161

Nice! thanks!

#8778 and #5161

My two cents:

We should not use Error() to marshal an error. I may have a custom error like:

type MyError struct {
    Code int `json:"code"`
    Message string `json:"message"`
}

func (err *MyError) Error() string {
    return fmt.Sprintf("[%d] %s", err.Code, err.Message")
}

If I JSON marshal it, nobody would expect:

error: "[2] this is a error"

instead:

error: {"code": 2, "message": "this is an error"}

We have to fix errorString, the error interface is just that, a interface. No Magic.

We don't want to let the low-level errors package import the encoding/json package. That will bring encoding/json, and all of its dependencies, into all sorts of programs that don't need it.

But it might work to implement a MarshalText method.

@ianlancetaylor that makes sense! I implemented MarshalText() for you.
https://github.com/gin-gonic/go/commit/52d7945d572c3546a7e47ad56b887cf641cd86e1

it works perfectly fine. I also reviewed the source code of json package and looks safe.

Let me introduce an use case:

The Gin Framework provides some facilities for error management during a http request. Basically it provides a tool to collect any error or several ones. Each error is defined internally as:

type errorMsg struct {
    Error error       `json:"error"`
    Type  int         `json:"-"`
    Meta  interface{} `json:"meta"`
}

Then, by using a middleware you can read the list of errors, and do whatever you want: write the errors is a log, send to sentry, write in the http response...

Simple error handler middleware:

func errorHandler(c *gin.Context) {
    c.Next()

    if len(c.Errors) > 0 {
        c.JSON(-1, c.Errors) // this does not work as expected
    }
}

As you can see, if the canonical value of the Error field is an instance of *errorString it will not work as expected since the error message is not marshalled.

Not all errors are from errors.New, what if the error is a custom error
type implemented in user code?

I know! what is the problem? I am not talking about error, I only want to fix errorString, the error created by errors.New().
https://github.com/gin-gonic/go/commit/52d7945d572c3546a7e47ad56b887cf641cd86e1

If the error is a custom error, for example:

type MyError struct {
    Code int `json:"code"`
    Message string `json:"message"`
}

func (err *MyError) Error() string {
    return fmt.Sprintf("[%d] %s", err.Code, err.Message")
}

json.Marshal(MyError{2, "message"}) // WORKS!
json.Marshal(errors.New("message")) // DOES NOT WORK! << should be fixed (only this case)

you don't really have to implement MashaslJSON() or MarshalText. The json package uses reflexion in order to generate the json string from the exported field (upper case).

What if the error is in user code that doesn't implement MashaslJSON
or MarshalText and doesn't have exported fields? Is marshaling to "{}"
acceptable?

Fixing errors.errorString this way doesn't fix the generic problem.

http://play.golang.org/p/5d08Ec2rdQ

@minux ok, I see your point now.
how about this solution, modifying the json package:

1. Try MarshalJSON(), continue if not implemented
2. Try MarshalText(), continue if not implemented
3. Try generate json with exported field, continue if empty
4. If interface is error, use Error()

NOTE: probably this should be implemented for XML, Gob... as well

I don't think we can do this. Users might already depend on their error types not being marshalled using the Error method, and such a change would break their code.

To address your specific case, why don't you make your JSON method recognise error values and handle them specially?

Design issues aside, your proposed solution introduces a circular dependency because encoding/json depends on the errors package. I do not know how you got it to work.

To be sure, I just tried it:

% go build
can't load package: import cycle not allowed
package errors
imports encoding/json
imports bytes
imports errors
%

You are better off handling this in your own code.

See also #3353

I'm not convinced that having automatic marshaling of errors is a good thing.
If something returns an error, it could be any one of many different types,
not all of which may be designed to be marshaled. Moreover there's no way that
we can provide enough information in the resulting JSON (or YAML or ...)
that the original error can be reassembled.

Implementing MarshalText on errors.errorString is never going to be that useful.
If you're marshaling an error, you're almost certainly making a mistake.

For the record, our approach to error marshaling is to have a function
that is called to explicitly transform errors to marshalable values,
for example:

func(err error) (httpStatus int, errorBody interface{})

That function can inspect the error and decide on how it should
be represented as an HTTP response (in this case), falling back
to using the error string if it doesn't fit with any of the known types.

This is not something that can be done by the individual error types themselves,
as an error may be used in many different marshaling contexts.

I agree with Roger.