proto, internal/errors: add Error sentinel, errors.Wrap
Add a sentinel proto.Error error which matches all errors returned by
packages in this module.
Document that protoregistry.NotFound is an exact sentinel value for
performance reasons.
Add a Wrap function to the internal/errors package and use it to wrap
errors from outside sources (resolvers). Wrapped errors match
proto.Error.
Fixes golang/protobuf#1021.
Change-Id: I45567df3fd6c8dc9a5caafdb55654827f6fb1941
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215338
Reviewed-by: Joe Tsai <joetsai@google.com>
diff --git a/internal/encoding/messageset/messageset.go b/internal/encoding/messageset/messageset.go
index 837e5c4..9d22dd7 100644
--- a/internal/encoding/messageset/messageset.go
+++ b/internal/encoding/messageset/messageset.go
@@ -61,9 +61,13 @@
// In text and JSON formats, the extension name used is the message itself.
// The extension field name is derived by appending ExtensionName.
func FindMessageSetExtension(r preg.ExtensionTypeResolver, s pref.FullName) (pref.ExtensionType, error) {
- xt, err := r.FindExtensionByName(s.Append(ExtensionName))
+ name := s.Append(ExtensionName)
+ xt, err := r.FindExtensionByName(name)
if err != nil {
- return nil, err
+ if err == preg.NotFound {
+ return nil, err
+ }
+ return nil, errors.Wrap(err, "%q", name)
}
if !IsMessageSetExtension(xt.TypeDescriptor()) {
return nil, preg.NotFound
diff --git a/internal/errors/errors.go b/internal/errors/errors.go
index bc495b4..20c17b3 100644
--- a/internal/errors/errors.go
+++ b/internal/errors/errors.go
@@ -6,20 +6,19 @@
package errors
import (
+ "errors"
"fmt"
"google.golang.org/protobuf/internal/detrand"
)
+// Error is a sentinel matching all errors produced by this package.
+var Error = errors.New("protobuf error")
+
// New formats a string according to the format specifier and arguments and
// returns an error that has a "proto" prefix.
func New(f string, x ...interface{}) error {
- for i := 0; i < len(x); i++ {
- if e, ok := x[i].(*prefixError); ok {
- x[i] = e.s // avoid "proto: " prefix when chaining
- }
- }
- return &prefixError{s: fmt.Sprintf(f, x...)}
+ return &prefixError{s: format(f, x...)}
}
type prefixError struct{ s string }
@@ -38,6 +37,49 @@
return prefix + e.s
}
+func (e *prefixError) Unwrap() error {
+ return Error
+}
+
+// Wrap returns an error that has a "proto" prefix, the formatted string described
+// by the format specifier and arguments, and a suffix of err. The error wraps err.
+func Wrap(err error, f string, x ...interface{}) error {
+ return &wrapError{
+ s: format(f, x...),
+ err: err,
+ }
+}
+
+type wrapError struct {
+ s string
+ err error
+}
+
+func (e *wrapError) Error() string {
+ return format("%v%v: %v", prefix, e.s, e.err)
+}
+
+func (e *wrapError) Unwrap() error {
+ return e.err
+}
+
+func (e *wrapError) Is(target error) bool {
+ return target == Error
+}
+
+func format(f string, x ...interface{}) string {
+ // avoid "proto: " prefix when chaining
+ for i := 0; i < len(x); i++ {
+ switch e := x[i].(type) {
+ case *prefixError:
+ x[i] = e.s
+ case *wrapError:
+ x[i] = format("%v: %v", e.s, e.err)
+ }
+ }
+ return fmt.Sprintf(f, x...)
+}
+
func InvalidUTF8(name string) error {
return New("field %v contains invalid UTF-8", name)
}
diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go
index b489edd..804eb96 100644
--- a/internal/errors/errors_test.go
+++ b/internal/errors/errors_test.go
@@ -5,27 +5,63 @@
package errors
import (
+ "errors"
"strings"
"testing"
)
-func TestNewPrefix(t *testing.T) {
- e1 := New("abc")
- got := e1.Error()
- if !strings.HasPrefix(got, "proto:") {
- t.Errorf("missing \"proto:\" prefix in %q", got)
- }
- if !strings.Contains(got, "abc") {
- t.Errorf("missing text \"abc\" in %q", got)
- }
-
- e2 := New("%v", e1)
- got = e2.Error()
- if !strings.HasPrefix(got, "proto:") {
- t.Errorf("missing \"proto:\" prefix in %q", got)
- }
- // Test to make sure prefix is removed from the embedded error.
- if strings.Contains(strings.TrimPrefix(got, "proto:"), "proto:") {
- t.Errorf("prefix \"proto:\" not elided in embedded error: %q", got)
+func TestErrors(t *testing.T) {
+ var sentinel = New("sentinel")
+ var foreign = errors.New("foreign")
+ for _, test := range []struct {
+ what string
+ err error
+ wantText string
+ is []error
+ isNot []error
+ }{{
+ what: `New("abc")`,
+ err: New("abc"),
+ wantText: "abc",
+ }, {
+ what: `New("%v", sentinel)`,
+ err: New("%v", sentinel),
+ wantText: "sentinel",
+ isNot: []error{sentinel},
+ }, {
+ what: `Wrap(sentinel, "%v", "text")`,
+ err: Wrap(sentinel, "%v", "text"),
+ wantText: "text: sentinel",
+ is: []error{sentinel},
+ }, {
+ what: `New("%v", foreign)`,
+ err: New("%v", foreign),
+ wantText: "foreign",
+ isNot: []error{foreign},
+ }, {
+ what: `Wrap(foreign, "%v", "text")`,
+ err: Wrap(foreign, "%v", "text"),
+ wantText: "text: foreign",
+ is: []error{foreign},
+ }} {
+ if got, want := test.err.Error(), prefix; !strings.HasPrefix(got, want) {
+ t.Errorf("%v.Error() = %q, want prefix %q", test.what, got, want)
+ }
+ if got, want := test.err.Error(), prefix+test.wantText; got != want {
+ t.Errorf("%v.Error() = %q, want %q", test.what, got, want)
+ }
+ if got, want := Is(test.err, Error), true; got != want {
+ t.Errorf("errors.Is(%v, errors.Error) = %v, want %v", test.what, got, want)
+ }
+ for _, err := range test.is {
+ if got, want := Is(test.err, err), true; got != want {
+ t.Errorf("errors.Is(%v, %v) = %v, want %v", test.what, err, got, want)
+ }
+ }
+ for _, err := range test.isNot {
+ if got, want := Is(test.err, err), false; got != want {
+ t.Errorf("errors.Is(%v, %v) = %v, want %v", test.what, err, got, want)
+ }
+ }
}
}
diff --git a/internal/errors/is_go112.go b/internal/errors/is_go112.go
new file mode 100644
index 0000000..0f5e585
--- /dev/null
+++ b/internal/errors/is_go112.go
@@ -0,0 +1,42 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build !go1.13
+
+package errors
+
+import "reflect"
+
+// Is is a copy of Go 1.13's errors.Is for use with older Go versions.
+func Is(err, target error) bool {
+ if target == nil {
+ return err == target
+ }
+
+ isComparable := reflect.TypeOf(target).Comparable()
+ for {
+ if isComparable && err == target {
+ return true
+ }
+ if x, ok := err.(interface{ Is(error) bool }); ok && x.Is(target) {
+ return true
+ }
+ // TODO: consider supporing target.Is(err). This would allow
+ // user-definable predicates, but also may allow for coping with sloppy
+ // APIs, thereby making it easier to get away with them.
+ if err = unwrap(err); err == nil {
+ return false
+ }
+ }
+}
+
+func unwrap(err error) error {
+ u, ok := err.(interface {
+ Unwrap() error
+ })
+ if !ok {
+ return nil
+ }
+ return u.Unwrap()
+}
diff --git a/internal/errors/is_go113.go b/internal/errors/is_go113.go
new file mode 100644
index 0000000..dc05f41
--- /dev/null
+++ b/internal/errors/is_go113.go
@@ -0,0 +1,12 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// +build go1.13
+
+package errors
+
+import "errors"
+
+// Is is errors.Is.
+func Is(err, target error) bool { return errors.Is(err, target) }
diff --git a/internal/impl/decode.go b/internal/impl/decode.go
index a2993d4..6e78faa 100644
--- a/internal/impl/decode.go
+++ b/internal/impl/decode.go
@@ -189,7 +189,7 @@
if err == preg.NotFound {
return out, errUnknown
}
- return out, err
+ return out, errors.New("%v: unable to resolve extension %v: %v", mi.Desc.FullName(), num, err)
}
}
xi := getExtensionFieldInfo(xt)
diff --git a/proto/decode.go b/proto/decode.go
index d8b9187..b712786 100644
--- a/proto/decode.go
+++ b/proto/decode.go
@@ -124,7 +124,7 @@
if fd == nil && md.ExtensionRanges().Has(num) {
extType, err := o.Resolver.FindExtensionByNumber(md.FullName(), num)
if err != nil && err != protoregistry.NotFound {
- return err
+ return errors.New("%v: unable to resolve extension %v: %v", md.FullName(), num, err)
}
if extType != nil {
fd = extType.TypeDescriptor()
diff --git a/proto/messageset.go b/proto/messageset.go
index e27e0b7..f9af2ad 100644
--- a/proto/messageset.go
+++ b/proto/messageset.go
@@ -78,7 +78,7 @@
return errUnknown
}
if err != nil {
- return err
+ return errors.New("%v: unable to resolve extension %v: %v", md.FullName(), num, err)
}
xd := xt.TypeDescriptor()
if err := o.unmarshalMessage(v, m.Mutable(xd).Message()); err != nil {
diff --git a/proto/proto.go b/proto/proto.go
index 983fc71..1fec109 100644
--- a/proto/proto.go
+++ b/proto/proto.go
@@ -5,8 +5,19 @@
package proto
import (
+ "google.golang.org/protobuf/internal/errors"
"google.golang.org/protobuf/reflect/protoreflect"
)
// Message is the top-level interface that all messages must implement.
type Message = protoreflect.ProtoMessage
+
+// Error matches all errors produced by packages in the protobuf module.
+//
+// That is, errors.Is(err, Error) reports whether an error is produced
+// by this module.
+var Error error
+
+func init() {
+ Error = errors.Error
+}
diff --git a/reflect/protodesc/desc_resolve.go b/reflect/protodesc/desc_resolve.go
index 1bf4799..cebb36c 100644
--- a/reflect/protodesc/desc_resolve.go
+++ b/reflect/protodesc/desc_resolve.go
@@ -187,7 +187,7 @@
}
foundButNotImported = d
} else if err != protoregistry.NotFound {
- return nil, err
+ return nil, errors.Wrap(err, "%q", s)
}
// Continue on at a higher level of scoping.
diff --git a/reflect/protoregistry/registry.go b/reflect/protoregistry/registry.go
index c77f77a..8385c75 100644
--- a/reflect/protoregistry/registry.go
+++ b/reflect/protoregistry/registry.go
@@ -47,6 +47,9 @@
var GlobalTypes *Types = new(Types)
// NotFound is a sentinel error value to indicate that the type was not found.
+//
+// Since registry lookup can happen in the critical performance path, resolvers
+// must return this exact error value, not an error wrapping it.
var NotFound = errors.New("not found")
// Files is a registry for looking up or iterating over files and the
