net/rpc: gob encoding error not returned in response

[gopherbot]

by charlieandrews.cwa:

I was working with the "net/rpc" package using the default gob client/server
codec. Requests were being sent fine and all responses were being returned fine, except
for one, which would just hang. I could see that everything was going fine until it went
to write the response. I traced the issue all the way to the
gobServerCodec.WriteResponse function in "net/rpc/server.go". I put some log
lines in, and noticed that it encoded the Response just fine, but errored on the
encoding of the body. It seems that there was no way for me to know it errored on the
body unless I had debugLog set to true (which there doesn't seem to be a nice way to
do), or put a log line in there. Is there a way that the order that the Response and
body get encoded could be switched, so that if an error occurs while encoding the body,
it will be reported in Response.Error ? I think the change would be trivial, so I will
do it if others think that I am correct in thinking that this is a bug.

[ianlancetaylor]

Comment 1:

Labels changed: added repo-main, release-go1.3maybe.

[gopherbot]

Comment 2 by charlieandrews.cwa:

I posted yesterday on the mailing list with a proposed change.
https://groups.google.com/forum/#!topic/golang-nuts/AMojGgap0YQ

[nigeltao]

Comment 3:

Owner changed to @robpike.

[rsc]

Comment 4:

I looked into this briefly. If you buffer the response you end up with an unnecessary
second copy in memory. Maybe it's not a big deal, I don't know.
At the least perhaps gobServerCodec.WriteResponse should detect the lack of I/O error
and do something like print a log message, shut down the connection, or emit a gob value
that will cause a decoding error on the other side.
    if err = c.enc.Encode(r); err != nil {
        if c.encBuf.Flush() == nil {
            // was a gob problem, not a write problem. do something
        }
        return
    }
    if err = c.enc.Encode(body); err != nil {
        if c.encBuf.Flush() == nil {
            // was a gob problem, not a write problem. do something
        }
        return
    }
    return c.encBuf.Flush()

Status changed to Accepted.

[robpike]

Comment 5:

Labels changed: added release-go1.4, removed release-go1.3maybe.

[gopherbot]

Comment 6:

CL https://golang.org/cl/146670043 mentions this issue.

[robpike]

Comment 7:

This issue was closed by revision cd5b785.

Status changed to Fixed.

[robpike]

Comment 8:

Issue #8173 has been merged into this issue.

[gopherbot]

Comment 9:

CL https://golang.org/cl/151370043 mentions this issue.