all: improve panic messages for better debugability
Change-Id: If3e505e715d5ce2c9a81249c868d26226a25b724
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/232339
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/internal/impl/message_reflect.go b/internal/impl/message_reflect.go
index 3eb2b13..0f4b8db 100644
--- a/internal/impl/message_reflect.go
+++ b/internal/impl/message_reflect.go
@@ -338,24 +338,27 @@
}
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()))
}
diff --git a/internal/impl/message_reflect_field.go b/internal/impl/message_reflect_field.go
index ea6f755..23124a8 100644
--- a/internal/impl/message_reflect_field.go
+++ b/internal/impl/message_reflect_field.go
@@ -31,13 +31,13 @@
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 @@
},
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 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 @@
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 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 {
diff --git a/internal/impl/message_reflect_gen.go b/internal/impl/message_reflect_gen.go
index 7d65f5c..741d6e5 100644
--- a/internal/impl/message_reflect_gen.go
+++ b/internal/impl/message_reflect_gen.go
@@ -114,7 +114,7 @@
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 @@
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()
diff --git a/internal/impl/pointer_unsafe.go b/internal/impl/pointer_unsafe.go
index 2741643..088aa85 100644
--- a/internal/impl/pointer_unsafe.go
+++ b/internal/impl/pointer_unsafe.go
@@ -148,7 +148,11 @@
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))))