server/rpc_codec: if c.codec.Write fails, reset write buffer and encode an error message about the encoding failure

When developing go-micro services, it is frequently possible to set invalid results in the response pointer. When this happens (as I and @trushton personally experienced), `sendResponse()` returns an error correctly explaining what happened (e.g. protobuf refused to encode a bad struct) but the `call()` function one above it in the stack ignores the returned error object.

Thus, invalid structs go un-encoded and the _client side times out_. @trushton and I first caught this in our CI builds when we left a protobuf.Empty field uninitialized (nil) instead of setting it to `&ptypes.Empty{}`. This resulted in an `proto: oneof field has nil value` error, but it was dropped and became a terribly confusing client timeout instead.

This patch is two independent changes:

* In rpc_codec, when a serialization failure occurs serialize an error message, which will correctly become a 500 for HTTP services, about the encoding failure. This means rpc_codec only returns an `error` when a socket failure occurs, which I believe is the behavior that rpc_service is expecting anyway.

* In rpc_service, log any errors returned by sendResponse instead of dropping the error object. This will make debugging client timeouts less of a hassle.

server/rpc_codec.go

@@ -7,6 +7,7 @@ import (
 	"github.com/micro/go-micro/codec/jsonrpc"
 	"github.com/micro/go-micro/codec/protorpc"
 	"github.com/micro/go-micro/transport"
+	"github.com/pkg/errors"
 )
 
 type rpcPlusCodec struct {
@@ -96,7 +97,11 @@ func (c *rpcPlusCodec) WriteResponse(r *response, body interface{}, last bool) e
 		Header: map[string]string{},
 	}
 	if err := c.codec.Write(m, body); err != nil {
-		return err
+		c.buf.wbuf.Reset()
+		m.Error = errors.Wrapf(err, "Unable to encode body").Error()
+		if err := c.codec.Write(m, nil); err != nil {
+			return err
+		}
 	}
 
 	m.Header["Content-Type"] = c.req.Header["Content-Type"]

server/rpc_codec_test.go

@@ -0,0 +1,100 @@
+package server
+
+import (
+	"bytes"
+	"errors"
+	"testing"
+
+	"github.com/micro/go-micro/codec"
+	"github.com/micro/go-micro/transport"
+)
+
+// testCodec is a dummy codec that only knows how to encode nil bodies
+type testCodec struct {
+	buf *bytes.Buffer
+}
+
+type testSocket struct {
+}
+
+// TestCodecWriteError simulates what happens when a codec is unable
+// to encode a message (e.g. a missing branch of an "oneof" message in
+// protobufs)
+//
+// We expect an error to be sent to the socket. Previously the socket
+// would remain open with no bytes sent, leading to client-side
+// timeouts.
+func TestCodecWriteError(t *testing.T) {
+	socket := testSocket{}
+	message := transport.Message{
+		Header: map[string]string{},
+		Body:   []byte{},
+	}
+
+	rwc := readWriteCloser{
+		rbuf: new(bytes.Buffer),
+		wbuf: new(bytes.Buffer),
+	}
+
+	c := rpcPlusCodec{
+		buf: &rwc,
+		codec: &testCodec{
+			buf: rwc.wbuf,
+		},
+		req:    &message,
+		socket: socket,
+	}
+
+	err := c.WriteResponse(&response{
+		ServiceMethod: "Service.Method",
+		Seq:           0,
+		Error:         "",
+		next:          nil,
+	}, "body", false)
+
+	if err != nil {
+		t.Fatalf(`Expected WriteResponse to fail; got "%+v" instead`, err)
+	}
+
+	const expectedError = "Unable to encode body: simulating a codec write failure"
+	actualError := rwc.wbuf.String()
+	if actualError != expectedError {
+		t.Fatalf(`Expected error "%+v" in the write buffer, got "%+v" instead`, expectedError, actualError)
+	}
+}
+
+func (c *testCodec) ReadHeader(message *codec.Message, typ codec.MessageType) error {
+	return nil
+}
+
+func (c *testCodec) ReadBody(dest interface{}) error {
+	return nil
+}
+
+func (c *testCodec) Write(message *codec.Message, dest interface{}) error {
+	if dest != nil {
+		return errors.New("simulating a codec write failure")
+	}
+	c.buf.Write([]byte(message.Error))
+	return nil
+}
+
+func (c *testCodec) Close() error {
+	return nil
+}
+
+func (c *testCodec) String() string {
+	return "string"
+}
+
+func (s testSocket) Recv(message *transport.Message) error {
+	return nil
+}
+
+func (s testSocket) Send(message *transport.Message) error {
+	return nil
+}
+
+func (s testSocket) Close() error {
+	return nil
+}

server/rpc_service.go

@@ -249,7 +249,10 @@ func (s *service) call(ctx context.Context, server *server, sending *sync.Mutex,
 			errmsg = err.Error()
 		}
 
-		server.sendResponse(sending, req, replyv.Interface(), codec, errmsg, true)
+		err = server.sendResponse(sending, req, replyv.Interface(), codec, errmsg, true)
+		if err != nil {
+			log.Log("rpc call: unable to send response: ", err)
+		}
 		server.freeRequest(req)
 		return
 	}