Revisit prefixing error messages with connect code

[mfridman]

Worth opening an issue as this came up again. There is an argument to be made that Connect should be returning user-supplied error messages as-is without adding the code string:

https://github.com/bufbuild/connect-go/blob/main/error.go#L80

If a backend chooses to surface an error, as-is, then it may cause a stutter (this may happen in our code). E.g.,

Before

Failure: bufbuild.internal/no-org-user/no-org-user-repo-private: does not exist

After

Failure: NotFound: "buf.build/no-org-user/no-org-user-repo-private": does not exist

Although I personally like this, this might be too descriptive for other codebases and add more migration effort. This would also mean we might have to audit our codebase and account for this.

cc @amckinney @robbertvanginkel

[akshayjshah]

@bufdev probably has an opinion here too :)

I'm okay with removing the prefix if it's causing more problems than it solves. I'm a little worried that keeping the original error string as-is hides the most important information from logs, debug prints, etc., though. I guess the code can often be deduced from the error string, though?

[bufdev]

Just for reference, what does prior art look like (grpc-go, twirp)?

[mfridman]

With gRPC we were able to retrieve the user-supplied error message, without the prefix.

Twirp was the worst, calling Error() returned both twirp prefix, code and message:

func (e *twerr) Error() string {
    return fmt.Sprintf("twirp error %s: %s", e.code, e.msg)
}

They did however expose .Msg() .. "a human-readable, unstructured messages describing the error" and .Code()

https://github.com/twitchtv/twirp/blob/main/errors.go#L351-L353

The JSON output for clients returned both fields separately..

{
  "code": "internal",
  "msg": "Something went wrong"
}

[mfridman]

More discussion in https://github.com/bufbuild/connect-go/pull/186. We decided to keep the existing error message format, and also expose a .Message() helper to make it easier for users to get at the underlying error.

The formatted output of .Error() is inline with other RPC frameworks, whereby both rpc code + message are captured.