TBR go.core: Change IPC protocol to remove error from out-args.
Vanadium IPC method calls look like this in the VDL:
type Arith interface {
Add(x, y int32) (int32 | error)
}
The purpose of the "int32 | error" syntax is to try to make it
clear(er) that you either get the out-args, or the error, but
never both. Note that when we generate code in languages that
use exceptions (e.g. Java, and sometimes in Javascript), there's
no way to return both the out-args and the error; the error is
thrown as an exception, and we can't collect the out-args.
Prior to this CL, we were still sending the error as the last
out-arg on the wire. This was purely legacy behavior, that we
never got around to cleaning up during the vom2 transition,
because it's a huge change that's really painful. This CL is the
huge painful change to clean things up again.
It's hard to separate out the changes into smaller CLs, so this
is an 8-part CL, with lots of changes. The main changes are in
this CL, which modifies the release.go.core repo. Here's a
summary of the changes:
a) VDL parser no longer returns error as the last out-arg.
b) VDL compiler no longer returns error as the last out-arg.
c) VDL codegen in each language is updated accordingly. The
largest changes are in Go, where we need to change the calls
to Finish to not pass in the error.
d) All manual (non-vdl-generated) calls to Finish updated to
not pass in the error.
e) ipc.ReflectInvoker updated to require a last out-arg on each
RPC method, which is captured and special-cased in Invoke.
f) RPC methods in tests updated to always return an error; some
of the tests that weren't using VDL weren't always returning
an error.
g) Some code was incorrectly relying on returning both the
out-args and the error, which happened to work in Go, but
wouldn't work with Java / JS clients. This was mostly in
tests. The only non-test code with this property is in:
veyron/services/mgmt/application/impl
This was still "trivial" though, since the returned out-arg
was only used in the tests.
MultiPart: 1/8
Change-Id: I2e02f50ed768bc1e90a9e4f438dcdccae70854d2
diff --git a/runtimes/google/ipc/simple_test.go b/runtimes/google/ipc/simple_test.go
index c5bf45c..f8546e8 100644
--- a/runtimes/google/ipc/simple_test.go
+++ b/runtimes/google/ipc/simple_test.go
@@ -7,7 +7,6 @@
"v.io/core/veyron2"
"v.io/core/veyron2/ipc"
- "v.io/core/veyron2/verror"
)
type simple struct {
@@ -45,7 +44,10 @@
i := 0
for {
if err := call.Recv(&i); err != nil {
- return i, err
+ if err == io.EOF {
+ return i, nil
+ }
+ return 0, err
}
}
}
@@ -55,12 +57,9 @@
for {
if err := call.Recv(&i); err != nil {
if err == io.EOF {
- // TODO(cnicolaou): this should return a verror, i.e.
- // verror.New(verror.EOF, call), but for now we
- // return an io.EOF
- return i, io.EOF
+ return i, nil
}
- return i, err
+ return 0, err
}
call.Send(i + inc)
}
@@ -78,9 +77,7 @@
t.Fatalf("unexpected error: %s", err)
}
response := ""
- var verr error
- err = call.Finish(&response, &verr)
- if err != nil {
+ if err := call.Finish(&response); err != nil {
t.Fatalf("unexpected error: %s", err)
}
if got, want := response, "pong"; got != want {
@@ -115,20 +112,11 @@
}
call.CloseSend()
final := -1
- verr := call.Finish(&final, &err)
- if verr != nil {
- t.Fatalf("unexpected error: %s", verr)
-
+ err = call.Finish(&final)
+ if err != nil {
+ t.Errorf("unexpected error: %#v", err)
}
- if !verror.Is(err, verror.ErrUnknown.ID) || err.Error() != `v.io/core/veyron2/verror.Unknown: EOF` {
- t.Errorf("wrong error: %#v", err)
- }
- /* TODO(cnicolaou): use this when verror/vom transition is done.
- if err != nil && !verror.Is(err, verror.EOF.ID) {
- t.Fatalf("unexpected error: %#v", err)
- }
- */
if got := final; got != want {
- t.Fatalf("got %d, want %d")
+ t.Fatalf("got %d, want %d", got, want)
}
}
