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/cmd/generate-types/impl.go b/internal/cmd/generate-types/impl.go
index 4b362c6..6068671 100644
--- a/internal/cmd/generate-types/impl.go
+++ b/internal/cmd/generate-types/impl.go
@@ -820,7 +820,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 *{{.}}) GetUnknown() protoreflect.RawFields {
m.messageInfo().init()
diff --git a/internal/filedesc/desc.go b/internal/filedesc/desc.go
index 13b7723..2540bef 100644
--- a/internal/filedesc/desc.go
+++ b/internal/filedesc/desc.go
@@ -607,7 +607,7 @@
// 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
}
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))))
diff --git a/proto/merge.go b/proto/merge.go
index 27ab1a6..d761ab3 100644
--- a/proto/merge.go
+++ b/proto/merge.go
@@ -5,6 +5,8 @@
package proto
import (
+ "fmt"
+
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/runtime/protoiface"
)
@@ -26,6 +28,9 @@
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 @@
}
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 {
diff --git a/proto/reset.go b/proto/reset.go
index 22048bc..3d7f894 100644
--- a/proto/reset.go
+++ b/proto/reset.go
@@ -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 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.
diff --git a/reflect/protoreflect/value_union.go b/reflect/protoreflect/value_union.go
index d695a4d..f334f71 100644
--- a/reflect/protoreflect/value_union.go
+++ b/reflect/protoreflect/value_union.go
@@ -7,7 +7,6 @@
import (
"fmt"
"math"
- "reflect"
)
// Value is a union where only one Go type may be set at a time.
@@ -87,7 +86,7 @@
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) 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 @@
case int32Type, int64Type:
return int64(v.num)
default:
- panic("proto: value type mismatch")
+ panic(v.panicMessage("int"))
}
}
@@ -223,7 +264,7 @@
case uint32Type, uint64Type:
return uint64(v.num)
default:
- panic("proto: value type mismatch")
+ panic(v.panicMessage("uint"))
}
}
@@ -233,7 +274,7 @@
case float32Type, float64Type:
return math.Float64frombits(uint64(v.num))
default:
- panic("proto: value type mismatch")
+ panic(v.panicMessage("float"))
}
}
@@ -254,7 +295,7 @@
case bytesType:
return v.getBytes()
default:
- panic("proto: value type mismatch")
+ panic(v.panicMessage("bytes"))
}
}
@@ -264,37 +305,37 @@
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 @@
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
