TBR wspr: Change IPC protocol to remove error from out-args.
This is related to:
https://vanadium-review.googlesource.com/#/c/4751/
The changes in wspr aren't just trivial VDL generation. Luckily
Benj had the foresight to add TODO comments around most places
where we were hacking things by adding / removing the final error
out-arg. So I've just found these places and removed the hacks.
AFAICT the javascript code itself doesn't need to change, since
Benj had already set things up assuming that we'd make this
change at some point. All go tests pass, but I'm just going to
wait for the presubmit to make sure the JS side really passes.
MultiPart: 2/8
Change-Id: Id888b4e86f290bd8931de821c668f4b8829b6c5f
diff --git a/services/wsprd/account/account.go b/services/wsprd/account/account.go
index 994bb4a..49bd1a0 100644
--- a/services/wsprd/account/account.go
+++ b/services/wsprd/account/account.go
@@ -28,11 +28,11 @@
return
}
var email string
- if ierr := call.Finish(&blessingObj, &email, &err); ierr != nil {
- err = ierr
+ if err := call.Finish(&blessingObj, &email); err != nil {
+ return security.WireBlessings{}, "", err
}
serverBlessings, _ := call.RemoteBlessings()
- return blessingObj, accountName(serverBlessings, email), err
+ return blessingObj, accountName(serverBlessings, email), nil
}
func accountName(serverBlessings []string, email string) string {
diff --git a/services/wsprd/app/app.go b/services/wsprd/app/app.go
index 9d50c87..05f30ac 100644
--- a/services/wsprd/app/app.go
+++ b/services/wsprd/app/app.go
@@ -168,13 +168,9 @@
w.Error(verror.New(marshallingError, ctx, "ResponseStreamClose"))
}
}
-
- // TODO(bprosnitz) Remove this when we remove error from out args everywhere.
- numOutArgsWithError := msg.NumOutArgs + 1
-
- results := make([]interface{}, numOutArgsWithError)
+ results := make([]interface{}, msg.NumOutArgs)
// This array will have pointers to the values in result.
- resultptrs := make([]interface{}, numOutArgsWithError)
+ resultptrs := make([]interface{}, msg.NumOutArgs)
for ax := range results {
resultptrs[ax] = &results[ax]
}
@@ -187,14 +183,7 @@
}
func (c *Controller) sendRPCResponse(ctx *context.T, w lib.ClientWriter, span vtrace.Span, results []interface{}) {
- // for now we assume last out argument is always error
- if err, ok := results[len(results)-1].(error); ok {
- // return the call Application error as is
- w.Error(err)
- return
- }
-
- outargs := make([]vdl.AnyRep, len(results)-1)
+ outargs := make([]vdl.AnyRep, len(results))
for i := range outargs {
outargs[i] = results[i]
}
diff --git a/services/wsprd/app/app_test.go b/services/wsprd/app/app_test.go
index f7a13ad..23d7ee4 100644
--- a/services/wsprd/app/app_test.go
+++ b/services/wsprd/app/app_test.go
@@ -546,29 +546,24 @@
}
var result interface{}
- var err2 error
+ err = call.Finish(&result)
- err = call.Finish(&result, &err2)
-
- // If err is nil and test.authErr is nil reflect.DeepEqual will return
- // false because the types are different. Because of this, we only use
- // reflect.DeepEqual if one of the values is non-nil. If both values
- // are nil, then we consider them equal.
- if (err != nil || test.authError != nil) && !verror.Equal(err, test.authError) {
- t.Errorf("unexpected err: got %#v, expected %#v", err, test.authError)
- }
-
- if err != nil {
- return
+ // Make sure the err matches either test.authError or test.err.
+ if want := test.authError; want != nil {
+ if !verror.Equal(err, want) {
+ t.Errorf("didn't match test.authError: got %#v, want %#v", err, want)
+ }
+ } else if want := test.err; want != nil {
+ if !verror.Equal(err, want) {
+ t.Errorf("didn't match test.err: got %#v, want %#v", err, want)
+ }
+ } else if err != nil {
+ t.Errorf("unexpected error: got %#v, want nil", err)
}
if !reflect.DeepEqual(result, test.finalResponse) {
t.Errorf("unexected final response: got %v, expected %v", result, test.finalResponse)
}
-
- if (err2 != nil || test.err != nil) && !verror.Equal(err2, test.err) {
- t.Errorf("unexpected error: got %#v, expected %#v", err2, test.err)
- }
}
func TestSimpleJSServer(t *testing.T) {
@@ -602,7 +597,6 @@
runJsServerTestCase(t, jsServerTestCase{
method: "Add",
inArgs: []interface{}{int32(1), int32(2)},
- finalResponse: int32(3),
hasAuthorizer: true,
authError: err,
})
diff --git a/services/wsprd/app/controller.vdl.go b/services/wsprd/app/controller.vdl.go
index 73607db..65cce72 100644
--- a/services/wsprd/app/controller.vdl.go
+++ b/services/wsprd/app/controller.vdl.go
@@ -50,9 +50,7 @@
if call, err = c.c(ctx).StartCall(ctx, c.name, "Serve", []interface{}{i0, i1}, opts...); err != nil {
return
}
- if ierr := call.Finish(&err); ierr != nil {
- err = ierr
- }
+ err = call.Finish()
return
}
@@ -123,9 +121,6 @@
{"name", ``}, // string
{"serverId", ``}, // uint32
},
- OutArgs: []ipc.ArgDesc{
- {"", ``}, // error
- },
},
},
}
diff --git a/services/wsprd/ipc/server/dispatcher.go b/services/wsprd/ipc/server/dispatcher.go
index 3bd0392..99f7185 100644
--- a/services/wsprd/ipc/server/dispatcher.go
+++ b/services/wsprd/ipc/server/dispatcher.go
@@ -10,7 +10,6 @@
"v.io/core/veyron2/ipc"
"v.io/core/veyron2/security"
- "v.io/core/veyron2/vdl"
"v.io/core/veyron2/vdl/vdlroot/src/signature"
"v.io/core/veyron2/verror"
"v.io/core/veyron2/vlog"
@@ -156,18 +155,6 @@
err2 := verror.Convert(verror.ErrInternal, nil, err).(verror.Standard)
reply.Err = &err2
}
-
- // TODO(bprosnitz) Remove this when error is removed from signature everywhere.
- // Add the error out arg into the signature received from JS (which will alawya lack the error arg).
- for s, _ := range reply.Signature {
- for m, _ := range reply.Signature[s].Methods {
- errArg := signature.Arg{
- Name: "err",
- Type: vdl.ErrorType,
- }
- reply.Signature[s].Methods[m].OutArgs = append(reply.Signature[s].Methods[m].OutArgs, errArg)
- }
- }
}
ch <- reply
}
diff --git a/services/wsprd/ipc/server/invoker.go b/services/wsprd/ipc/server/invoker.go
index 7e1462d..0938785 100644
--- a/services/wsprd/ipc/server/invoker.go
+++ b/services/wsprd/ipc/server/invoker.go
@@ -67,20 +67,15 @@
// Wait for the result
reply := <-replychan
- var err error
if reply.Err != nil {
- err = reply.Err
+ return nil, reply.Err
}
// Convert the reply.Results from []vdl.AnyRep to []interface{}
- results := make([]interface{}, len(reply.Results)+1)
+ results := make([]interface{}, len(reply.Results))
for i, r := range reply.Results {
results[i] = interface{}(r)
}
-
- // Add error as last out arg.
- // TODO(bprosnitz) Remove this when we stop sending error in out args in ipc.
- results[len(reply.Results)] = err
return results, nil
}
diff --git a/services/wsprd/lib/signature_manager.go b/services/wsprd/lib/signature_manager.go
index b59f8b8..76177a3 100644
--- a/services/wsprd/lib/signature_manager.go
+++ b/services/wsprd/lib/signature_manager.go
@@ -111,14 +111,6 @@
return nil, verror.New(verror.ErrNoServers, ctx, name, err)
}
- // TODO(bprosnitz) Remove this when error is removed from signature everywhere.
- // Remove the error out arg from the signature received on the wire so that it matches the format expected by JS.
- for s, _ := range sig {
- for m, _ := range sig[s].Methods {
- sig[s].Methods[m].OutArgs = sig[s].Methods[m].OutArgs[:len(sig[s].Methods[m].OutArgs)-1]
- }
- }
-
// Add to the cache.
sm.cache[name] = &cacheEntry{
sig: sig,
diff --git a/services/wsprd/lib/signature_manager_test.go b/services/wsprd/lib/signature_manager_test.go
index 4efac95..5f35f9e 100644
--- a/services/wsprd/lib/signature_manager_test.go
+++ b/services/wsprd/lib/signature_manager_test.go
@@ -25,16 +25,15 @@
{
Methods: []signature.Method{
{
- Name: "Method1",
- InArgs: []signature.Arg{{Type: vdl.StringType}},
- OutArgs: []signature.Arg{{Type: vdl.ErrorType}},
+ Name: "Method1",
+ InArgs: []signature.Arg{{Type: vdl.StringType}},
},
},
},
}
client := mocks_ipc.NewSimpleClient(
map[string][]interface{}{
- "__Signature": []interface{}{initialSig, nil},
+ "__Signature": []interface{}{initialSig},
},
)
ctx = fake.SetClient(ctx, client)
@@ -56,9 +55,8 @@
{
Methods: []signature.Method{
{
- Name: "Method1",
- InArgs: []signature.Arg{{Type: vdl.StringType}},
- OutArgs: []signature.Arg{},
+ Name: "Method1",
+ InArgs: []signature.Arg{{Type: vdl.StringType}},
},
},
},
