Merge pull request go-zookeeper#134 from nomis52/multi-fix

Handle Multi() errors correctly.

zk/conn.go

@@ -885,6 +885,7 @@ func (c *Conn) Sync(path string) (string, error) {
 type MultiResponse struct {
 	Stat   *Stat
 	String string
+	Error  error
 }
 
 // Multi executes multiple ZooKeeper operations or none of them. The provided
@@ -915,7 +916,7 @@ func (c *Conn) Multi(ops ...interface{}) ([]MultiResponse, error) {
 	_, err := c.request(opMulti, req, res, nil)
 	mr := make([]MultiResponse, len(res.Ops))
 	for i, op := range res.Ops {
-		mr[i] = MultiResponse{Stat: op.Stat, String: op.String}
+		mr[i] = MultiResponse{Stat: op.Stat, String: op.String, Error: op.Err.toError()}
 	}
 	return mr, err
 }

zk/constants.go

@@ -28,6 +28,7 @@ const (
 	opClose        = -11
 	opSetAuth      = 100
 	opSetWatches   = 101
+	opError        = -1
 	// Not in protocol, used internally
 	opWatcherEvent = -2
 )

zk/structs.go

@@ -270,6 +270,7 @@ type multiResponseOp struct {
 	Header multiHeader
 	String string
 	Stat   *Stat
+	Err    ErrCode
 }
 type multiResponse struct {
 	Ops        []multiResponseOp
@@ -327,6 +328,8 @@ func (r *multiRequest) Decode(buf []byte) (int, error) {
 }
 
 func (r *multiResponse) Decode(buf []byte) (int, error) {
+	var multiErr error
+
 	r.Ops = make([]multiResponseOp, 0)
 	r.DoneHeader = multiHeader{-1, true, -1}
 	total := 0
@@ -347,6 +350,8 @@ func (r *multiResponse) Decode(buf []byte) (int, error) {
 		switch header.Type {
 		default:
 			return total, ErrAPIError
+		case opError:
+			w = reflect.ValueOf(&res.Err)
 		case opCreate:
 			w = reflect.ValueOf(&res.String)
 		case opSetData:
@@ -362,8 +367,12 @@ func (r *multiResponse) Decode(buf []byte) (int, error) {
 			total += n
 		}
 		r.Ops = append(r.Ops, res)
+		if multiErr == nil && res.Err != errOk {
+			// Use the first error as the error returned from Multi().
+			multiErr = res.Err.toError()
+		}
 	}
-	return total, nil
+	return total, multiErr
 }
 
 type watcherEvent struct {

zk/zk_test.go

@@ -125,6 +125,54 @@ func TestMulti(t *testing.T) {
 	}
 }
 
+func TestMultiFailures(t *testing.T) {
+	// This test case ensures that we return the errors associated with each
+	// opeThis in the event a call to Multi() fails.
+	const firstPath = "/gozk-test-first"
+	const secondPath = "/gozk-test-second"
+
+	ts, err := StartTestCluster(1, nil, logWriter{t: t, p: "[ZKERR] "})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ts.Stop()
+	zk, _, err := ts.ConnectAll()
+	if err != nil {
+		t.Fatalf("Connect returned error: %+v", err)
+	}
+	defer zk.Close()
+
+	// Ensure firstPath doesn't exist and secondPath does. This will cause the
+	// 2nd operation in the Multi() to fail.
+	if err := zk.Delete(firstPath, -1); err != nil && err != ErrNoNode {
+		t.Fatalf("Delete returned error: %+v", err)
+	}
+	if _, err := zk.Create(secondPath, nil /* data */, 0, WorldACL(PermAll)); err != nil {
+		t.Fatalf("Create returned error: %+v", err)
+	}
+
+	ops := []interface{}{
+		&CreateRequest{Path: firstPath, Data: []byte{1, 2}, Acl: WorldACL(PermAll)},
+		&CreateRequest{Path: secondPath, Data: []byte{3, 4}, Acl: WorldACL(PermAll)},
+	}
+	res, err := zk.Multi(ops...)
+	if err != ErrNodeExists {
+		t.Fatalf("Multi() didn't return correct error: %+v", err)
+	}
+	if len(res) != 2 {
+		t.Fatalf("Expected 2 responses received %d", len(res))
+	}
+	if res[0].Error != nil {
+		t.Fatalf("First operation returned an unexpected error %+v", res[0].Error)
+	}
+	if res[1].Error != ErrNodeExists {
+		t.Fatalf("Second operation returned incorrect error %+v", res[1].Error)
+	}
+	if _, _, err := zk.Get(firstPath); err != ErrNoNode {
+		t.Fatalf("Node %s was incorrectly created: %+v", firstPath, err)
+	}
+}
+
 func TestGetSetACL(t *testing.T) {
 	ts, err := StartTestCluster(1, nil, logWriter{t: t, p: "[ZKERR] "})
 	if err != nil {