From 274d5518fab1d0c2e81fa5f9e1f8135019383e48 Mon Sep 17 00:00:00 2001 From: James Protzman Date: Sun, 8 Mar 2020 17:26:44 -0400 Subject: [PATCH 1/9] Move StatusError to internal/status --- internal/status/status.go | 32 ++++++++++++++++++++++++++++++++ status/status.go | 36 +++++++----------------------------- status/status_test.go | 11 +++++------ 3 files changed, 44 insertions(+), 35 deletions(-) create mode 100644 internal/status/status.go diff --git a/internal/status/status.go b/internal/status/status.go new file mode 100644 index 00000000000..9c0f62ebd82 --- /dev/null +++ b/internal/status/status.go @@ -0,0 +1,32 @@ +package status + +import ( + "fmt" + "github.com/golang/protobuf/proto" + spb "google.golang.org/genproto/googleapis/rpc/status" + "google.golang.org/grpc/codes" +) + +// StatusError is an alias of a status proto. It implements error and Status, +// and a nil StatusError should never be returned by this package. +type StatusError spb.Status + +func (se *StatusError) Error() string { + p := (*spb.Status)(se) + return fmt.Sprintf("rpc error: code = %s desc = %s", codes.Code(p.GetCode()), p.GetMessage()) +} + +func (se *StatusError) GRPCStatus() *spb.Status { + return (*spb.Status)(se) +} + +// Is implements future error.Is functionality. +// A statusError is equivalent if the code and message are identical. +func (se *StatusError) Is(target error) bool { + tse, ok := target.(*StatusError) + if !ok { + return false + } + + return proto.Equal((*spb.Status)(se), (*spb.Status)(tse)) +} \ No newline at end of file diff --git a/status/status.go b/status/status.go index a1348e9b16b..5fdeed64c05 100644 --- a/status/status.go +++ b/status/status.go @@ -37,6 +37,7 @@ import ( spb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" "google.golang.org/grpc/internal" + "google.golang.org/grpc/internal/status" ) func init() { @@ -45,30 +46,6 @@ func init() { func statusRawProto(s *Status) *spb.Status { return s.s } -// statusError is an alias of a status proto. It implements error and Status, -// and a nil statusError should never be returned by this package. -type statusError spb.Status - -func (se *statusError) Error() string { - p := (*spb.Status)(se) - return fmt.Sprintf("rpc error: code = %s desc = %s", codes.Code(p.GetCode()), p.GetMessage()) -} - -func (se *statusError) GRPCStatus() *Status { - return &Status{s: (*spb.Status)(se)} -} - -// Is implements future error.Is functionality. -// A statusError is equivalent if the code and message are identical. -func (se *statusError) Is(target error) bool { - tse, ok := target.(*statusError) - if !ok { - return false - } - - return proto.Equal((*spb.Status)(se), (*spb.Status)(tse)) -} - // Status represents an RPC status code, message, and details. It is immutable // and should be created with New, Newf, or FromProto. type Status struct { @@ -105,7 +82,7 @@ func (s *Status) Err() error { if s.Code() == codes.OK { return nil } - return (*statusError)(s.s) + return (*status.StatusError)(s.s) } // New returns a Status representing c and msg. @@ -146,9 +123,10 @@ func FromError(err error) (s *Status, ok bool) { return nil, true } if se, ok := err.(interface { - GRPCStatus() *Status + GRPCStatus() *spb.Status }); ok { - return se.GRPCStatus(), true + s := se.GRPCStatus() + return FromProto(s), true } return New(codes.Unknown, err.Error()), false } @@ -204,9 +182,9 @@ func Code(err error) codes.Code { return codes.OK } if se, ok := err.(interface { - GRPCStatus() *Status + GRPCStatus() *spb.Status }); ok { - return se.GRPCStatus().Code() + return codes.Code(se.GRPCStatus().Code) } return codes.Unknown } diff --git a/status/status_test.go b/status/status_test.go index 35917acbb85..d07cde52626 100644 --- a/status/status_test.go +++ b/status/status_test.go @@ -24,6 +24,7 @@ import ( "fmt" "testing" + "google.golang.org/grpc/internal/status" "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes" apb "github.com/golang/protobuf/ptypes/any" @@ -63,7 +64,7 @@ func (s) TestErrorsWithSameParameters(t *testing.T) { e1 := Errorf(codes.AlreadyExists, description) e2 := Errorf(codes.AlreadyExists, description) if e1 == e2 || !errEqual(e1, e2) { - t.Fatalf("Errors should be equivalent but unique - e1: %v, %v e2: %p, %v", e1.(*statusError), e1, e2.(*statusError), e2) + t.Fatalf("Errors should be equivalent but unique - e1: %v, %v e2: %p, %v", e1.(*status.StatusError), e1, e2.(*status.StatusError), e2) } } @@ -115,7 +116,7 @@ func (s) TestError(t *testing.T) { func (s) TestErrorOK(t *testing.T) { err := Error(codes.OK, "foo") if err != nil { - t.Fatalf("Error(codes.OK, _) = %p; want nil", err.(*statusError)) + t.Fatalf("Error(codes.OK, _) = %p; want nil", err.(*status.StatusError)) } } @@ -153,13 +154,11 @@ func (c customError) Error() string { return fmt.Sprintf("rpc error: code = %s desc = %s", c.Code, c.Message) } -func (c customError) GRPCStatus() *Status { - return &Status{ - s: &spb.Status{ +func (c customError) GRPCStatus() *spb.Status { + return &spb.Status{ Code: int32(c.Code), Message: c.Message, Details: c.Details, - }, } } From e6f0e5c4866b4534f07ad072230c5b29569b8e64 Mon Sep 17 00:00:00 2001 From: James Protzman Date: Sun, 8 Mar 2020 17:54:46 -0400 Subject: [PATCH 2/9] go fmt --- internal/status/status.go | 2 +- status/status_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/status/status.go b/internal/status/status.go index 9c0f62ebd82..c36f1a801d0 100644 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -29,4 +29,4 @@ func (se *StatusError) Is(target error) bool { } return proto.Equal((*spb.Status)(se), (*spb.Status)(tse)) -} \ No newline at end of file +} diff --git a/status/status_test.go b/status/status_test.go index d07cde52626..2374c738b35 100644 --- a/status/status_test.go +++ b/status/status_test.go @@ -24,7 +24,6 @@ import ( "fmt" "testing" - "google.golang.org/grpc/internal/status" "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes" apb "github.com/golang/protobuf/ptypes/any" @@ -35,6 +34,7 @@ import ( spb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" "google.golang.org/grpc/internal/grpctest" + "google.golang.org/grpc/internal/status" ) type s struct { @@ -156,9 +156,9 @@ func (c customError) Error() string { func (c customError) GRPCStatus() *spb.Status { return &spb.Status{ - Code: int32(c.Code), - Message: c.Message, - Details: c.Details, + Code: int32(c.Code), + Message: c.Message, + Details: c.Details, } } From 849966feaf4ac4aa461c1e9529f664a7f0145746 Mon Sep 17 00:00:00 2001 From: James Protzman Date: Sun, 8 Mar 2020 23:00:13 -0400 Subject: [PATCH 3/9] Add license statement --- internal/status/status.go | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/internal/status/status.go b/internal/status/status.go index c36f1a801d0..0040426ab5c 100644 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -1,7 +1,34 @@ +/* + * + * Copyright 2020 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +// Package status implements errors returned by gRPC. These errors are +// serialized and transmitted on the wire between server and client, and allow +// for additional data to be transmitted via the Details field in the status +// proto. gRPC service handlers should return an error created by this +// package, and gRPC clients should expect a corresponding error to be +// returned from the RPC call. +// + package status import ( "fmt" + "github.com/golang/protobuf/proto" spb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" From 28dcba3cff127ba783aa54bdb6ca0c48d6c296d0 Mon Sep 17 00:00:00 2001 From: James Protzman Date: Thu, 26 Mar 2020 00:37:19 -0400 Subject: [PATCH 4/9] Break into statustype and internal/status --- internal/status/status.go | 9 +- internal/status/statustype/statustype.go | 135 +++++++++++++++++++++++ security/advancedtls/go.mod | 7 +- status/status.go | 97 ++-------------- status/status_test.go | 23 ++-- 5 files changed, 174 insertions(+), 97 deletions(-) create mode 100644 internal/status/statustype/statustype.go diff --git a/internal/status/status.go b/internal/status/status.go index 0040426ab5c..22984f051bd 100644 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -30,12 +30,14 @@ import ( "fmt" "github.com/golang/protobuf/proto" + spb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" + "google.golang.org/grpc/internal/status/statustype" ) // StatusError is an alias of a status proto. It implements error and Status, -// and a nil StatusError should never be returned by this package. +// and a nil statusError should never be returned by this package. type StatusError spb.Status func (se *StatusError) Error() string { @@ -43,8 +45,8 @@ func (se *StatusError) Error() string { return fmt.Sprintf("rpc error: code = %s desc = %s", codes.Code(p.GetCode()), p.GetMessage()) } -func (se *StatusError) GRPCStatus() *spb.Status { - return (*spb.Status)(se) +func (se *StatusError) GRPCStatus() *statustype.Status { + return statustype.FromProto((*spb.Status)(se)) } // Is implements future error.Is functionality. @@ -54,6 +56,5 @@ func (se *StatusError) Is(target error) bool { if !ok { return false } - return proto.Equal((*spb.Status)(se), (*spb.Status)(tse)) } diff --git a/internal/status/statustype/statustype.go b/internal/status/statustype/statustype.go new file mode 100644 index 00000000000..8452667b880 --- /dev/null +++ b/internal/status/statustype/statustype.go @@ -0,0 +1,135 @@ +/* + * + * Copyright 2020 gRPC authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +package statustype + +import ( + "errors" + "fmt" + + "github.com/golang/protobuf/proto" + "github.com/golang/protobuf/ptypes" + spb "google.golang.org/genproto/googleapis/rpc/status" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/internal/status" +) + +// Status represents an RPC status code, message, and details. It is immutable +// and should be created with New, Newf, or FromProto. +type Status struct { + s *spb.Status +} + +// New returns a Status representing c and msg. +func New(c codes.Code, msg string) *Status { + return &Status{s: &spb.Status{Code: int32(c), Message: msg}} +} + +// Newf returns New(c, fmt.Sprintf(format, a...)). +func Newf(c codes.Code, format string, a ...interface{}) *Status { + return New(c, fmt.Sprintf(format, a...)) +} + +// FromProto returns a Status representing s. +func FromProto(s *spb.Status) *Status { + return &Status{s: proto.Clone(s).(*spb.Status)} +} + +// Error returns an error representing c and msg. If c is OK, returns nil. +func Error(c codes.Code, msg string) error { + return New(c, msg).Err() +} + +// Errorf returns Error(c, fmt.Sprintf(format, a...)). +func Errorf(c codes.Code, format string, a ...interface{}) error { + return Error(c, fmt.Sprintf(format, a...)) +} + +// Code returns the status code contained in s. +func (s *Status) Code() codes.Code { + if s == nil || s.s == nil { + return codes.OK + } + return codes.Code(s.s.Code) +} + +// Message returns the message contained in s. +func (s *Status) Message() string { + if s == nil || s.s == nil { + return "" + } + return s.s.Message +} + +// Proto returns s's status as an spb.Status proto message. +func (s *Status) Proto() *spb.Status { + if s == nil { + return nil + } + return proto.Clone(s.s).(*spb.Status) +} + +// Err returns an immutable error representing s; returns nil if s.Code() is OK. +func (s *Status) Err() error { + if s.Code() == codes.OK { + return nil + } + return (*status.StatusError)(s.Proto()) +} + +func (s *Status) Error() string { + p := s.Proto() + return fmt.Sprintf("rpc error: code = %s desc = %s", codes.Code(p.GetCode()), p.GetMessage()) +} + +// WithDetails returns a new status with the provided details messages appended to the status. +// If any errors are encountered, it returns nil and the first error encountered. +func (s *Status) WithDetails(details ...proto.Message) (*Status, error) { + if s.Code() == codes.OK { + return nil, errors.New("no error details for status with code OK") + } + // s.Code() != OK implies that s.Proto() != nil. + p := s.Proto() + for _, detail := range details { + any, err := ptypes.MarshalAny(detail) + if err != nil { + return nil, err + } + p.Details = append(p.Details, any) + } + return &Status{s: p}, nil +} + +// Details returns a slice of details messages attached to the status. +// If a detail cannot be decoded, the error is returned in place of the detail. +func (s *Status) Details() []interface{} { + if s == nil || s.s == nil { + return nil + } + details := make([]interface{}, 0, len(s.s.Details)) + for _, any := range s.s.Details { + detail := &ptypes.DynamicAny{} + if err := ptypes.UnmarshalAny(any, detail); err != nil { + details = append(details, err) + continue + } + details = append(details, detail.Message) + } + return details +} diff --git a/security/advancedtls/go.mod b/security/advancedtls/go.mod index a9f7f46dbbb..8f920f39f33 100644 --- a/security/advancedtls/go.mod +++ b/security/advancedtls/go.mod @@ -2,4 +2,9 @@ module google.golang.org/grpc/security/advancedtls go 1.13 -require google.golang.org/grpc v1.27.0 +require ( + golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect + golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect + google.golang.org/grpc v1.27.0 + honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc // indirect +) diff --git a/status/status.go b/status/status.go index 5fdeed64c05..dcb06f6d09c 100644 --- a/status/status.go +++ b/status/status.go @@ -29,90 +29,53 @@ package status import ( "context" - "errors" "fmt" - "github.com/golang/protobuf/proto" - "github.com/golang/protobuf/ptypes" + "google.golang.org/grpc/internal/status" + spb "google.golang.org/genproto/googleapis/rpc/status" + "google.golang.org/grpc/codes" "google.golang.org/grpc/internal" - "google.golang.org/grpc/internal/status" + "google.golang.org/grpc/internal/status/statustype" ) func init() { internal.StatusRawProto = statusRawProto } -func statusRawProto(s *Status) *spb.Status { return s.s } +func statusRawProto(s *Status) *spb.Status { return s.Proto() } -// Status represents an RPC status code, message, and details. It is immutable -// and should be created with New, Newf, or FromProto. -type Status struct { - s *spb.Status -} - -// Code returns the status code contained in s. -func (s *Status) Code() codes.Code { - if s == nil || s.s == nil { - return codes.OK - } - return codes.Code(s.s.Code) -} - -// Message returns the message contained in s. -func (s *Status) Message() string { - if s == nil || s.s == nil { - return "" - } - return s.s.Message -} - -// Proto returns s's status as an spb.Status proto message. -func (s *Status) Proto() *spb.Status { - if s == nil { - return nil - } - return proto.Clone(s.s).(*spb.Status) -} - -// Err returns an immutable error representing s; returns nil if s.Code() is -// OK. -func (s *Status) Err() error { - if s.Code() == codes.OK { - return nil - } - return (*status.StatusError)(s.s) -} +type Status = statustype.Status // New returns a Status representing c and msg. func New(c codes.Code, msg string) *Status { - return &Status{s: &spb.Status{Code: int32(c), Message: msg}} + return statustype.New(c, msg) } // Newf returns New(c, fmt.Sprintf(format, a...)). func Newf(c codes.Code, format string, a ...interface{}) *Status { - return New(c, fmt.Sprintf(format, a...)) + return statustype.Newf(c, fmt.Sprintf(format, a...)) } // Error returns an error representing c and msg. If c is OK, returns nil. func Error(c codes.Code, msg string) error { - return New(c, msg).Err() + return statustype.New(c, msg).Err() } // Errorf returns Error(c, fmt.Sprintf(format, a...)). func Errorf(c codes.Code, format string, a ...interface{}) error { - return Error(c, fmt.Sprintf(format, a...)) + return (*status.StatusError)(statustype.Newf(c, format, a...).Proto()) } // ErrorProto returns an error representing s. If s.Code is OK, returns nil. func ErrorProto(s *spb.Status) error { - return FromProto(s).Err() + return statustype.FromProto(s).Err() } // FromProto returns a Status representing s. func FromProto(s *spb.Status) *Status { - return &Status{s: proto.Clone(s).(*spb.Status)} + return statustype.FromProto(s) } // FromError returns a Status representing err if it was produced from this @@ -138,42 +101,6 @@ func Convert(err error) *Status { return s } -// WithDetails returns a new status with the provided details messages appended to the status. -// If any errors are encountered, it returns nil and the first error encountered. -func (s *Status) WithDetails(details ...proto.Message) (*Status, error) { - if s.Code() == codes.OK { - return nil, errors.New("no error details for status with code OK") - } - // s.Code() != OK implies that s.Proto() != nil. - p := s.Proto() - for _, detail := range details { - any, err := ptypes.MarshalAny(detail) - if err != nil { - return nil, err - } - p.Details = append(p.Details, any) - } - return &Status{s: p}, nil -} - -// Details returns a slice of details messages attached to the status. -// If a detail cannot be decoded, the error is returned in place of the detail. -func (s *Status) Details() []interface{} { - if s == nil || s.s == nil { - return nil - } - details := make([]interface{}, 0, len(s.s.Details)) - for _, any := range s.s.Details { - detail := &ptypes.DynamicAny{} - if err := ptypes.UnmarshalAny(any, detail); err != nil { - details = append(details, err) - continue - } - details = append(details, detail.Message) - } - return details -} - // Code returns the Code of the error if it is a Status error, codes.OK if err // is nil, or codes.Unknown otherwise. func Code(err error) codes.Code { diff --git a/status/status_test.go b/status/status_test.go index 2374c738b35..eac8fc6c349 100644 --- a/status/status_test.go +++ b/status/status_test.go @@ -24,6 +24,10 @@ import ( "fmt" "testing" + "google.golang.org/grpc/internal/status/statustype" + + "google.golang.org/grpc/internal/status" + "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes" apb "github.com/golang/protobuf/ptypes/any" @@ -34,7 +38,6 @@ import ( spb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" "google.golang.org/grpc/internal/grpctest" - "google.golang.org/grpc/internal/status" ) type s struct { @@ -154,12 +157,18 @@ func (c customError) Error() string { return fmt.Sprintf("rpc error: code = %s desc = %s", c.Code, c.Message) } -func (c customError) GRPCStatus() *spb.Status { - return &spb.Status{ - Code: int32(c.Code), - Message: c.Message, - Details: c.Details, +func (c customError) GRPCStatus() *Status { + s := statustype.New(c.Code, c.Message) + var a []proto.Message + for _, d := range c.Details { + any, err := ptypes.MarshalAny(d) + if err != nil { + break + } + a = append(a, any) } + s, _ = s.WithDetails(a...) + return s } func (s) TestFromErrorImplementsInterface(t *testing.T) { @@ -343,7 +352,7 @@ func str(s *Status) string { if s.s == nil { return "" } - return fmt.Sprintf("", codes.Code(s.s.GetCode()), s.s.GetMessage(), s.s.GetDetails()) + return fmt.Sprintf("", s.Code(), s.Message(), s.Details()) } // mustMarshalAny converts a protobuf message to an any. From 4f60e1ac626cc8b2bdb2eb2d5fd7087600fb2a06 Mon Sep 17 00:00:00 2001 From: James Protzman Date: Sun, 29 Mar 2020 21:10:25 -0400 Subject: [PATCH 5/9] Move status / statusError into internal status package --- internal/status/status.go | 115 ++++++++++++++++++- internal/status/statustype/statustype.go | 135 ----------------------- status/status.go | 36 +++--- status/status_test.go | 23 ++-- 4 files changed, 134 insertions(+), 175 deletions(-) delete mode 100644 internal/status/statustype/statustype.go diff --git a/internal/status/status.go b/internal/status/status.go index 22984f051bd..03efe30df04 100644 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -27,15 +27,122 @@ package status import ( + "errors" "fmt" "github.com/golang/protobuf/proto" - + "github.com/golang/protobuf/ptypes" spb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" - "google.golang.org/grpc/internal/status/statustype" ) +// Status represents an RPC status code, message, and details. It is immutable +// and should be created with New, Newf, or FromProto. +type Status struct { + s *spb.Status +} + +// New returns a Status representing c and msg. +func New(c codes.Code, msg string) *Status { + return &Status{s: &spb.Status{Code: int32(c), Message: msg}} +} + +// Newf returns New(c, fmt.Sprintf(format, a...)). +func Newf(c codes.Code, format string, a ...interface{}) *Status { + return New(c, fmt.Sprintf(format, a...)) +} + +// FromProto returns a Status representing s. +func FromProto(s *spb.Status) *Status { + if s != nil { + fmt.Println(s) + } + return &Status{s: proto.Clone(s).(*spb.Status)} +} + +// Error returns an error representing c and msg. If c is OK, returns nil. +func Error(c codes.Code, msg string) error { + return New(c, msg).Err() +} + +// Errorf returns Error(c, fmt.Sprintf(format, a...)). +func Errorf(c codes.Code, format string, a ...interface{}) error { + return Error(c, fmt.Sprintf(format, a...)) +} + +// Code returns the status code contained in s. +func (s *Status) Code() codes.Code { + if s == nil || s.s == nil { + return codes.OK + } + return codes.Code(s.s.Code) +} + +// Message returns the message contained in s. +func (s *Status) Message() string { + if s == nil || s.s == nil { + return "" + } + return s.s.Message +} + +// Proto returns s's status as an spb.Status proto message. +func (s *Status) Proto() *spb.Status { + if s == nil { + return nil + } + return proto.Clone(s.s).(*spb.Status) +} + +// Err returns an immutable error representing s; returns nil if s.Code() is OK. +func (s *Status) Err() error { + if s.Code() == codes.OK { + return nil + } + return (*StatusError)(s.Proto()) +} + +func (s *Status) Error() string { + p := s.Proto() + return fmt.Sprintf("rpc error: code = %s desc = %s", codes.Code(p.GetCode()), p.GetMessage()) +} + +// WithDetails returns a new status with the provided details messages appended to the status. +// If any errors are encountered, it returns nil and the first error encountered. +func (s *Status) WithDetails(details ...proto.Message) (*Status, error) { + if s.Code() == codes.OK { + return nil, errors.New("no error details for status with code OK") + } + // s.Code() != OK implies that s.Proto() != nil. + p := s.Proto() + for _, detail := range details { + any, err := ptypes.MarshalAny(detail) + if err != nil { + return nil, err + } + p.Details = append(p.Details, any) + } + return &Status{s: p}, nil +} + +// Details returns a slice of details messages attached to the status. +// If a detail cannot be decoded, the error is returned in place of the detail. +func (s *Status) Details() []interface{} { + if s == nil || s.s == nil { + return nil + } + details := make([]interface{}, 0, len(s.s.Details)) + for _, any := range s.s.Details { + detail := &ptypes.DynamicAny{} + if err := ptypes.UnmarshalAny(any, detail); err != nil { + details = append(details, err) + continue + } + details = append(details, detail.Message) + } + return details +} + // StatusError is an alias of a status proto. It implements error and Status, // and a nil statusError should never be returned by this package. type StatusError spb.Status @@ -45,8 +152,8 @@ func (se *StatusError) Error() string { return fmt.Sprintf("rpc error: code = %s desc = %s", codes.Code(p.GetCode()), p.GetMessage()) } -func (se *StatusError) GRPCStatus() *statustype.Status { - return statustype.FromProto((*spb.Status)(se)) +func (se *StatusError) GRPCStatus() *Status { + return FromProto((*spb.Status)(se)) } // Is implements future error.Is functionality. diff --git a/internal/status/statustype/statustype.go b/internal/status/statustype/statustype.go deleted file mode 100644 index 8452667b880..00000000000 --- a/internal/status/statustype/statustype.go +++ /dev/null @@ -1,135 +0,0 @@ -/* - * - * Copyright 2020 gRPC authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - */ - -package statustype - -import ( - "errors" - "fmt" - - "github.com/golang/protobuf/proto" - "github.com/golang/protobuf/ptypes" - spb "google.golang.org/genproto/googleapis/rpc/status" - - "google.golang.org/grpc/codes" - "google.golang.org/grpc/internal/status" -) - -// Status represents an RPC status code, message, and details. It is immutable -// and should be created with New, Newf, or FromProto. -type Status struct { - s *spb.Status -} - -// New returns a Status representing c and msg. -func New(c codes.Code, msg string) *Status { - return &Status{s: &spb.Status{Code: int32(c), Message: msg}} -} - -// Newf returns New(c, fmt.Sprintf(format, a...)). -func Newf(c codes.Code, format string, a ...interface{}) *Status { - return New(c, fmt.Sprintf(format, a...)) -} - -// FromProto returns a Status representing s. -func FromProto(s *spb.Status) *Status { - return &Status{s: proto.Clone(s).(*spb.Status)} -} - -// Error returns an error representing c and msg. If c is OK, returns nil. -func Error(c codes.Code, msg string) error { - return New(c, msg).Err() -} - -// Errorf returns Error(c, fmt.Sprintf(format, a...)). -func Errorf(c codes.Code, format string, a ...interface{}) error { - return Error(c, fmt.Sprintf(format, a...)) -} - -// Code returns the status code contained in s. -func (s *Status) Code() codes.Code { - if s == nil || s.s == nil { - return codes.OK - } - return codes.Code(s.s.Code) -} - -// Message returns the message contained in s. -func (s *Status) Message() string { - if s == nil || s.s == nil { - return "" - } - return s.s.Message -} - -// Proto returns s's status as an spb.Status proto message. -func (s *Status) Proto() *spb.Status { - if s == nil { - return nil - } - return proto.Clone(s.s).(*spb.Status) -} - -// Err returns an immutable error representing s; returns nil if s.Code() is OK. -func (s *Status) Err() error { - if s.Code() == codes.OK { - return nil - } - return (*status.StatusError)(s.Proto()) -} - -func (s *Status) Error() string { - p := s.Proto() - return fmt.Sprintf("rpc error: code = %s desc = %s", codes.Code(p.GetCode()), p.GetMessage()) -} - -// WithDetails returns a new status with the provided details messages appended to the status. -// If any errors are encountered, it returns nil and the first error encountered. -func (s *Status) WithDetails(details ...proto.Message) (*Status, error) { - if s.Code() == codes.OK { - return nil, errors.New("no error details for status with code OK") - } - // s.Code() != OK implies that s.Proto() != nil. - p := s.Proto() - for _, detail := range details { - any, err := ptypes.MarshalAny(detail) - if err != nil { - return nil, err - } - p.Details = append(p.Details, any) - } - return &Status{s: p}, nil -} - -// Details returns a slice of details messages attached to the status. -// If a detail cannot be decoded, the error is returned in place of the detail. -func (s *Status) Details() []interface{} { - if s == nil || s.s == nil { - return nil - } - details := make([]interface{}, 0, len(s.s.Details)) - for _, any := range s.s.Details { - detail := &ptypes.DynamicAny{} - if err := ptypes.UnmarshalAny(any, detail); err != nil { - details = append(details, err) - continue - } - details = append(details, detail.Message) - } - return details -} diff --git a/status/status.go b/status/status.go index dcb06f6d09c..3e05bb183ba 100644 --- a/status/status.go +++ b/status/status.go @@ -31,51 +31,48 @@ import ( "context" "fmt" - "google.golang.org/grpc/internal/status" - spb "google.golang.org/genproto/googleapis/rpc/status" - "google.golang.org/grpc/codes" "google.golang.org/grpc/internal" - "google.golang.org/grpc/internal/status/statustype" + "google.golang.org/grpc/internal/status" ) +type Status = status.Status + func init() { internal.StatusRawProto = statusRawProto } func statusRawProto(s *Status) *spb.Status { return s.Proto() } -type Status = statustype.Status - // New returns a Status representing c and msg. func New(c codes.Code, msg string) *Status { - return statustype.New(c, msg) + return status.New(c, msg) } // Newf returns New(c, fmt.Sprintf(format, a...)). func Newf(c codes.Code, format string, a ...interface{}) *Status { - return statustype.Newf(c, fmt.Sprintf(format, a...)) + return status.Newf(c, format, a...) } // Error returns an error representing c and msg. If c is OK, returns nil. func Error(c codes.Code, msg string) error { - return statustype.New(c, msg).Err() + return status.New(c, msg).Err() } // Errorf returns Error(c, fmt.Sprintf(format, a...)). func Errorf(c codes.Code, format string, a ...interface{}) error { - return (*status.StatusError)(statustype.Newf(c, format, a...).Proto()) + return Error(c, fmt.Sprintf(format, a...)) } // ErrorProto returns an error representing s. If s.Code is OK, returns nil. func ErrorProto(s *spb.Status) error { - return statustype.FromProto(s).Err() + return FromProto(s).Err() } // FromProto returns a Status representing s. func FromProto(s *spb.Status) *Status { - return statustype.FromProto(s) + return status.FromProto(s) } // FromError returns a Status representing err if it was produced from this @@ -86,10 +83,9 @@ func FromError(err error) (s *Status, ok bool) { return nil, true } if se, ok := err.(interface { - GRPCStatus() *spb.Status + GRPCStatus() *Status }); ok { - s := se.GRPCStatus() - return FromProto(s), true + return se.GRPCStatus(), true } return New(codes.Unknown, err.Error()), false } @@ -109,9 +105,9 @@ func Code(err error) codes.Code { return codes.OK } if se, ok := err.(interface { - GRPCStatus() *spb.Status + GRPCStatus() *Status }); ok { - return codes.Code(se.GRPCStatus().Code) + return se.GRPCStatus().Code() } return codes.Unknown } @@ -124,10 +120,10 @@ func FromContextError(err error) *Status { case nil: return nil case context.DeadlineExceeded: - return New(codes.DeadlineExceeded, err.Error()) + return status.New(codes.DeadlineExceeded, err.Error()) case context.Canceled: - return New(codes.Canceled, err.Error()) + return status.New(codes.Canceled, err.Error()) default: - return New(codes.Unknown, err.Error()) + return status.New(codes.Unknown, err.Error()) } } diff --git a/status/status_test.go b/status/status_test.go index eac8fc6c349..56634ee108c 100644 --- a/status/status_test.go +++ b/status/status_test.go @@ -24,10 +24,6 @@ import ( "fmt" "testing" - "google.golang.org/grpc/internal/status/statustype" - - "google.golang.org/grpc/internal/status" - "github.com/golang/protobuf/proto" "github.com/golang/protobuf/ptypes" apb "github.com/golang/protobuf/ptypes/any" @@ -38,6 +34,7 @@ import ( spb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" "google.golang.org/grpc/internal/grpctest" + "google.golang.org/grpc/internal/status" ) type s struct { @@ -158,17 +155,11 @@ func (c customError) Error() string { } func (c customError) GRPCStatus() *Status { - s := statustype.New(c.Code, c.Message) - var a []proto.Message - for _, d := range c.Details { - any, err := ptypes.MarshalAny(d) - if err != nil { - break - } - a = append(a, any) - } - s, _ = s.WithDetails(a...) - return s + return status.FromProto(&spb.Status{ + Code: int32(c.Code), + Message: c.Message, + Details: c.Details, + }) } func (s) TestFromErrorImplementsInterface(t *testing.T) { @@ -349,7 +340,7 @@ func str(s *Status) string { if s == nil { return "nil" } - if s.s == nil { + if s.Proto() == nil { return "" } return fmt.Sprintf("", s.Code(), s.Message(), s.Details()) From 21b5a13759b8f3541f06a0609cfdcfc900b0ca1f Mon Sep 17 00:00:00 2001 From: James Protzman Date: Sun, 29 Mar 2020 21:22:45 -0400 Subject: [PATCH 6/9] remove security/advancedtls/go.mod changes, undo uneccessary status references --- security/advancedtls/go.mod | 7 +------ status/status.go | 10 +++++----- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/security/advancedtls/go.mod b/security/advancedtls/go.mod index 8f920f39f33..a9f7f46dbbb 100644 --- a/security/advancedtls/go.mod +++ b/security/advancedtls/go.mod @@ -2,9 +2,4 @@ module google.golang.org/grpc/security/advancedtls go 1.13 -require ( - golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect - golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect - google.golang.org/grpc v1.27.0 - honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc // indirect -) +require google.golang.org/grpc v1.27.0 diff --git a/status/status.go b/status/status.go index 3e05bb183ba..8b14e7b0354 100644 --- a/status/status.go +++ b/status/status.go @@ -52,12 +52,12 @@ func New(c codes.Code, msg string) *Status { // Newf returns New(c, fmt.Sprintf(format, a...)). func Newf(c codes.Code, format string, a ...interface{}) *Status { - return status.Newf(c, format, a...) + return New(c, fmt.Sprintf(format, a...)) } // Error returns an error representing c and msg. If c is OK, returns nil. func Error(c codes.Code, msg string) error { - return status.New(c, msg).Err() + return New(c, msg).Err() } // Errorf returns Error(c, fmt.Sprintf(format, a...)). @@ -120,10 +120,10 @@ func FromContextError(err error) *Status { case nil: return nil case context.DeadlineExceeded: - return status.New(codes.DeadlineExceeded, err.Error()) + return New(codes.DeadlineExceeded, err.Error()) case context.Canceled: - return status.New(codes.Canceled, err.Error()) + return New(codes.Canceled, err.Error()) default: - return status.New(codes.Unknown, err.Error()) + return New(codes.Unknown, err.Error()) } } From cf4fbe7c904c2f1c1d00a0c2625bfd7b79c997e9 Mon Sep 17 00:00:00 2001 From: James Protzman Date: Thu, 2 Apr 2020 20:37:09 -0400 Subject: [PATCH 7/9] Fix PR comments, remove statusRawProto from internal --- internal/internal.go | 5 ----- internal/status/status.go | 6 ++---- internal/transport/http2_server.go | 7 +------ security/advancedtls/go.mod | 7 ++++++- status/status.go | 12 +++++------- 5 files changed, 14 insertions(+), 23 deletions(-) diff --git a/internal/internal.go b/internal/internal.go index 0912f0bf4c3..c6fbe8bb1b2 100644 --- a/internal/internal.go +++ b/internal/internal.go @@ -37,11 +37,6 @@ var ( // KeepaliveMinPingTime is the minimum ping interval. This must be 10s by // default, but tests may wish to set it lower for convenience. KeepaliveMinPingTime = 10 * time.Second - // StatusRawProto is exported by status/status.go. This func returns a - // pointer to the wrapped Status proto for a given status.Status without a - // call to proto.Clone(). The returned Status proto should not be mutated by - // the caller. - StatusRawProto interface{} // func (*status.Status) *spb.Status // NewRequestInfoContext creates a new context based on the argument context attaching // the passed in RequestInfo to the new context. NewRequestInfoContext interface{} // func(context.Context, credentials.RequestInfo) context.Context diff --git a/internal/status/status.go b/internal/status/status.go index 03efe30df04..3f27a59bc4f 100644 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -23,7 +23,8 @@ // package, and gRPC clients should expect a corresponding error to be // returned from the RPC call. // - +// This package upholds the invariants that a non-nil error may not +// contain an OK code, and an OK code must result in a nil error. package status import ( @@ -54,9 +55,6 @@ func Newf(c codes.Code, format string, a ...interface{}) *Status { // FromProto returns a Status representing s. func FromProto(s *spb.Status) *Status { - if s != nil { - fmt.Println(s) - } return &Status{s: proto.Clone(s).(*spb.Status)} } diff --git a/internal/transport/http2_server.go b/internal/transport/http2_server.go index cbcedc8db27..fa33ffb1885 100644 --- a/internal/transport/http2_server.go +++ b/internal/transport/http2_server.go @@ -35,11 +35,9 @@ import ( "golang.org/x/net/http2" "golang.org/x/net/http2/hpack" - spb "google.golang.org/genproto/googleapis/rpc/status" "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/grpclog" - "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/channelz" "google.golang.org/grpc/internal/grpcrand" "google.golang.org/grpc/keepalive" @@ -57,9 +55,6 @@ var ( // ErrHeaderListSizeLimitViolation indicates that the header list size is larger // than the limit set by peer. ErrHeaderListSizeLimitViolation = errors.New("transport: trying to send header list size larger than the limit set by peer") - // statusRawProto is a function to get to the raw status proto wrapped in a - // status.Status without a proto.Clone(). - statusRawProto = internal.StatusRawProto.(func(*status.Status) *spb.Status) ) // serverConnectionCounter counts the number of connections a server has seen @@ -850,7 +845,7 @@ func (t *http2Server) WriteStatus(s *Stream, st *status.Status) error { headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-status", Value: strconv.Itoa(int(st.Code()))}) headerFields = append(headerFields, hpack.HeaderField{Name: "grpc-message", Value: encodeGrpcMessage(st.Message())}) - if p := statusRawProto(st); p != nil && len(p.Details) > 0 { + if p := st.Proto(); p != nil && len(p.Details) > 0 { stBytes, err := proto.Marshal(p) if err != nil { // TODO: return error instead, when callers are able to handle it. diff --git a/security/advancedtls/go.mod b/security/advancedtls/go.mod index a9f7f46dbbb..8f920f39f33 100644 --- a/security/advancedtls/go.mod +++ b/security/advancedtls/go.mod @@ -2,4 +2,9 @@ module google.golang.org/grpc/security/advancedtls go 1.13 -require google.golang.org/grpc v1.27.0 +require ( + golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect + golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect + google.golang.org/grpc v1.27.0 + honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc // indirect +) diff --git a/status/status.go b/status/status.go index 8b14e7b0354..01e182c306c 100644 --- a/status/status.go +++ b/status/status.go @@ -32,19 +32,17 @@ import ( "fmt" spb "google.golang.org/genproto/googleapis/rpc/status" + "google.golang.org/grpc/codes" - "google.golang.org/grpc/internal" "google.golang.org/grpc/internal/status" ) +// Status references google.golang.org/grpc/internal/status. It represents an +// RPC status code, message, and details. It is immutable and should be +// created with New, Newf, or FromProto. +// https://godoc.org/google.golang.org/grpc/internal/status type Status = status.Status -func init() { - internal.StatusRawProto = statusRawProto -} - -func statusRawProto(s *Status) *spb.Status { return s.Proto() } - // New returns a Status representing c and msg. func New(c codes.Code, msg string) *Status { return status.New(c, msg) From 5f104d4833ab70f32a00919fe596358b8e9890dc Mon Sep 17 00:00:00 2001 From: James Protzman Date: Fri, 3 Apr 2020 13:51:10 -0400 Subject: [PATCH 8/9] revert mod file to master --- security/advancedtls/go.mod | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/security/advancedtls/go.mod b/security/advancedtls/go.mod index 8f920f39f33..a9f7f46dbbb 100644 --- a/security/advancedtls/go.mod +++ b/security/advancedtls/go.mod @@ -2,9 +2,4 @@ module google.golang.org/grpc/security/advancedtls go 1.13 -require ( - golang.org/x/lint v0.0.0-20190313153728-d0100b6bd8b3 // indirect - golang.org/x/tools v0.0.0-20190524140312-2c0ae7006135 // indirect - google.golang.org/grpc v1.27.0 - honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc // indirect -) +require google.golang.org/grpc v1.27.0 From d9fa8220818282b3b0eb37706cfcf81d10a16a2a Mon Sep 17 00:00:00 2001 From: James Protzman Date: Fri, 3 Apr 2020 18:17:10 -0400 Subject: [PATCH 9/9] Fix lint issues, rename StatusError to Error, and comment function --- internal/status/status.go | 25 +++++++++++++------------ status/status_test.go | 4 ++-- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/internal/status/status.go b/internal/status/status.go index 3f27a59bc4f..23dae8e5679 100644 --- a/internal/status/status.go +++ b/internal/status/status.go @@ -58,14 +58,14 @@ func FromProto(s *spb.Status) *Status { return &Status{s: proto.Clone(s).(*spb.Status)} } -// Error returns an error representing c and msg. If c is OK, returns nil. -func Error(c codes.Code, msg string) error { +// Err returns an error representing c and msg. If c is OK, returns nil. +func Err(c codes.Code, msg string) error { return New(c, msg).Err() } // Errorf returns Error(c, fmt.Sprintf(format, a...)). func Errorf(c codes.Code, format string, a ...interface{}) error { - return Error(c, fmt.Sprintf(format, a...)) + return Err(c, fmt.Sprintf(format, a...)) } // Code returns the status code contained in s. @@ -97,7 +97,7 @@ func (s *Status) Err() error { if s.Code() == codes.OK { return nil } - return (*StatusError)(s.Proto()) + return (*Error)(s.Proto()) } func (s *Status) Error() string { @@ -141,23 +141,24 @@ func (s *Status) Details() []interface{} { return details } -// StatusError is an alias of a status proto. It implements error and Status, -// and a nil statusError should never be returned by this package. -type StatusError spb.Status +// Error is an alias of a status proto. It implements error and Status, +// and a nil Error should never be returned by this package. +type Error spb.Status -func (se *StatusError) Error() string { +func (se *Error) Error() string { p := (*spb.Status)(se) return fmt.Sprintf("rpc error: code = %s desc = %s", codes.Code(p.GetCode()), p.GetMessage()) } -func (se *StatusError) GRPCStatus() *Status { +// GRPCStatus returns the Status represented by se. +func (se *Error) GRPCStatus() *Status { return FromProto((*spb.Status)(se)) } // Is implements future error.Is functionality. -// A statusError is equivalent if the code and message are identical. -func (se *StatusError) Is(target error) bool { - tse, ok := target.(*StatusError) +// A Error is equivalent if the code and message are identical. +func (se *Error) Is(target error) bool { + tse, ok := target.(*Error) if !ok { return false } diff --git a/status/status_test.go b/status/status_test.go index 56634ee108c..839a3c390ed 100644 --- a/status/status_test.go +++ b/status/status_test.go @@ -64,7 +64,7 @@ func (s) TestErrorsWithSameParameters(t *testing.T) { e1 := Errorf(codes.AlreadyExists, description) e2 := Errorf(codes.AlreadyExists, description) if e1 == e2 || !errEqual(e1, e2) { - t.Fatalf("Errors should be equivalent but unique - e1: %v, %v e2: %p, %v", e1.(*status.StatusError), e1, e2.(*status.StatusError), e2) + t.Fatalf("Errors should be equivalent but unique - e1: %v, %v e2: %p, %v", e1.(*status.Error), e1, e2.(*status.Error), e2) } } @@ -116,7 +116,7 @@ func (s) TestError(t *testing.T) { func (s) TestErrorOK(t *testing.T) { err := Error(codes.OK, "foo") if err != nil { - t.Fatalf("Error(codes.OK, _) = %p; want nil", err.(*status.StatusError)) + t.Fatalf("Error(codes.OK, _) = %p; want nil", err.(*status.Error)) } }