code review 154159: Make non-errored RPC calls return 'nil' error to caller.

Make non-errored RPC calls return 'nil' error to caller.

Error information is carried from RPC server to client in the string
'Error' field of rpc.Response. An empty string is sent in the success
case. This empty string was being returned to the caller (of Client.Call
or Client.Go), resulting in a non-nil error response.

This change detects an empty-string Response.Error at the client, and
translates it into a nil value in Call.Error.

Tests updated to check error return in success cases.

[Aroneous, 2009-11-16 11:01:32]

Hello r, rsc,

I'd like you to review the following change.

[r, 2009-11-17 00:42:04]

http://codereview.appspot.com/154159/diff/4/1003
File src/pkg/rpc/client.go (right):

http://codereview.appspot.com/154159/diff/4/1003#newcode91
src/pkg/rpc/client.go:91: }
i have this fixed already in my client.  please revert.

i like your tests, though. if you want to keep them, i'm happy to patch them
from here after you fix the %qs

http://codereview.appspot.com/154159/diff/4/1004
File src/pkg/rpc/server_test.go (right):

http://codereview.appspot.com/154159/diff/4/1004#newcode90
src/pkg/rpc/server_test.go:90: t.Errorf("Add: expected no error but got string
%#v", err.String())
%#v is fine but please use %q

[Aroneous, 2009-11-17 01:16:16]

On 2009/11/17 00:42:04, r wrote:
> http://codereview.appspot.com/154159/diff/4/1003
> File src/pkg/rpc/client.go (right):
> 
> http://codereview.appspot.com/154159/diff/4/1003#newcode91
> src/pkg/rpc/client.go:91: }
> i have this fixed already in my client.  please revert.

Reverted and removed from changeset. This does, of course, cause test failures
in absence of the fix.

> 
> i like your tests, though. if you want to keep them, i'm happy to patch them
> from here after you fix the %qs

%#vs changed to %qs in latest version. Patch away!

> 
> http://codereview.appspot.com/154159/diff/4/1004
> File src/pkg/rpc/server_test.go (right):
> 
> http://codereview.appspot.com/154159/diff/4/1004#newcode90
> src/pkg/rpc/server_test.go:90: t.Errorf("Add: expected no error but got string
> %#v", err.String())
> %#v is fine but please use %q

[Aroneous, 2009-11-17 01:18:27]

Requested changes made.

Best,
Aron

[rsc, 2009-11-17 07:09:49]

Could you please send a different CL updating AUTHORS
and CONTRIBUTORS as described at
http://golang.org/doc/contribute.html#copyright ?

Thanks.

[Aroneous, 2009-11-17 07:50:01]

Looks like my reading comprehension petered out toward the end of that page;
sorry.
154175 submitted.

Regards,
Aron

On 2009/11/17 07:09:49, rsc wrote:
> Could you please send a different CL updating AUTHORS
> and CONTRIBUTORS as described at
> http://golang.org/doc/contribute.html#copyright ?
> 
> Thanks.

[r, 2009-11-17 19:05:02]

LGTM

i will submit

[r, 2009-11-17 19:29:04]

*** Submitted as http://code.google.com/p/go/source/detail?r=0294d46a1f6e ***

Make non-errored RPC calls return 'nil' error to caller.

Error information is carried from RPC server to client in the string
'Error' field of rpc.Response. An empty string is sent in the success
case. This empty string was being returned to the caller (of Client.Call
or Client.Go), resulting in a non-nil error response.

This change detects an empty-string Response.Error at the client, and
translates it into a nil value in Call.Error.

Tests updated to check error return in success cases.

R=r, rsc
http://codereview.appspot.com/154159

Committer: Rob Pike <r@golang.org>

[rsc, 2009-11-30 22:49:48]

aroneous: please run hg sync in your client
or visit http://codereview.appspot.com/154175/show
and click the "x in a circle" next to the Issue headline,
so that this issue will be marked as closed.

thanks.

[rsc, 2009-11-30 22:52:40]

On 2009/11/30 22:49:48, rsc wrote:
> or visit http://codereview.appspot.com/154175/show

sorry i meant 154159