From 4eed2ba8c631e181e34c7d555bb6d359b2690006 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst Date: Mon, 26 Feb 2018 21:52:41 +0000 Subject: [PATCH 1/2] Use the status Message when possible in errors --- runtime/handler.go | 13 +++++-------- runtime/handler_test.go | 41 ++++++++++++++++++++++++++++++++--------- 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/runtime/handler.go b/runtime/handler.go index 1770b85344d..ff71be75865 100644 --- a/runtime/handler.go +++ b/runtime/handler.go @@ -42,7 +42,7 @@ func ForwardResponseStream(ctx context.Context, mux *ServeMux, marshaler Marshal if d, ok := marshaler.(Delimited); ok { delimiter = d.Delimiter() } else { - delimiter = []byte("\n") + delimiter = []byte("\n") } var wroteHeader bool @@ -168,16 +168,13 @@ func handleForwardResponseStreamError(wroteHeader bool, marshaler Marshaler, w h func streamChunk(result proto.Message, err error) map[string]proto.Message { if err != nil { - grpcCode := codes.Unknown - if s, ok := status.FromError(err); ok { - grpcCode = s.Code() - } - httpCode := HTTPStatusFromCode(grpcCode) + s, _ := status.FromError(err) + httpCode := HTTPStatusFromCode(s.Code()) return map[string]proto.Message{ "error": &internal.StreamError{ - GrpcCode: int32(grpcCode), + GrpcCode: int32(s.Code()), HttpCode: int32(httpCode), - Message: err.Error(), + Message: s.Message(), HttpStatus: http.StatusText(httpCode), }, } diff --git a/runtime/handler_test.go b/runtime/handler_test.go index 493aa5e5495..8709aa5e111 100644 --- a/runtime/handler_test.go +++ b/runtime/handler_test.go @@ -10,9 +10,11 @@ import ( "github.com/golang/protobuf/proto" pb "github.com/grpc-ecosystem/grpc-gateway/examples/examplepb" "github.com/grpc-ecosystem/grpc-gateway/runtime" + "github.com/grpc-ecosystem/grpc-gateway/runtime/internal" "golang.org/x/net/context" "google.golang.org/grpc" "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) func TestForwardResponseStream(t *testing.T) { @@ -84,9 +86,31 @@ func TestForwardResponseStream(t *testing.T) { w.Body.Close() var want []byte - for _, msg := range tt.msgs { + for i, msg := range tt.msgs { if msg.err != nil { - t.Skip("checking erorr encodings") + if i == 0 { + // Skip non-stream errors + t.Skip("checking error encodings") + } + st, _ := status.FromError(msg.err) + httpCode := runtime.HTTPStatusFromCode(st.Code()) + b, err := marshaler.Marshal(map[string]proto.Message{ + "error": &internal.StreamError{ + GrpcCode: int32(st.Code()), + HttpCode: int32(httpCode), + Message: st.Message(), + HttpStatus: http.StatusText(httpCode), + }, + }) + if err != nil { + t.Errorf("marshaler.Marshal() failed %v", err) + } + errBytes := body[len(want):] + if string(errBytes) != string(b) { + t.Errorf("ForwardResponseStream() = \"%s\" want \"%s\"", errBytes, b) + } + + return } b, err := marshaler.Marshal(map[string]proto.Message{"result": msg.pb}) if err != nil { @@ -103,17 +127,16 @@ func TestForwardResponseStream(t *testing.T) { } } - // A custom marshaler implementation, that doesn't implement the delimited interface type CustomMarshaler struct { - m *runtime.JSONPb + m *runtime.JSONPb } -func (c *CustomMarshaler) Marshal(v interface{}) ([]byte, error) { return c.m.Marshal(v) } -func (c *CustomMarshaler) Unmarshal(data []byte, v interface{}) error { return c.m.Unmarshal(data, v) } -func (c *CustomMarshaler) NewDecoder(r io.Reader) runtime.Decoder { return c.m.NewDecoder(r) } -func (c *CustomMarshaler) NewEncoder(w io.Writer) runtime.Encoder { return c.m.NewEncoder(w) } -func (c *CustomMarshaler) ContentType() string { return c.m.ContentType() } +func (c *CustomMarshaler) Marshal(v interface{}) ([]byte, error) { return c.m.Marshal(v) } +func (c *CustomMarshaler) Unmarshal(data []byte, v interface{}) error { return c.m.Unmarshal(data, v) } +func (c *CustomMarshaler) NewDecoder(r io.Reader) runtime.Decoder { return c.m.NewDecoder(r) } +func (c *CustomMarshaler) NewEncoder(w io.Writer) runtime.Encoder { return c.m.NewEncoder(w) } +func (c *CustomMarshaler) ContentType() string { return c.m.ContentType() } func TestForwardResponseStreamCustomMarshaler(t *testing.T) { type msg struct { From 16c3ceec4d1745c6127026c05167d04a748ef613 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst Date: Tue, 27 Feb 2018 12:51:49 +0000 Subject: [PATCH 2/2] Add details to stream error response. --- Makefile | 6 ++-- runtime/handler.go | 17 ++++++++--- runtime/handler_test.go | 1 + runtime/internal/stream_chunk.pb.go | 46 ++++++++++++++++++----------- runtime/internal/stream_chunk.proto | 3 ++ 5 files changed, 50 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index c6a645e0ef7..a4dc3e36c62 100644 --- a/Makefile +++ b/Makefile @@ -5,7 +5,9 @@ PKG=github.com/grpc-ecosystem/grpc-gateway GO_PLUGIN=bin/protoc-gen-go -GO_PLUGIN_PKG=github.com/golang/protobuf/protoc-gen-go +GO_PROTOBUF_REPO=github.com/golang/protobuf +GO_PLUGIN_PKG=$(GO_PROTOBUF_REPO)/protoc-gen-go +GO_PTYPES_ANY_PKG=$(GO_PROTOBUF_REPO)/ptypes/any SWAGGER_PLUGIN=bin/protoc-gen-swagger SWAGGER_PLUGIN_SRC= utilities/doc.go \ utilities/pattern.go \ @@ -91,7 +93,7 @@ $(GO_PLUGIN): go build -o $@ $(GO_PLUGIN_PKG) $(RUNTIME_GO): $(RUNTIME_PROTO) $(GO_PLUGIN) - protoc -I $(PROTOC_INC_PATH) --plugin=$(GO_PLUGIN) -I. --go_out=$(PKGMAP):. $(RUNTIME_PROTO) + protoc -I $(PROTOC_INC_PATH) --plugin=$(GO_PLUGIN) -I $(GOPATH)/src/$(GO_PTYPES_ANY_PKG) -I. --go_out=$(PKGMAP):. $(RUNTIME_PROTO) $(OPENAPIV2_GO): $(OPENAPIV2_PROTO) $(GO_PLUGIN) protoc -I $(PROTOC_INC_PATH) --plugin=$(GO_PLUGIN) -I. --go_out=$(PKGMAP):$(GOPATH)/src $(OPENAPIV2_PROTO) diff --git a/runtime/handler.go b/runtime/handler.go index ff71be75865..50e9c88ca2e 100644 --- a/runtime/handler.go +++ b/runtime/handler.go @@ -7,6 +7,7 @@ import ( "net/textproto" "github.com/golang/protobuf/proto" + "github.com/golang/protobuf/ptypes/any" "github.com/grpc-ecosystem/grpc-gateway/runtime/internal" "golang.org/x/net/context" "google.golang.org/grpc/codes" @@ -168,14 +169,22 @@ func handleForwardResponseStreamError(wroteHeader bool, marshaler Marshaler, w h func streamChunk(result proto.Message, err error) map[string]proto.Message { if err != nil { - s, _ := status.FromError(err) - httpCode := HTTPStatusFromCode(s.Code()) + grpcCode := codes.Unknown + grpcMessage := err.Error() + var grpcDetails []*any.Any + if s, ok := status.FromError(err); ok { + grpcCode = s.Code() + grpcMessage = s.Message() + grpcDetails = s.Proto().GetDetails() + } + httpCode := HTTPStatusFromCode(grpcCode) return map[string]proto.Message{ "error": &internal.StreamError{ - GrpcCode: int32(s.Code()), + GrpcCode: int32(grpcCode), HttpCode: int32(httpCode), - Message: s.Message(), + Message: grpcMessage, HttpStatus: http.StatusText(httpCode), + Details: grpcDetails, }, } } diff --git a/runtime/handler_test.go b/runtime/handler_test.go index 8709aa5e111..59838f2b04f 100644 --- a/runtime/handler_test.go +++ b/runtime/handler_test.go @@ -100,6 +100,7 @@ func TestForwardResponseStream(t *testing.T) { HttpCode: int32(httpCode), Message: st.Message(), HttpStatus: http.StatusText(httpCode), + Details: st.Proto().GetDetails(), }, }) if err != nil { diff --git a/runtime/internal/stream_chunk.pb.go b/runtime/internal/stream_chunk.pb.go index 44550f393b4..82af3a616be 100644 --- a/runtime/internal/stream_chunk.pb.go +++ b/runtime/internal/stream_chunk.pb.go @@ -15,6 +15,7 @@ package internal import proto "github.com/golang/protobuf/proto" import fmt "fmt" import math "math" +import google_protobuf "github.com/golang/protobuf/ptypes/any" // Reference imports to suppress errors if they are not otherwise used. var _ = proto.Marshal @@ -30,10 +31,11 @@ const _ = proto.ProtoPackageIsVersion2 // please upgrade the proto package // StreamError is a response type which is returned when // streaming rpc returns an error. type StreamError struct { - GrpcCode int32 `protobuf:"varint,1,opt,name=grpc_code,json=grpcCode" json:"grpc_code,omitempty"` - HttpCode int32 `protobuf:"varint,2,opt,name=http_code,json=httpCode" json:"http_code,omitempty"` - Message string `protobuf:"bytes,3,opt,name=message" json:"message,omitempty"` - HttpStatus string `protobuf:"bytes,4,opt,name=http_status,json=httpStatus" json:"http_status,omitempty"` + GrpcCode int32 `protobuf:"varint,1,opt,name=grpc_code,json=grpcCode" json:"grpc_code,omitempty"` + HttpCode int32 `protobuf:"varint,2,opt,name=http_code,json=httpCode" json:"http_code,omitempty"` + Message string `protobuf:"bytes,3,opt,name=message" json:"message,omitempty"` + HttpStatus string `protobuf:"bytes,4,opt,name=http_status,json=httpStatus" json:"http_status,omitempty"` + Details []*google_protobuf.Any `protobuf:"bytes,5,rep,name=details" json:"details,omitempty"` } func (m *StreamError) Reset() { *m = StreamError{} } @@ -69,6 +71,13 @@ func (m *StreamError) GetHttpStatus() string { return "" } +func (m *StreamError) GetDetails() []*google_protobuf.Any { + if m != nil { + return m.Details + } + return nil +} + func init() { proto.RegisterType((*StreamError)(nil), "grpc.gateway.runtime.StreamError") } @@ -76,17 +85,20 @@ func init() { func init() { proto.RegisterFile("runtime/internal/stream_chunk.proto", fileDescriptor0) } var fileDescriptor0 = []byte{ - // 181 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x34, 0x8e, 0xbf, 0xee, 0x82, 0x30, - 0x14, 0x85, 0xd3, 0xdf, 0x1f, 0x85, 0xcb, 0x46, 0x1c, 0x9a, 0x38, 0x48, 0x74, 0x61, 0x82, 0xc1, - 0x37, 0xd0, 0xf8, 0x02, 0xb0, 0xb9, 0x90, 0x0a, 0x37, 0x40, 0x94, 0x96, 0xdc, 0x5e, 0x62, 0x5c, - 0x7d, 0x72, 0xd3, 0x22, 0xe3, 0xf9, 0xbe, 0x73, 0x92, 0x03, 0x07, 0x9a, 0x34, 0xf7, 0x03, 0xe6, - 0xbd, 0x66, 0x24, 0xad, 0x1e, 0xb9, 0x65, 0x42, 0x35, 0x54, 0x75, 0x37, 0xe9, 0x7b, 0x36, 0x92, - 0x61, 0x13, 0x6f, 0x5a, 0x1a, 0xeb, 0xac, 0x55, 0x8c, 0x4f, 0xf5, 0xca, 0xbe, 0x8b, 0xfd, 0x5b, - 0x40, 0x54, 0xfa, 0xf2, 0x85, 0xc8, 0x50, 0xbc, 0x85, 0xd0, 0xf5, 0xaa, 0xda, 0x34, 0x28, 0x45, - 0x22, 0xd2, 0xff, 0x22, 0x70, 0xe0, 0x6c, 0x1a, 0x74, 0xb2, 0x63, 0x1e, 0x67, 0xf9, 0x33, 0x4b, - 0x07, 0xbc, 0x94, 0xb0, 0x1e, 0xd0, 0x5a, 0xd5, 0xa2, 0xfc, 0x4d, 0x44, 0x1a, 0x16, 0x4b, 0x8c, - 0x77, 0x10, 0xf9, 0x99, 0x65, 0xc5, 0x93, 0x95, 0x7f, 0xde, 0x82, 0x43, 0xa5, 0x27, 0x27, 0xb8, - 0x06, 0xcb, 0xf3, 0xdb, 0xca, 0xbf, 0x3d, 0x7e, 0x02, 0x00, 0x00, 0xff, 0xff, 0xa9, 0x07, 0x92, - 0xb6, 0xd4, 0x00, 0x00, 0x00, + // 226 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x34, 0x90, 0xc1, 0x4e, 0x84, 0x30, + 0x10, 0x86, 0x83, 0xeb, 0xba, 0xbb, 0xc3, 0x8d, 0xec, 0xa1, 0xea, 0x41, 0xa2, 0x17, 0x4e, 0x25, + 0xd1, 0x27, 0x50, 0xe3, 0x0b, 0xb0, 0x37, 0x2f, 0x9b, 0x59, 0x18, 0x0b, 0x11, 0x5a, 0x32, 0x1d, + 0x62, 0x78, 0x2d, 0x9f, 0xd0, 0xb4, 0xc8, 0xb1, 0xdf, 0xf7, 0xff, 0x93, 0x3f, 0x85, 0x27, 0x9e, + 0xac, 0x74, 0x03, 0x95, 0x9d, 0x15, 0x62, 0x8b, 0x7d, 0xe9, 0x85, 0x09, 0x87, 0x73, 0xdd, 0x4e, + 0xf6, 0x5b, 0x8f, 0xec, 0xc4, 0x65, 0x47, 0xc3, 0x63, 0xad, 0x0d, 0x0a, 0xfd, 0xe0, 0xac, 0xff, + 0x1b, 0x77, 0xb7, 0xc6, 0x39, 0xd3, 0x53, 0x19, 0x33, 0x97, 0xe9, 0xab, 0x44, 0x3b, 0x2f, 0x85, + 0xc7, 0xdf, 0x04, 0xd2, 0x53, 0xbc, 0xf3, 0xc1, 0xec, 0x38, 0xbb, 0x87, 0x43, 0x38, 0x71, 0xae, + 0x5d, 0x43, 0x2a, 0xc9, 0x93, 0x62, 0x5b, 0xed, 0x03, 0x78, 0x77, 0x0d, 0x05, 0xd9, 0x8a, 0x8c, + 0x8b, 0xbc, 0x5a, 0x64, 0x00, 0x51, 0x2a, 0xd8, 0x0d, 0xe4, 0x3d, 0x1a, 0x52, 0x9b, 0x3c, 0x29, + 0x0e, 0xd5, 0xfa, 0xcc, 0x1e, 0x20, 0x8d, 0x35, 0x2f, 0x28, 0x93, 0x57, 0xd7, 0xd1, 0x42, 0x40, + 0xa7, 0x48, 0x32, 0x0d, 0xbb, 0x86, 0x04, 0xbb, 0xde, 0xab, 0x6d, 0xbe, 0x29, 0xd2, 0xe7, 0xa3, + 0x5e, 0x16, 0xeb, 0x75, 0xb1, 0x7e, 0xb5, 0x73, 0xb5, 0x86, 0xde, 0xe0, 0x73, 0xbf, 0x7e, 0xc2, + 0xe5, 0x26, 0x46, 0x5e, 0xfe, 0x02, 0x00, 0x00, 0xff, 0xff, 0x16, 0x75, 0x92, 0x08, 0x1f, 0x01, + 0x00, 0x00, } diff --git a/runtime/internal/stream_chunk.proto b/runtime/internal/stream_chunk.proto index f7fba56c35b..55f42ce63ec 100644 --- a/runtime/internal/stream_chunk.proto +++ b/runtime/internal/stream_chunk.proto @@ -2,6 +2,8 @@ syntax = "proto3"; package grpc.gateway.runtime; option go_package = "internal"; +import "google/protobuf/any.proto"; + // StreamError is a response type which is returned when // streaming rpc returns an error. message StreamError { @@ -9,4 +11,5 @@ message StreamError { int32 http_code = 2; string message = 3; string http_status = 4; + repeated google.protobuf.Any details = 5; }