diff --git a/client_duplex_stream.go b/client_duplex_stream.go index e73bd883..557e4301 100644 --- a/client_duplex_stream.go +++ b/client_duplex_stream.go @@ -323,13 +323,15 @@ func extractError(protobuf codec.Codec, h http.Header) *Error { if len(detailsBinaryEncoded) > 0 { detailsBinary, err := DecodeBinaryHeader(detailsBinaryEncoded) if err != nil { - return Errorf(CodeUnknown, "server returned invalid grpc-error-details-bin trailer: %w", err) + return Errorf(CodeUnknown, "server returned invalid grpc-status-details-bin trailer: %w", err) } var status statuspb.Status if err := protobuf.Unmarshal(detailsBinary, &status); err != nil { return Errorf(CodeUnknown, "server returned invalid protobuf for error details: %w", err) } - ret.details = status.Details + for _, d := range status.Details { + ret.details = append(ret.details, d) + } // Prefer the protobuf-encoded data to the headers (grpc-go does this too). ret.code = Code(status.Code) ret.err = errors.New(status.Message) diff --git a/error.go b/error.go index e77027b1..66679a01 100644 --- a/error.go +++ b/error.go @@ -5,9 +5,25 @@ import ( "fmt" "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/types/known/anypb" + "google.golang.org/protobuf/reflect/protoreflect" ) +// An ErrorDetail is a protobuf message attached to an *Error. Error details +// are sent over the network to clients, which can then work with +// strongly-typed data rather than trying to parse a complex error message. +// +// The ErrorDetail interface is implemented by protobuf's Any type, provided in +// Go by the google.golang.org/protobuf/types/known/anypb package. The +// google.golang.org/genproto/googleapis/rpc/errdetails package contains a +// variety of protobuf messages commonly used as error details. +type ErrorDetail interface { + proto.Message + + MessageIs(proto.Message) bool + MessageName() protoreflect.FullName + UnmarshalTo(proto.Message) error +} + // An Error captures three pieces of information: a Code, a human-readable // message, and an optional collection of arbitrary protobuf messages called // "details" (more on those below). Servers send the code, message, and details @@ -34,7 +50,7 @@ import ( type Error struct { code Code err error - details []*anypb.Any + details []ErrorDetail } // Wrap annotates any error with a status code. If the code is CodeOK, the @@ -60,7 +76,7 @@ func AsError(err error) (*Error, bool) { } func (e *Error) Error() string { - text := fmt.Sprintf("%v", e.err) + text := e.err.Error() if text == "" { return e.code.String() } @@ -81,41 +97,14 @@ func (e *Error) Code() Code { return e.code } -// Details returns a deep copy of the error's details. -func (e *Error) Details() []*anypb.Any { - if len(e.details) == 0 { - return nil - } - ds := make([]*anypb.Any, len(e.details)) - for i, d := range e.details { - ds[i] = proto.Clone(d).(*anypb.Any) - } - return ds +// Details returns the error's details. +func (e *Error) Details() []ErrorDetail { + return e.details } // AddDetail appends a message to the error's details. -func (e *Error) AddDetail(m proto.Message) error { - if d, ok := m.(*anypb.Any); ok { - e.details = append(e.details, proto.Clone(d).(*anypb.Any)) - return nil - } - detail, err := anypb.New(m) - if err != nil { - return fmt.Errorf("can't add message to error details: %w", err) - } - e.details = append(e.details, detail) - return nil -} - -// SetDetails overwrites the error's details. -func (e *Error) SetDetails(details ...proto.Message) error { - e.details = make([]*anypb.Any, 0, len(details)) - for _, d := range details { - if err := e.AddDetail(d); err != nil { - return err - } - } - return nil +func (e *Error) AddDetail(d ErrorDetail) { + e.details = append(e.details, d) } // CodeOf returns the error's status code if it is or wraps a *rerpc.Error, diff --git a/error_test.go b/error_test.go index 720b48f5..eedc5176 100644 --- a/error_test.go +++ b/error_test.go @@ -46,8 +46,6 @@ func TestErrorDetails(t *testing.T) { assert.Nil(t, err, "create anypb.Any") rerr := Errorf(CodeUnknown, "details") assert.Zero(t, rerr.Details(), "fresh error") - assert.Nil(t, rerr.AddDetail(second), "add detail") - assert.Equal(t, rerr.Details(), []*anypb.Any{detail}, "retrieve details") - assert.Nil(t, rerr.SetDetails(second, second), "overwrite details") - assert.Equal(t, rerr.Details(), []*anypb.Any{detail, detail}, "retrieve details") + rerr.AddDetail(detail) + assert.Equal(t, rerr.Details(), []ErrorDetail{detail}, "retrieve details") } diff --git a/handler_stream.go b/handler_stream.go index 553d1095..3e551433 100644 --- a/handler_stream.go +++ b/handler_stream.go @@ -6,6 +6,7 @@ import ( "strconv" "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/known/anypb" "github.com/rerpc/rerpc/codec" "github.com/rerpc/rerpc/compress" @@ -91,7 +92,10 @@ func (hs *handlerSender) sendErrorGRPC(err error) error { hs.writer.Header().Set("Grpc-Status-Details-Bin", "") return nil } - s := statusFromError(err) + s, statusError := statusFromError(err) + if statusError != nil { + return statusError + } code := strconv.Itoa(int(s.Code)) bin, err := hs.protobuf.Marshal(s) if err != nil { @@ -151,19 +155,37 @@ func (hr *handlerReceiver) Header() http.Header { return hr.request.Header } -func statusFromError(err error) *statuspb.Status { +func statusFromError(err error) (*statuspb.Status, *Error) { s := &statuspb.Status{ Code: int32(CodeUnknown), Message: err.Error(), } if re, ok := AsError(err); ok { s.Code = int32(re.Code()) - s.Details = re.Details() + for _, d := range re.details { + // If the detail is already a protobuf Any, we're golden. + if anyProtoDetail, ok := d.(*anypb.Any); ok { + s.Details = append(s.Details, anyProtoDetail) + continue + } + // Otherwise, we convert it to an Any. + // TODO: Should we also attempt to delegate this to the detail by + // attempting an upcast to interface{ ToAny() *anypb.Any }? + anyProtoDetail, err := anypb.New(d) + if err != nil { + return nil, Errorf( + CodeInternal, + "can't create an *anypb.Any from %v (type %T): %v", + d, d, err, + ) + } + s.Details = append(s.Details, anyProtoDetail) + } if e := re.Unwrap(); e != nil { s.Message = e.Error() // don't repeat code } } - return s + return s, nil } func discard(r io.Reader) { diff --git a/internal/gen/proto/go-rerpc/rerpc/ping/v1test/ping_rerpc.pb.go b/internal/gen/proto/go-rerpc/rerpc/ping/v1test/ping_rerpc.pb.go index 261bdf42..c91b63fd 100644 --- a/internal/gen/proto/go-rerpc/rerpc/ping/v1test/ping_rerpc.pb.go +++ b/internal/gen/proto/go-rerpc/rerpc/ping/v1test/ping_rerpc.pb.go @@ -9,13 +9,12 @@ package pingv1test import ( context "context" errors "errors" - strings "strings" - rerpc "github.com/rerpc/rerpc" clientstream "github.com/rerpc/rerpc/clientstream" protobuf "github.com/rerpc/rerpc/codec/protobuf" handlerstream "github.com/rerpc/rerpc/handlerstream" v1test "github.com/rerpc/rerpc/internal/gen/proto/go/rerpc/ping/v1test" + strings "strings" ) // This is a compile-time assertion to ensure that this generated file and the