v.io/core/veyron2/verror2: First step is making verror2.E be a struct
This CL makes verror2.E still be an interface (that implements "error"),
but removes from it all other methods.
Now, all operations are accessed via standalone functions in verror2.
Operations like Make() and Convert() now return an "error", as opposed to a verror2.E.
The intent is to make "error" be the only interface tyype that clients deal with.
There is a companion CL
https://vanadium-review.googlesource.com/#/c/3205/
which changes part of wspr to use error instead of verror2.E in a couple of places.
Tested with
cd release/go/src/v.io;
v23 go test ./...
v23 test run vanadium-go-test
Change-Id: I0154f12fab9e11a32e2c5a7374fcc7bfbc5aebe8
diff --git a/runtimes/google/ipc/client.go b/runtimes/google/ipc/client.go
index 15c86de..711fe33 100644
--- a/runtimes/google/ipc/client.go
+++ b/runtimes/google/ipc/client.go
@@ -18,7 +18,6 @@
"v.io/core/veyron2/options"
"v.io/core/veyron2/security"
"v.io/core/veyron2/vdl"
- old_verror "v.io/core/veyron2/verror"
verror "v.io/core/veyron2/verror2"
"v.io/core/veyron2/vlog"
"v.io/core/veyron2/vom"
@@ -165,7 +164,7 @@
return encrypted
}
-func (c *client) createFlow(ctx *context.T, ep naming.Endpoint, vcOpts []stream.VCOpt) (stream.Flow, verror.E) {
+func (c *client) createFlow(ctx *context.T, ep naming.Endpoint, vcOpts []stream.VCOpt) (stream.Flow, error) {
c.vcMapMu.Lock()
defer c.vcMapMu.Unlock()
if c.vcMap == nil {
@@ -312,7 +311,7 @@
return impetus
}
-// startCall ensures StartCall always returns verror.E.
+// startCall ensures StartCall always returns verror.Standard.
func (c *client) startCall(ctx *context.T, name, method string, args []interface{}, opts []ipc.CallOpt) (ipc.Call, error) {
if !ctx.Initialized() {
return nil, verror.ExplicitMake(verror.BadArg, i18n.NoLangID, "ipc.Client", "StartCall")
@@ -369,7 +368,7 @@
index int
suffix string
flow stream.Flow
- err verror.E
+ err error
}
// tryCreateFlow attempts to establish a Flow to "server" (which must be a
@@ -531,7 +530,7 @@
go cleanupTryCall(r, responses, ch)
fc, err := newFlowClient(ctx, serverB, r.flow, c.dc)
if err != nil {
- return nil, verror.NoRetry, err.(verror.E)
+ return nil, verror.NoRetry, err.(error)
}
if doneChan != nil {
@@ -592,7 +591,7 @@
// failedTryCall performs asynchronous cleanup for tryCall, and returns an
// appropriate error from the responses we've already received. All parallel
// calls in tryCall failed or we timed out if we get here.
-func (c *client) failedTryCall(ctx *context.T, name, method string, responses []*serverStatus, ch chan *serverStatus) (ipc.Call, verror.ActionCode, verror.E) {
+func (c *client) failedTryCall(ctx *context.T, name, method string, responses []*serverStatus, ch chan *serverStatus) (ipc.Call, verror.ActionCode, error) {
go cleanupTryCall(nil, responses, ch)
c.ns.FlushCacheEntry(name)
noconn, untrusted := []string{}, []string{}
@@ -628,7 +627,7 @@
// the RPC name.method for the client (local end of the flow). It returns the blessings at the
// server that are authorized for this purpose and any blessings that are to be granted to
// the server (via ipc.Granter implementations in opts.)
-func (c *client) authorizeServer(ctx *context.T, flow stream.Flow, name, method string, serverPattern security.BlessingPattern, opts []ipc.CallOpt) (serverBlessings []string, grantedBlessings security.Blessings, err verror.E) {
+func (c *client) authorizeServer(ctx *context.T, flow stream.Flow, name, method string, serverPattern security.BlessingPattern, opts []ipc.CallOpt) (serverBlessings []string, grantedBlessings security.Blessings, err error) {
if flow.RemoteBlessings() == nil {
return nil, nil, verror.Make(errNoBlessings, ctx)
}
@@ -868,8 +867,8 @@
return fc.closeSend()
}
-// closeSend ensures CloseSend always returns verror.E.
-func (fc *flowClient) closeSend() verror.E {
+// closeSend ensures CloseSend always returns verror.Standard.
+func (fc *flowClient) closeSend() error {
fc.sendClosedMu.Lock()
defer fc.sendClosedMu.Unlock()
if fc.sendClosed {
@@ -902,7 +901,7 @@
return err
}
-// finish ensures Finish always returns verror.E.
+// finish ensures Finish always returns a verror.Standard.
func (fc *flowClient) finish(resultptrs ...interface{}) error {
if fc.finished {
err := verror.Make(errClientFinishAlreadyCalled, fc.ctx)
@@ -945,7 +944,7 @@
if fc.response.Error != nil {
// TODO(cnicolaou): remove verror.NoAccess with verror version
// when ipc.Server is converted.
- if verror.Is(fc.response.Error, old_verror.NoAccess) && fc.dc != nil {
+ if verror.Is(fc.response.Error, verror.NoAccess.ID) && fc.dc != nil {
// In case the error was caused by a bad discharge, we do not want to get stuck
// with retrying again and again with this discharge. As there is no direct way
// to detect it, we conservatively flush all discharges we used from the cache.
diff --git a/runtimes/google/ipc/client_test.go b/runtimes/google/ipc/client_test.go
index 548db9a..e63551c 100644
--- a/runtimes/google/ipc/client_test.go
+++ b/runtimes/google/ipc/client_test.go
@@ -209,7 +209,7 @@
}
stack := ""
if err != nil {
- stack = err.(verror.E).Stack().String()
+ stack = verror.Stack(err).String()
}
t.Fatalf("%s: expecting one of: %v, got: %v: stack: %s", loc, verr, err, stack)
}
@@ -383,7 +383,7 @@
if err != nil {
if !verror.Is(err, verror.Timeout.ID) {
t.Fatalf("verror should be a timeout not %s: stack %s",
- err, err.(verror.E).Stack())
+ err, verror.Stack(err))
}
return
}
diff --git a/runtimes/google/ipc/full_test.go b/runtimes/google/ipc/full_test.go
index e40ece3..959c3fc 100644
--- a/runtimes/google/ipc/full_test.go
+++ b/runtimes/google/ipc/full_test.go
@@ -1053,7 +1053,7 @@
} else if err == nil && !test.authorized {
t.Errorf("%s call.Finish succeeded, expected authorization failure", name)
} else if !test.authorized && !verror.Is(err, verror.NoAccess.ID) {
- t.Errorf("%s. call.Finish returned error %v(%v), wanted %v", name, verror.Convert(verror.NoAccess, nil, err).ErrorID(), err, verror.NoAccess)
+ t.Errorf("%s. call.Finish returned error %v(%v), wanted %v", name, verror.ErrorID(verror.Convert(verror.NoAccess, nil, err)), err, verror.NoAccess)
}
}
}
@@ -1078,7 +1078,7 @@
if b.client, err = InternalNewClient(b.sm, b.ns, vc.LocalPrincipal{pclient}); err != nil {
t.Fatalf("InternalNewClient failed: %v", err)
}
- call := func() verror.E {
+ call := func() error {
call, err := b.client.StartCall(testContext(), "mountpoint/server/aclAuth", "Echo", []interface{}{"batman"})
if err != nil {
return err.(verror.E) //fmt.Errorf("client.StartCall failed: %v", err)
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index 11802cd..1cf87e2 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -53,7 +53,7 @@
// state for each requested proxy
type proxyState struct {
endpoint naming.Endpoint
- err verror.E
+ err error
}
type dhcpState struct {
@@ -1058,7 +1058,7 @@
argptrs, tags, err := invoker.Prepare(fs.method, numArgs)
fs.tags = tags
if err != nil {
- return nil, old_verror.Makef(old_verror.ErrorID(err), "%s: name: %q", err, fs.suffix)
+ return nil, old_verror.Makef(verror.ErrorID(err), "%s: name: %q", err, fs.suffix)
}
if len(argptrs) != numArgs {
return nil, old_verror.BadProtocolf(fmt.Sprintf("ipc: wrong number of input arguments for method %q, name %q (called with %d args, expected %d)", fs.method, fs.suffix, numArgs, len(argptrs)))
diff --git a/runtimes/google/ipc/stream/manager/listener.go b/runtimes/google/ipc/stream/manager/listener.go
index 632a428..5920fe2 100644
--- a/runtimes/google/ipc/stream/manager/listener.go
+++ b/runtimes/google/ipc/stream/manager/listener.go
@@ -15,7 +15,7 @@
"v.io/core/veyron2/ipc/stream"
"v.io/core/veyron2/naming"
- "v.io/core/veyron2/verror"
+ verror "v.io/core/veyron2/verror2"
"v.io/core/veyron2/vlog"
"v.io/core/veyron2/vom"
)
@@ -176,7 +176,7 @@
vc, err := vf.Dial(ln.proxyEP, dialOpts...)
if err != nil {
vf.StopAccepting()
- if verror.ErrorID(err) == verror.Aborted {
+ if verror.ErrorID(err) == verror.Aborted.ID {
ln.manager.vifs.Delete(vf)
}
return nil, nil, fmt.Errorf("VC establishment with proxy failed: %v", err)
diff --git a/runtimes/google/ipc/stream/manager/manager.go b/runtimes/google/ipc/stream/manager/manager.go
index 7b2b05c..bc57117 100644
--- a/runtimes/google/ipc/stream/manager/manager.go
+++ b/runtimes/google/ipc/stream/manager/manager.go
@@ -12,6 +12,7 @@
"v.io/core/veyron2/ipc/stream"
"v.io/core/veyron2/naming"
"v.io/core/veyron2/verror"
+ "v.io/core/veyron2/verror2"
"v.io/core/veyron2/vlog"
"v.io/core/veyron/lib/stats"
@@ -135,7 +136,7 @@
return nil, err
}
vc, err := vf.Dial(remote, append(opts, m.sessionCache)...)
- if !retry || verror.ErrorID(err) != verror.Aborted {
+ if !retry || verror2.ErrorID(err) != verror2.Aborted.ID {
return vc, err
}
vf.Close()
diff --git a/runtimes/google/naming/namespace/namespace.go b/runtimes/google/naming/namespace/namespace.go
index 92af50b..915b36d 100644
--- a/runtimes/google/naming/namespace/namespace.go
+++ b/runtimes/google/naming/namespace/namespace.go
@@ -10,6 +10,7 @@
"v.io/core/veyron2/naming"
"v.io/core/veyron2/security"
"v.io/core/veyron2/verror"
+ "v.io/core/veyron2/verror2"
"v.io/core/veyron2/vlog"
)
@@ -142,15 +143,15 @@
// notAnMT returns true if the error indicates this isn't a mounttable server.
func notAnMT(err error) bool {
- switch verror.ErrorID(err) {
- case verror.BadArg:
+ switch verror2.ErrorID(err) {
+ case verror2.BadArg.ID:
// This should cover "ipc: wrong number of in-args".
return true
- case verror.NoExist:
+ case verror2.NoExist.ID:
// This should cover "ipc: unknown method", "ipc: dispatcher not
// found", and dispatcher Lookup not found errors.
return true
- case verror.BadProtocol:
+ case verror2.BadProtocol.ID:
// This covers "ipc: response decoding failed: EOF".
return true
}
diff --git a/runtimes/google/rt/ipc_test.go b/runtimes/google/rt/ipc_test.go
index fb9febf..4e1a6f7 100644
--- a/runtimes/google/rt/ipc_test.go
+++ b/runtimes/google/rt/ipc_test.go
@@ -16,7 +16,7 @@
tsecurity "v.io/core/veyron/lib/testutil/security"
_ "v.io/core/veyron/profiles"
"v.io/core/veyron2/vdl"
- "v.io/core/veyron2/verror"
+ verror "v.io/core/veyron2/verror2"
)
func init() { testutil.Init() }
@@ -266,7 +266,7 @@
if err := pserver.BlessingStore().SetDefault(rootServerInvalidTPCaveat); err != nil {
t.Fatal(err)
}
- if call, err := client.StartCall(clientCtx, serverName, "EchoBlessings", nil); verror.Is(err, verror.NoAccess) {
+ if call, err := client.StartCall(clientCtx, serverName, "EchoBlessings", nil); verror.Is(err, verror.NoAccess.ID) {
remoteBlessings, _ := call.RemoteBlessings()
t.Errorf("client.StartCall passed unexpectedly with remote end authenticated as: %v", remoteBlessings)
}
diff --git a/security/agent/agent_test.go b/security/agent/agent_test.go
index 0c9005e..89f1516 100644
--- a/security/agent/agent_test.go
+++ b/security/agent/agent_test.go
@@ -36,7 +36,7 @@
Method string
Args V
Result interface{} // Result returned by the Method call.
- Error verror2.E // If Error is not nil will be compared to the last result.
+ Error error // If Error is not nil will be compared to the last result.
}
const pkgPath = "v.io/core/veyron/security/agent/"
@@ -110,7 +110,7 @@
}
// We only set the error value when error is the only output to ensure the real function gets called.
if test.Error != nil {
- if got := results[len(results)-1]; got == nil || !verror2.Is(got.(error), test.Error.ErrorID()) {
+ if got := results[len(results)-1]; got == nil || !verror2.Is(got.(error), verror2.ErrorID(test.Error)) {
t.Errorf("p.%v(%#v) returned an incorrect error: %v, expected %v", test.Method, test.Args, got, test.Error)
}
if len(results) == 1 {
diff --git a/services/mgmt/application/impl/acl_test.go b/services/mgmt/application/impl/acl_test.go
index 2f5b710..fdf1713 100644
--- a/services/mgmt/application/impl/acl_test.go
+++ b/services/mgmt/application/impl/acl_test.go
@@ -14,7 +14,7 @@
"v.io/core/veyron2/security"
"v.io/core/veyron2/services/mgmt/application"
"v.io/core/veyron2/services/security/access"
- "v.io/core/veyron2/verror"
+ verror "v.io/core/veyron2/verror2"
"v.io/core/veyron2/vlog"
"v.io/core/veyron/lib/modules"
diff --git a/services/mgmt/application/impl/dispatcher.go b/services/mgmt/application/impl/dispatcher.go
index b08cddd..13b2478 100644
--- a/services/mgmt/application/impl/dispatcher.go
+++ b/services/mgmt/application/impl/dispatcher.go
@@ -7,7 +7,7 @@
"v.io/core/veyron2/naming"
"v.io/core/veyron2/security"
"v.io/core/veyron2/services/security/access"
- "v.io/core/veyron2/verror"
+ verror "v.io/core/veyron2/verror2"
"v.io/core/veyron2/vlog"
"v.io/core/veyron/security/flag"
diff --git a/services/mgmt/binary/impl/acl_test.go b/services/mgmt/binary/impl/acl_test.go
index e599972..a45811d 100644
--- a/services/mgmt/binary/impl/acl_test.go
+++ b/services/mgmt/binary/impl/acl_test.go
@@ -13,7 +13,7 @@
"v.io/core/veyron2/security"
"v.io/core/veyron2/services/mgmt/repository"
"v.io/core/veyron2/services/security/access"
- "v.io/core/veyron2/verror"
+ verror "v.io/core/veyron2/verror2"
"v.io/core/veyron2/vlog"
"v.io/core/veyron/lib/modules"
@@ -127,11 +127,11 @@
vlog.VI(2).Infof("Verify that in the beginning other can't access bini/private or bini/shared")
binary = repository.BinaryClient("bini/private")
- if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess) {
+ if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess.ID) {
t.Fatalf("Stat() should have failed but didn't: %v", err)
}
binary = repository.BinaryClient("bini/shared")
- if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess) {
+ if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess.ID) {
t.Fatalf("Stat() should have failed but didn't: %v", err)
}
@@ -191,7 +191,7 @@
// can't accidentally expose the server without setting a root ACL.
vlog.VI(2).Infof(" Verify that other still can't access bini/shared.")
binary = repository.BinaryClient("bini/shared")
- if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess) {
+ if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess.ID) {
t.Fatalf("Stat() should have failed but didn't: %v", err)
}
@@ -211,7 +211,7 @@
t.Fatalf("Stat() shouldn't have failed: %v", err)
}
binary = repository.BinaryClient("bini/private")
- if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess) {
+ if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess.ID) {
t.Fatalf("Stat() should have failed but didn't: %v", err)
}
@@ -293,18 +293,18 @@
vlog.VI(2).Infof("And now other can do nothing. Other should be penitent.")
binary = repository.BinaryClient("bini/nototherbinary")
- if err := binary.Create(otherCtx, 1, repository.MediaInfo{Type: "application/octet-stream"}); !verror.Is(err, verror.NoAccess) {
+ if err := binary.Create(otherCtx, 1, repository.MediaInfo{Type: "application/octet-stream"}); !verror.Is(err, verror.NoAccess.ID) {
t.Fatalf("Create() should have failed %v", err)
}
binary = repository.BinaryClient("bini/shared")
- if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess) {
+ if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess.ID) {
t.Fatalf("Stat() should have failed but didn't: %v", err)
}
vlog.VI(2).Infof("Pennance includes no access to the binary that other made.")
binary = repository.BinaryClient("bini/otherbinary")
- if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess) {
+ if _, _, err := binary.Stat(otherCtx); !verror.Is(err, verror.NoAccess.ID) {
t.Fatalf("Stat() should have failed but didn't: %v", err)
}
}
