mirror/protobuf-go
1
0
Fork 0
mirror of https://github.com/protocolbuffers/protobuf-go.git synced 2025-02-24 18:39:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

all: improve panic messages for better debugability

Browse Source 
Change-Id: If3e505e715d5ce2c9a81249c868d26226a25b724
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/232339
Reviewed-by: Damien Neil <dneil@google.com>
This commit is contained in:
Joe Tsai 2020-04-22 21:58:56 -07:00
parent ce5d8318a0
commit 1f5b6fe645
9 changed files with 106 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
internal/cmd/generate-types/impl.go
View File

@ -820,7 +820,7 @@ func (m *{{.}}) WhichOneof(od protoreflect.OneofDescriptor) protoreflect.FieldDe
if oi := m.messageInfo().oneofs[od.Name()]; oi != nil && oi.oneofDesc == od {
return od.Fields().ByNumber(oi.which(m.pointer()))
}
panic("invalid oneof descriptor")
panic("invalid oneof descriptor " + string(od.FullName()) + " for message " + string(m.Descriptor().FullName()))
}
func (m *{{.}}) GetUnknown() protoreflect.RawFields {
m.messageInfo().init()

2
internal/filedesc/desc.go
View File

@ -607,7 +607,7 @@ func (dv *defaultValue) get(fd pref.FieldDescriptor) pref.Value {
// TODO: Avoid panic if we're running with the race detector
// and instead spawn a goroutine that periodically resets
// this value back to the original to induce a race.
panic("detected mutation on the default bytes")
panic(fmt.Sprintf("detected mutation on the default bytes for %v", fd.FullName()))
}
return dv.val
}

15
internal/impl/message_reflect.go
View File

@ -338,24 +338,27 @@ func (mi *MessageInfo) checkField(fd pref.FieldDescriptor) (*fieldInfo, pref.Ext
}
if fi != nil {
if fi.fieldDesc != fd {
panic("mismatching field descriptor")
if got, want := fd.FullName(), fi.fieldDesc.FullName(); got != want {
panic(fmt.Sprintf("mismatching field: got %v, want %v", got, want))
}
panic(fmt.Sprintf("mismatching field: %v", fd.FullName()))
}
return fi, nil
}
if fd.IsExtension() {
if fd.ContainingMessage().FullName() != mi.Desc.FullName() {
if got, want := fd.ContainingMessage().FullName(), mi.Desc.FullName(); got != want {
// TODO: Should this be exact containing message descriptor match?
panic("mismatching containing message")
panic(fmt.Sprintf("extension %v has mismatching containing message: got %v, want %v", fd.FullName(), got, want))
}
if !mi.Desc.ExtensionRanges().Has(fd.Number()) {
panic("invalid extension field")
panic(fmt.Sprintf("extension %v extends %v outside the extension range", fd.FullName(), mi.Desc.FullName()))
}
xtd, ok := fd.(pref.ExtensionTypeDescriptor)
if !ok {
panic("extension descriptor does not implement ExtensionTypeDescriptor")
panic(fmt.Sprintf("extension %v does not implement protoreflect.ExtensionTypeDescriptor", fd.FullName()))
}
return nil, xtd.Type()
}
panic("invalid field descriptor")
panic(fmt.Sprintf("field %v is invalid", fd.FullName()))
}

29
internal/impl/message_reflect_field.go
View File

@ -31,13 +31,13 @@ type fieldInfo struct {
func fieldInfoForOneof(fd pref.FieldDescriptor, fs reflect.StructField, x exporter, ot reflect.Type) fieldInfo {
ft := fs.Type
if ft.Kind() != reflect.Interface {
panic(fmt.Sprintf("invalid type: got %v, want interface kind", ft))
panic(fmt.Sprintf("field %v has invalid type: got %v, want interface kind", fd.FullName(), ft))
}
if ot.Kind() != reflect.Struct {
panic(fmt.Sprintf("invalid type: got %v, want struct kind", ot))
panic(fmt.Sprintf("field %v has invalid type: got %v, want struct kind", fd.FullName(), ot))
}
if !reflect.PtrTo(ot).Implements(ft) {
panic(fmt.Sprintf("invalid type: %v does not implement %v", ot, ft))
panic(fmt.Sprintf("field %v has invalid type: %v does not implement %v", fd.FullName(), ot, ft))
}
conv := NewConverter(ot.Field(0).Type, fd)
isMessage := fd.Message() != nil
@ -90,7 +90,7 @@ func fieldInfoForOneof(fd pref.FieldDescriptor, fs reflect.StructField, x export
},
mutable: func(p pointer) pref.Value {
if !isMessage {
panic("invalid Mutable on field with non-composite type")
panic(fmt.Sprintf("field %v with invalid Mutable call on field with non-composite type", fd.FullName()))
}
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
if rv.IsNil() || rv.Elem().Type().Elem() != ot || rv.Elem().IsNil() {
@ -114,7 +114,7 @@ func fieldInfoForOneof(fd pref.FieldDescriptor, fs reflect.StructField, x export
func fieldInfoForMap(fd pref.FieldDescriptor, fs reflect.StructField, x exporter) fieldInfo {
ft := fs.Type
if ft.Kind() != reflect.Map {
panic(fmt.Sprintf("invalid type: got %v, want map kind", ft))
panic(fmt.Sprintf("field %v has invalid type: got %v, want map kind", fd.FullName(), ft))
}
conv := NewConverter(ft, fd)
@ -147,7 +147,7 @@ func fieldInfoForMap(fd pref.FieldDescriptor, fs reflect.StructField, x exporter
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
pv := conv.GoValueOf(v)
if pv.IsNil() {
panic(fmt.Sprintf("invalid value: setting map field to read-only value"))
panic(fmt.Sprintf("map field %v cannot be set with read-only value", fd.FullName()))
}
rv.Set(pv)
},
@ -167,7 +167,7 @@ func fieldInfoForMap(fd pref.FieldDescriptor, fs reflect.StructField, x exporter
func fieldInfoForList(fd pref.FieldDescriptor, fs reflect.StructField, x exporter) fieldInfo {
ft := fs.Type
if ft.Kind() != reflect.Slice {
panic(fmt.Sprintf("invalid type: got %v, want slice kind", ft))
panic(fmt.Sprintf("field %v has invalid type: got %v, want slice kind", fd.FullName(), ft))
}
conv := NewConverter(reflect.PtrTo(ft), fd)
@ -200,7 +200,7 @@ func fieldInfoForList(fd pref.FieldDescriptor, fs reflect.StructField, x exporte
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
pv := conv.GoValueOf(v)
if pv.IsNil() {
panic(fmt.Sprintf("invalid value: setting repeated field to read-only value"))
panic(fmt.Sprintf("list field %v cannot be set with read-only value", fd.FullName()))
}
rv.Set(pv.Elem())
},
@ -225,7 +225,7 @@ func fieldInfoForScalar(fd pref.FieldDescriptor, fs reflect.StructField, x expor
isBytes := ft.Kind() == reflect.Slice && ft.Elem().Kind() == reflect.Uint8
if nullable {
if ft.Kind() != reflect.Ptr && ft.Kind() != reflect.Slice {
panic(fmt.Sprintf("invalid type: got %v, want pointer", ft))
panic(fmt.Sprintf("field %v has invalid type: got %v, want pointer", fd.FullName(), ft))
}
if ft.Kind() == reflect.Ptr {
ft = ft.Elem()
@ -257,7 +257,7 @@ func fieldInfoForScalar(fd pref.FieldDescriptor, fs reflect.StructField, x expor
case reflect.String, reflect.Slice:
return rv.Len() > 0
default:
panic(fmt.Sprintf("invalid type: %v", rv.Type())) // should never happen
panic(fmt.Sprintf("field %v has invalid type: %v", fd.FullName(), rv.Type())) // should never happen
}
},
clear: func(p pointer) {
@ -314,7 +314,7 @@ func fieldInfoForWeakMessage(fd pref.FieldDescriptor, weakOffset offset) fieldIn
messageName := fd.Message().FullName()
messageType, _ = preg.GlobalTypes.FindMessageByName(messageName)
if messageType == nil {
panic(fmt.Sprintf("weak message %v is not linked in", messageName))
panic(fmt.Sprintf("weak message %v for field %v is not linked in", messageName, fd.FullName()))
}
})
}
@ -347,7 +347,10 @@ func fieldInfoForWeakMessage(fd pref.FieldDescriptor, weakOffset offset) fieldIn
lazyInit()
m := v.Message()
if m.Descriptor() != messageType.Descriptor() {
panic("mismatching message descriptor")
if got, want := m.Descriptor().FullName(), messageType.Descriptor().FullName(); got != want {
panic(fmt.Sprintf("field %v has mismatching message descriptor: got %v, want %v", fd.FullName(), got, want))
}
panic(fmt.Sprintf("field %v has mismatching message descriptor: %v", fd.FullName(), m.Descriptor().FullName()))
}
p.Apply(weakOffset).WeakFields().set(num, m.Interface())
},
@ -402,7 +405,7 @@ func fieldInfoForMessage(fd pref.FieldDescriptor, fs reflect.StructField, x expo
rv := p.Apply(fieldOffset).AsValueOf(fs.Type).Elem()
rv.Set(conv.GoValueOf(v))
if rv.IsNil() {
panic("invalid nil pointer")
panic(fmt.Sprintf("field %v has invalid nil pointer", fd.FullName()))
}
},
mutable: func(p pointer) pref.Value {

4
internal/impl/message_reflect_gen.go
View File

@ -114,7 +114,7 @@ func (m *messageState) WhichOneof(od protoreflect.OneofDescriptor) protoreflect.
if oi := m.messageInfo().oneofs[od.Name()]; oi != nil && oi.oneofDesc == od {
return od.Fields().ByNumber(oi.which(m.pointer()))
}
panic("invalid oneof descriptor")
panic("invalid oneof descriptor " + string(od.FullName()) + " for message " + string(m.Descriptor().FullName()))
}
func (m *messageState) GetUnknown() protoreflect.RawFields {
m.messageInfo().init()
@ -234,7 +234,7 @@ func (m *messageReflectWrapper) WhichOneof(od protoreflect.OneofDescriptor) prot
if oi := m.messageInfo().oneofs[od.Name()]; oi != nil && oi.oneofDesc == od {
return od.Fields().ByNumber(oi.which(m.pointer()))
}
panic("invalid oneof descriptor")
panic("invalid oneof descriptor " + string(od.FullName()) + " for message " + string(m.Descriptor().FullName()))
}
func (m *messageReflectWrapper) GetUnknown() protoreflect.RawFields {
m.messageInfo().init()

6
internal/impl/pointer_unsafe.go
View File

@ -148,7 +148,11 @@ func (ms *messageState) pointer() pointer {
return pointer{p: unsafe.Pointer(ms)}
}
func (ms *messageState) messageInfo() *MessageInfo {
return ms.LoadMessageInfo()
mi := ms.LoadMessageInfo()
if mi == nil {
panic("invalid nil message info; this suggests memory corruption due to a race or shallow copy on the message struct")
}
return mi
}
func (ms *messageState) LoadMessageInfo() *MessageInfo {
return (*MessageInfo)(atomic.LoadPointer((*unsafe.Pointer)(unsafe.Pointer(&ms.atomicMessageInfo))))

7
proto/merge.go
View File

@ -5,6 +5,8 @@
package proto
import (
"fmt"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/runtime/protoiface"
)
@ -26,6 +28,9 @@ func Merge(dst, src Message) {
dstMsg, srcMsg := dst.ProtoReflect(), src.ProtoReflect()
if dstMsg.Descriptor() != srcMsg.Descriptor() {
if got, want := dstMsg.Descriptor().FullName(), srcMsg.Descriptor().FullName(); got != want {
panic(fmt.Sprintf("descriptor mismatch: %v != %v", got, want))
}
panic("descriptor mismatch")
}
mergeOptions{}.mergeMessage(dstMsg, srcMsg)
@ -72,7 +77,7 @@ func (o mergeOptions) mergeMessage(dst, src protoreflect.Message) {
}
if !dst.IsValid() {
panic("cannot merge into invalid destination message")
panic(fmt.Sprintf("cannot merge into invalid %v message", dst.Descriptor().FullName()))
}
src.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {

8
proto/reset.go
View File

@ -4,7 +4,11 @@
package proto
import "google.golang.org/protobuf/reflect/protoreflect"
import (
"fmt"
"google.golang.org/protobuf/reflect/protoreflect"
)
// Reset clears every field in the message.
// The resulting message shares no observable memory with its previous state
@ -19,7 +23,7 @@ func Reset(m Message) {
func resetMessage(m protoreflect.Message) {
if !m.IsValid() {
panic("cannot reset invalid message")
panic(fmt.Sprintf("cannot reset invalid %v message", m.Descriptor().FullName()))
}
// Clear all known fields.

78
reflect/protoreflect/value_union.go
View File

@ -7,7 +7,6 @@ package protoreflect
import (
"fmt"
"math"
"reflect"
)
// Value is a union where only one Go type may be set at a time.
@ -87,7 +86,7 @@ func ValueOf(v interface{}) Value {
case Message, List, Map:
return valueOfIface(v)
default:
panic(fmt.Sprintf("invalid type: %v", reflect.TypeOf(v)))
panic(fmt.Sprintf("invalid type: %T", v))
}
}
@ -197,13 +196,55 @@ func (v Value) Interface() interface{} {
}
}
func (v Value) typeName() string {
switch v.typ {
case nilType:
return "nil"
case boolType:
return "bool"
case int32Type:
return "int32"
case int64Type:
return "int64"
case uint32Type:
return "uint32"
case uint64Type:
return "uint64"
case float32Type:
return "float32"
case float64Type:
return "float64"
case stringType:
return "string"
case bytesType:
return "bytes"
case enumType:
return "enum"
default:
switch v := v.getIface().(type) {
case Message:
return "message"
case List:
return "list"
case Map:
return "map"
default:
return fmt.Sprintf("<unknown: %T>", v)
}
}
}
func (v Value) panicMessage(what string) string {
return fmt.Sprintf("type mismatch: cannot convert %v to %s", v.typeName(), what)
}
// Bool returns v as a bool and panics if the type is not a bool.
func (v Value) Bool() bool {
switch v.typ {
case boolType:
return v.num > 0
default:
panic("proto: value type mismatch")
panic(v.panicMessage("bool"))
}
}
@ -213,7 +254,7 @@ func (v Value) Int() int64 {
case int32Type, int64Type:
return int64(v.num)
default:
panic("proto: value type mismatch")
panic(v.panicMessage("int"))
}
}
@ -223,7 +264,7 @@ func (v Value) Uint() uint64 {
case uint32Type, uint64Type:
return uint64(v.num)
default:
panic("proto: value type mismatch")
panic(v.panicMessage("uint"))
}
}
@ -233,7 +274,7 @@ func (v Value) Float() float64 {
case float32Type, float64Type:
return math.Float64frombits(uint64(v.num))
default:
panic("proto: value type mismatch")
panic(v.panicMessage("float"))
}
}
@ -254,7 +295,7 @@ func (v Value) Bytes() []byte {
case bytesType:
return v.getBytes()
default:
panic("proto: value type mismatch")
panic(v.panicMessage("bytes"))
}
}
@ -264,37 +305,37 @@ func (v Value) Enum() EnumNumber {
case enumType:
return EnumNumber(v.num)
default:
panic("proto: value type mismatch")
panic(v.panicMessage("enum"))
}
}
// Message returns v as a Message and panics if the type is not a Message.
func (v Value) Message() Message {
switch v := v.getIface().(type) {
switch vi := v.getIface().(type) {
case Message:
return v
return vi
default:
panic("proto: value type mismatch")
panic(v.panicMessage("message"))
}
}
// List returns v as a List and panics if the type is not a List.
func (v Value) List() List {
switch v := v.getIface().(type) {
switch vi := v.getIface().(type) {
case List:
return v
return vi
default:
panic("proto: value type mismatch")
panic(v.panicMessage("list"))
}
}
// Map returns v as a Map and panics if the type is not a Map.
func (v Value) Map() Map {
switch v := v.getIface().(type) {
switch vi := v.getIface().(type) {
case Map:
return v
return vi
default:
panic("proto: value type mismatch")
panic(v.panicMessage("map"))
}
}
@ -303,8 +344,9 @@ func (v Value) MapKey() MapKey {
switch v.typ {
case boolType, int32Type, int64Type, uint32Type, uint64Type, stringType:
return MapKey(v)
default:
panic(v.panicMessage("map key"))
}
panic("proto: invalid map key type")
}
// MapKey is used to index maps, where the Go type of the MapKey must match