zetcd: fix encoding of multiop errors

The current multiop implementation was not properly encoding a single error
in a multiop request.  If op 1 succeeds, but op 2 fails, then zookeeper
should return two responses, one success and one failure of type -1.

Fixes etcd-io#98

constants.go

@@ -23,6 +23,7 @@ const (
 )
 
 const (
+	opError        = -1
 	opCreate       = 1
 	opDelete       = 2
 	opExists       = 3

encode.go

@@ -329,6 +329,8 @@ func (r *MultiResponse) Encode(buf []byte) (int, error) {
 			n, err = encodePacketValue(buf[total:], reflect.ValueOf(op.String))
 		case opSetData:
 			n, err = encodePacketValue(buf[total:], reflect.ValueOf(op.Stat))
+		case opError:
+			n, err = encodePacketValue(buf[total:], reflect.ValueOf(&op.Header.Err))
 		}
 		total += n
 		if err != nil {

integration/integration_test.go

@@ -705,6 +705,23 @@ func testMultiOp(t *testing.T, c *zk.Conn) {
 	if s1.Mzxid != s2.Mzxid {
 		t.Fatalf("expected zxids in %+v to match %+v", *s1, *s2)
 	}
+	// test partial success
+	ops = []interface{}{
+		&zk.CheckVersionRequest{Path: "/test2", Version: 0},
+		&zk.CreateRequest{Path: "/foo", Data: []byte("foo"), Acl: acl},
+	}
+	if resp, err = c.Multi(ops...); err == nil || err.Error() != zetcd.ErrNodeExists.Error() {
+		t.Fatalf("expected %v, got %v", zetcd.ErrNodeExists, err)
+	}
+	if len(resp) != 2 {
+		t.Fatalf("expected %d results, got %d", 2, len(resp))
+	}
+	if resp[0].Error != nil {
+		t.Fatalf("expected checkop error to be nil, got %v", resp[0].Error)
+	}
+	if resp[1].Error == nil || resp[1].Error.Error() != zetcd.ErrNodeExists.Error() {
+		t.Fatalf("expected createop error to be %v, got %v", zetcd.ErrNodeExists.Error(), resp[1].Error)
+	}
 }
 
 func runTest(t *testing.T, f func(*testing.T, *zk.Conn)) {

server.go

@@ -76,7 +76,7 @@ func serveRequest(s Session, zke ZK, zkreq ZKRequest) error {
 	}
 	zkresp := DispatchZK(zke, zkreq.xid, zkreq.req)
 	if zkresp.Err != nil {
-		glog.V(9).Infof("dispatch error", zkresp.Err)
+		glog.V(9).Infof("dispatch error %v", zkresp.Err)
 		return zkresp.Err
 	}
 	if zkresp.Hdr.Err == 0 {

zketcd.go

@@ -502,12 +502,10 @@ func (z *zkEtcd) Multi(xid Xid, mreq *MultiRequest) ZKResponse {
 		for i, b := range bs {
 			if err := b.apply(s); err != nil {
 				var ok bool
+				mresp.Ops[i].Header.Type = opError
 				if mresp.Ops[i].Header.Err, ok = errorToErrCode[err]; !ok {
 					mresp.Ops[i].Header.Err = errAPIError
 				}
-				if mresp.DoneHeader.Err == 0 {
-					mresp.DoneHeader.Err = mresp.Ops[i].Header.Err
-				}
 				return err
 			}
 		}
@@ -524,7 +522,7 @@ func (z *zkEtcd) Multi(xid Xid, mreq *MultiRequest) ZKResponse {
 				mresp.Ops[i].Stat = &r.Stat
 			}
 		}
-		return mkZKPartialResp(xid, zxid, mresp, mresp.DoneHeader.Err)
+		return mkZKResp(xid, zxid, mresp)
 	}
 
 	resp, _ := z.doSTM(apply, prefetch...)
@@ -536,6 +534,7 @@ func (z *zkEtcd) Multi(xid Xid, mreq *MultiRequest) ZKResponse {
 		return reply(xid, zxid)
 	}
 
+	glog.V(7).Infof("Multi(%v) = (zxid=%v); txnresp: %+v\n", *mreq, resp.Header.Revision, *resp)
 	return reply(xid, ZXid(resp.Header.Revision))
 }
 
@@ -728,10 +727,6 @@ func mkZKResp(xid Xid, zxid ZXid, resp interface{}) ZKResponse {
 	return ZKResponse{Hdr: &ResponseHeader{xid, zxid - 1, 0}, Resp: resp}
 }
 
-func mkZKPartialResp(xid Xid, zxid ZXid, resp interface{}, err ErrCode) ZKResponse {
-	return ZKResponse{Hdr: &ResponseHeader{xid, zxid - 1, err}, Resp: resp}
-}
-
 // wrapErr is to pass back error info but still get the txn response
 func wrapErr(err *error, f func(s v3sync.STM) error) func(s v3sync.STM) error {
 	return func(s v3sync.STM) error {