profiles/internal/ipc: Pass more errors to the client
The change makes more Glob errors visible to the client.
MultiPart: 1/2
Change-Id: I127f1983a4ef7a8e81720085ec9b2aa3037fdf4d
diff --git a/profiles/internal/ipc/glob_test.go b/profiles/internal/ipc/glob_test.go
index a829c68..28c88b3 100644
--- a/profiles/internal/ipc/glob_test.go
+++ b/profiles/internal/ipc/glob_test.go
@@ -11,6 +11,7 @@
"v.io/v23/context"
"v.io/v23/i18n"
"v.io/v23/ipc"
+ "v.io/v23/ipc/reserved"
"v.io/v23/naming"
"v.io/v23/security"
"v.io/v23/verror"
@@ -59,6 +60,12 @@
}
defer stop()
+ var (
+ noExist = verror.New(verror.ErrNoExist, ctx, "")
+ notImplemented = ipc.NewErrGlobNotImplemented(ctx, "")
+ maxRecursion = reserved.NewErrGlobMaxRecursionReached(ctx)
+ )
+
testcases := []struct {
name, pattern string
expected []string
@@ -78,7 +85,7 @@
"a/x/y",
"a/x/y/z",
"leaf",
- }, []naming.GlobError{{Name: "leaf", Error: ipc.NewErrGlobNotImplemented(ctx, "leaf")}}},
+ }, []naming.GlobError{{Name: "leaf", Error: notImplemented}}},
{"a", "...", []string{
"",
"b",
@@ -122,7 +129,7 @@
"",
}, nil},
{"", "", []string{""}, nil},
- {"", "*", []string{"a", "leaf"}, []naming.GlobError{{Name: "leaf", Error: ipc.NewErrGlobNotImplemented(ctx, "leaf")}}},
+ {"", "*", []string{"a", "leaf"}, []naming.GlobError{{Name: "leaf", Error: notImplemented}}},
{"a", "", []string{""}, nil},
{"a", "*", []string{"b", "x"}, nil},
{"a/b", "", []string{""}, nil},
@@ -135,13 +142,16 @@
{"a", "*/*", []string{"b/c1", "b/c2", "x/y"}, nil},
{"a", "*/*/*", []string{"b/c1/d1", "b/c1/d2", "b/c2/d1", "b/c2/d2", "x/y/z"}, nil},
{"a/x", "*/*", []string{"y/z"}, nil},
- {"bad", "", []string{}, nil},
- {"a/bad", "", []string{}, nil},
- {"a/b/bad", "", []string{}, nil},
- {"a/b/c1/bad", "", []string{}, nil},
- {"a/x/bad", "", []string{}, nil},
- {"a/x/y/bad", "", []string{}, nil},
- {"leaf", "", []string{""}, []naming.GlobError{{Name: "", Error: ipc.NewErrGlobNotImplemented(ctx, "")}}},
+ {"bad", "", []string{}, []naming.GlobError{{Name: "", Error: noExist}}},
+ {"bad/foo", "", []string{}, []naming.GlobError{{Name: "", Error: noExist}}},
+ {"a/bad", "", []string{}, []naming.GlobError{{Name: "", Error: noExist}}},
+ {"a/b/bad", "", []string{}, []naming.GlobError{{Name: "", Error: noExist}}},
+ {"a/b/c1/bad", "", []string{}, []naming.GlobError{{Name: "", Error: noExist}}},
+ {"a/x/bad", "", []string{}, []naming.GlobError{{Name: "", Error: noExist}}},
+ {"a/x/y/bad", "", []string{}, []naming.GlobError{{Name: "", Error: noExist}}},
+ {"leaf", "", []string{""}, []naming.GlobError{{Name: "", Error: notImplemented}}},
+ {"leaf", "*", []string{}, []naming.GlobError{{Name: "", Error: notImplemented}}},
+ {"leaf/foo", "", []string{}, []naming.GlobError{{Name: "", Error: noExist}}},
// muah is an infinite space to test rescursion limit.
{"muah", "*", []string{"ha"}, nil},
{"muah", "*/*", []string{"ha/ha"}, nil},
@@ -158,7 +168,7 @@
"ha/ha/ha/ha/ha/ha/ha/ha",
"ha/ha/ha/ha/ha/ha/ha/ha/ha",
"ha/ha/ha/ha/ha/ha/ha/ha/ha/ha",
- }, nil},
+ }, []naming.GlobError{{Name: "ha/ha/ha/ha/ha/ha/ha/ha/ha/ha/ha", Error: maxRecursion}}},
}
for _, tc := range testcases {
name := naming.JoinAddressName(ep, tc.name)
@@ -171,11 +181,15 @@
t.Errorf("unexpected result for (%q, %q). Got %q, want %q", tc.name, tc.pattern, results, tc.expected)
}
if len(globErrors) != len(tc.errors) {
- t.Errorf("unexpected number of glob errors for (%q, %q): %v", tc.name, tc.pattern, globErrors)
+ t.Errorf("unexpected number of glob errors for (%q, %q): got %#v, expected %#v", tc.name, tc.pattern, globErrors, tc.errors)
}
for i, e := range globErrors {
+ if i >= len(tc.errors) {
+ t.Errorf("unexpected glob error for (%q, %q): %#v", tc.name, tc.pattern, e.Error)
+ continue
+ }
if e.Name != tc.errors[i].Name {
- t.Errorf("unexpected glob errors for (%q, %q): %v", tc.name, tc.pattern, e)
+ t.Errorf("unexpected glob error for (%q, %q): %v", tc.name, tc.pattern, e)
}
if got, expected := verror.ErrorID(e.Error), verror.ErrorID(tc.errors[i].Error); got != expected {
t.Errorf("unexpected error ID for (%q, %q): Got %v, expected %v", tc.name, tc.pattern, got, expected)
@@ -261,7 +275,7 @@
return ipc.ChildrenGlobberInvoker("ha"), auth, nil
}
- if len(elems) != 0 && elems[0] == "leaf" {
+ if len(elems) != 0 && elems[len(elems)-1] == "leaf" {
return leafObject{}, auth, nil
}
if len(elems) < 2 || (elems[0] == "a" && elems[1] == "x") {
@@ -288,7 +302,7 @@
}
n := o.n.find(o.suffix, false)
if n == nil {
- return nil, nil
+ return nil, verror.New(verror.ErrNoExist, call.Context(), o.suffix)
}
ch := make(chan naming.VDLGlobReply)
go func() {
@@ -317,10 +331,10 @@
suffix []string
}
-func (o *vChildrenObject) GlobChildren__(ipc.ServerCall) (<-chan string, error) {
+func (o *vChildrenObject) GlobChildren__(call ipc.ServerCall) (<-chan string, error) {
n := o.n.find(o.suffix, false)
if n == nil {
- return nil, fmt.Errorf("object does not exist")
+ return nil, verror.New(verror.ErrNoExist, call.Context(), o.suffix)
}
ch := make(chan string, len(n.children))
for child, _ := range n.children {
diff --git a/profiles/internal/ipc/reserved.go b/profiles/internal/ipc/reserved.go
index 61f1e7b..aa9cbdb 100644
--- a/profiles/internal/ipc/reserved.go
+++ b/profiles/internal/ipc/reserved.go
@@ -6,11 +6,13 @@
"v.io/v23/context"
"v.io/v23/ipc"
+ "v.io/v23/ipc/reserved"
"v.io/v23/naming"
"v.io/v23/security"
"v.io/v23/services/security/access"
"v.io/v23/vdl"
"v.io/v23/vdlroot/signature"
+ "v.io/v23/verror"
"v.io/x/lib/vlog"
"v.io/x/ref/lib/glob"
@@ -225,15 +227,24 @@
call.M.Suffix = naming.Join(i.receiver, state.name)
if state.depth > maxRecursiveGlobDepth {
vlog.Errorf("ipc Glob: exceeded recursion limit (%d): %q", maxRecursiveGlobDepth, call.Suffix())
+ call.Send(naming.VDLGlobReplyError{
+ naming.GlobError{Name: state.name, Error: reserved.NewErrGlobMaxRecursionReached(call.Context())},
+ })
continue
}
obj, auth, err := disp.Lookup(call.Suffix())
if err != nil {
vlog.VI(3).Infof("ipc Glob: Lookup failed for %q: %v", call.Suffix(), err)
+ call.Send(naming.VDLGlobReplyError{
+ naming.GlobError{Name: state.name, Error: verror.Convert(verror.ErrNoExist, call.Context(), err)},
+ })
continue
}
if obj == nil {
vlog.VI(3).Infof("ipc Glob: object not found for %q", call.Suffix())
+ call.Send(naming.VDLGlobReplyError{
+ naming.GlobError{Name: state.name, Error: verror.New(verror.ErrNoExist, call.Context(), "nil object")},
+ })
continue
}
@@ -249,6 +260,9 @@
invoker, err := objectToInvoker(obj)
if err != nil {
vlog.VI(3).Infof("ipc Glob: object for %q cannot be converted to invoker: %v", call.Suffix(), err)
+ call.Send(naming.VDLGlobReplyError{
+ naming.GlobError{Name: state.name, Error: verror.Convert(verror.ErrInternal, call.Context(), err)},
+ })
continue
}
gs := invoker.Globber()
@@ -266,6 +280,7 @@
ch, err := gs.AllGlobber.Glob__(call, state.glob.String())
if err != nil {
vlog.VI(3).Infof("ipc Glob: %q.Glob(%q) failed: %v", call.Suffix(), state.glob, err)
+ call.Send(naming.VDLGlobReplyError{naming.GlobError{Name: state.name, Error: verror.Convert(verror.ErrInternal, call.Context(), err)}})
continue
}
if ch == nil {
@@ -287,6 +302,7 @@
children, err := gs.ChildrenGlobber.GlobChildren__(call)
// The requested object doesn't exist.
if err != nil {
+ call.Send(naming.VDLGlobReplyError{naming.GlobError{Name: state.name, Error: verror.Convert(verror.ErrInternal, call.Context(), err)}})
continue
}
// The glob pattern matches the current object.
