Merge lp:~rogpeppe/gozk/better-error-messages into lp:gozk/zookeeper

Proposed by Roger Peppe
Status: Merged
Merged at revision: 33
Proposed branch: lp:~rogpeppe/gozk/better-error-messages
Merge into: lp:gozk/zookeeper
Diff against target: 638 lines (+162/-91)
4 files modified
close_test.go (+1/-1)
retry_test.go (+3/-3)
zk.go (+104/-72)
zk_test.go (+54/-15)
To merge this branch: bzr merge lp:~rogpeppe/gozk/better-error-messages
Reviewer Review Type Date Requested Status
The Go Language Gophers Pending
Review via email: mp+97613@code.launchpad.net

Description of the change

zookeeper: make error messages more informative.

https://codereview.appspot.com/5835045/

To post a comment you must log in.
Revision history for this message
Roger Peppe (rogpeppe) wrote :

Reviewers: mp+97613_code.launchpad.net,

Message:
Please take a look.

Description:

https://code.launchpad.net/~rogpeppe/gozk/better-error-messages/+merge/97613

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/5835045/

Affected files:
   A [revision details]
   M close_test.go
   M retry_test.go
   M zk.go
   M zk_test.go

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Very nice. Just a few ideas.

https://codereview.appspot.com/5835045/diff/2001/zk.go
File zk.go (right):

https://codereview.appspot.com/5835045/diff/2001/zk.go#newcode120
zk.go:120: // HasErrorCode returns whether the error has the given
It's nice to have this. I was a bit concerned it might be boring to deal
with it.

Can we please call the function as IsError? For example:

   if zookeeper.IsError(err, zookeeper.ZNONODE) { ... }

Also, for the comment, I suggest something like this:

// IsError returns whether err is an *Error with the given
// error code.

https://codereview.appspot.com/5835045/diff/2001/zk.go#newcode457
zk.go:457: func errClosing(op string) error {
Can we please call this closingError and put it next to zkError? They're
pretty related.

Also, closingError should take a path, as many of the places it's used
have it in context.

https://codereview.appspot.com/5835045/diff/2001/zk.go#newcode584
zk.go:584: return nil, nil, nil, errClosing("childrenW")
The upper/lower casing is inconsistent. We have "ACL", "addauth",
"setACL", and "childrenW". I suggest we lowercase them all completely.
This is an error message, and more of a hint than something that has to
reflect the method name used exactly.

https://codereview.appspot.com/5835045/diff/2001/zk.go#newcode925
zk.go:925: if err != nil && !HasErrorCode(err, ZNONODE) {
err != nil isn't necessary here.

https://codereview.appspot.com/5835045/diff/2001/zk_test.go
File zk_test.go (right):

https://codereview.appspot.com/5835045/diff/2001/zk_test.go#newcode1
zk_test.go:1: package zookeeper_test
The Error.Error method has a few different possibilities now as far as
building the error message goes. Would you mind to add a test case for
those? We don't want to find out that we broke that logic at some point
in actual usage.

https://codereview.appspot.com/5835045/diff/2001/zk_test.go#newcode161
zk_test.go:161: c.Assert(err, ErrorMatches, `zookeeper: get
"/non-existent": no node`)
Nice!

https://codereview.appspot.com/5835045/

Revision history for this message
Roger Peppe (rogpeppe) wrote :
39. By Roger Peppe

changes for review

40. By Roger Peppe

go fmt

Revision history for this message
Roger Peppe (rogpeppe) wrote :

https://codereview.appspot.com/5835045/diff/2001/zk.go
File zk.go (right):

https://codereview.appspot.com/5835045/diff/2001/zk.go#newcode120
zk.go:120: // HasErrorCode returns whether the error has the given
On 2012/03/15 14:31:38, niemeyer wrote:
> It's nice to have this. I was a bit concerned it might be boring to
deal with
> it.

> Can we please call the function as IsError? For example:

> if zookeeper.IsError(err, zookeeper.ZNONODE) { ... }

> Also, for the comment, I suggest something like this:

> // IsError returns whether err is an *Error with the given
> // error code.

yeah, that's nicer. done.

https://codereview.appspot.com/5835045/diff/2001/zk.go#newcode457
zk.go:457: func errClosing(op string) error {
On 2012/03/15 14:31:38, niemeyer wrote:
> Can we please call this closingError and put it next to zkError?
They're pretty
> related.

> Also, closingError should take a path, as many of the places it's used
have it
> in context.

Done.

https://codereview.appspot.com/5835045/diff/2001/zk.go#newcode584
zk.go:584: return nil, nil, nil, errClosing("childrenW")
On 2012/03/15 14:31:38, niemeyer wrote:
> The upper/lower casing is inconsistent. We have "ACL", "addauth",
"setACL", and
> "childrenW". I suggest we lowercase them all completely. This is an
error
> message, and more of a hint than something that has to reflect the
method name
> used exactly.

yeah, i wondered about that. done.

https://codereview.appspot.com/5835045/diff/2001/zk.go#newcode925
zk.go:925: if err != nil && !HasErrorCode(err, ZNONODE) {
On 2012/03/15 14:31:38, niemeyer wrote:
> err != nil isn't necessary here.

it is necessary, i think, otherwise we'll return when err is nil.

https://codereview.appspot.com/5835045/diff/2001/zk_test.go
File zk_test.go (right):

https://codereview.appspot.com/5835045/diff/2001/zk_test.go#newcode1
zk_test.go:1: package zookeeper_test
On 2012/03/15 14:31:38, niemeyer wrote:
> The Error.Error method has a few different possibilities now as far as
building
> the error message goes. Would you mind to add a test case for those?
We don't
> want to find out that we broke that logic at some point in actual
usage.

Done.

https://codereview.appspot.com/5835045/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :
Revision history for this message
Roger Peppe (rogpeppe) wrote :

*** Submitted:

zookeeper: make error messages more informative.

R=niemeyer
CC=
https://codereview.appspot.com/5835045

https://codereview.appspot.com/5835045/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'close_test.go'
2--- close_test.go 2012-02-28 09:13:50 +0000
3+++ close_test.go 2012-03-15 15:14:18 +0000
4@@ -130,7 +130,7 @@
5 }
6 p.close()
7 err = f(conn, "/closetest")
8- c.Check(err, Equals, zk.ZCLOSING)
9+ c.Check(zk.IsError(err, zk.ZCLOSING), Equals, true, Commentf("%v", err))
10 }
11 }
12
13
14=== modified file 'retry_test.go'
15--- retry_test.go 2012-02-27 17:15:50 +0000
16+++ retry_test.go 2012-03-15 15:14:18 +0000
17@@ -209,7 +209,7 @@
18 return "", nil
19 })
20 c.Assert(err, NotNil)
21- c.Assert(err, Equals, zk.ZNOAUTH)
22+ c.Check(zk.IsError(err, zk.ZNOAUTH), Equals, true, Commentf("%v", err))
23
24 stat, err := conn.Exists("/test")
25 c.Assert(err, IsNil)
26@@ -232,7 +232,7 @@
27 called = true
28 return "", nil
29 })
30- c.Assert(err, Equals, zk.ZNOAUTH)
31+ c.Check(zk.IsError(err, zk.ZNOAUTH), Equals, true, Commentf("%v", err))
32
33 stat, err := conn.Exists("/test")
34 c.Assert(err, IsNil)
35@@ -256,7 +256,7 @@
36 return "", nil
37 })
38 c.Assert(err, NotNil)
39- c.Assert(err, Equals, zk.ZNOAUTH)
40+ c.Check(zk.IsError(err, zk.ZNOAUTH), Equals, true, Commentf("%v", err))
41
42 stat, err := conn.Exists("/test/sub")
43 c.Assert(err, IsNil)
44
45=== modified file 'zk.go'
46--- zk.go 2012-02-27 17:15:50 +0000
47+++ zk.go 2012-03-15 15:14:18 +0000
48@@ -98,57 +98,89 @@
49 }
50
51 // Error represents a ZooKeeper error.
52-type Error int
53+type Error struct {
54+ Op string
55+ Code ErrorCode
56+ // SystemError holds an error if Code is ZSYSTEMERROR.
57+ SystemError error
58+ Path string
59+}
60+
61+func (e *Error) Error() string {
62+ s := e.Code.String()
63+ if e.Code == ZSYSTEMERROR && e.SystemError != nil {
64+ s = e.SystemError.Error()
65+ }
66+ if e.Path == "" {
67+ return fmt.Sprintf("zookeeper: %s: %v", e.Op, s)
68+ }
69+ return fmt.Sprintf("zookeeper: %s %q: %v", e.Op, e.Path, s)
70+}
71+
72+// IsError returns whether the error is a *Error
73+// with the given error code.
74+func IsError(err error, code ErrorCode) bool {
75+ if err, _ := err.(*Error); err != nil {
76+ return err.Code == code
77+ }
78+ return false
79+}
80+
81+// ErrorCode represents a kind of ZooKeeper error.
82+type ErrorCode int
83
84 const (
85- ZOK Error = C.ZOK
86- ZSYSTEMERROR Error = C.ZSYSTEMERROR
87- ZRUNTIMEINCONSISTENCY Error = C.ZRUNTIMEINCONSISTENCY
88- ZDATAINCONSISTENCY Error = C.ZDATAINCONSISTENCY
89- ZCONNECTIONLOSS Error = C.ZCONNECTIONLOSS
90- ZMARSHALLINGERROR Error = C.ZMARSHALLINGERROR
91- ZUNIMPLEMENTED Error = C.ZUNIMPLEMENTED
92- ZOPERATIONTIMEOUT Error = C.ZOPERATIONTIMEOUT
93- ZBADARGUMENTS Error = C.ZBADARGUMENTS
94- ZINVALIDSTATE Error = C.ZINVALIDSTATE
95- ZAPIERROR Error = C.ZAPIERROR
96- ZNONODE Error = C.ZNONODE
97- ZNOAUTH Error = C.ZNOAUTH
98- ZBADVERSION Error = C.ZBADVERSION
99- ZNOCHILDRENFOREPHEMERALS Error = C.ZNOCHILDRENFOREPHEMERALS
100- ZNODEEXISTS Error = C.ZNODEEXISTS
101- ZNOTEMPTY Error = C.ZNOTEMPTY
102- ZSESSIONEXPIRED Error = C.ZSESSIONEXPIRED
103- ZINVALIDCALLBACK Error = C.ZINVALIDCALLBACK
104- ZINVALIDACL Error = C.ZINVALIDACL
105- ZAUTHFAILED Error = C.ZAUTHFAILED
106- ZCLOSING Error = C.ZCLOSING
107- ZNOTHING Error = C.ZNOTHING
108- ZSESSIONMOVED Error = C.ZSESSIONMOVED
109+ ZOK ErrorCode = C.ZOK
110+ ZSYSTEMERROR ErrorCode = C.ZSYSTEMERROR
111+ ZRUNTIMEINCONSISTENCY ErrorCode = C.ZRUNTIMEINCONSISTENCY
112+ ZDATAINCONSISTENCY ErrorCode = C.ZDATAINCONSISTENCY
113+ ZCONNECTIONLOSS ErrorCode = C.ZCONNECTIONLOSS
114+ ZMARSHALLINGERROR ErrorCode = C.ZMARSHALLINGERROR
115+ ZUNIMPLEMENTED ErrorCode = C.ZUNIMPLEMENTED
116+ ZOPERATIONTIMEOUT ErrorCode = C.ZOPERATIONTIMEOUT
117+ ZBADARGUMENTS ErrorCode = C.ZBADARGUMENTS
118+ ZINVALIDSTATE ErrorCode = C.ZINVALIDSTATE
119+ ZAPIERROR ErrorCode = C.ZAPIERROR
120+ ZNONODE ErrorCode = C.ZNONODE
121+ ZNOAUTH ErrorCode = C.ZNOAUTH
122+ ZBADVERSION ErrorCode = C.ZBADVERSION
123+ ZNOCHILDRENFOREPHEMERALS ErrorCode = C.ZNOCHILDRENFOREPHEMERALS
124+ ZNODEEXISTS ErrorCode = C.ZNODEEXISTS
125+ ZNOTEMPTY ErrorCode = C.ZNOTEMPTY
126+ ZSESSIONEXPIRED ErrorCode = C.ZSESSIONEXPIRED
127+ ZINVALIDCALLBACK ErrorCode = C.ZINVALIDCALLBACK
128+ ZINVALIDACL ErrorCode = C.ZINVALIDACL
129+ ZAUTHFAILED ErrorCode = C.ZAUTHFAILED
130+ ZCLOSING ErrorCode = C.ZCLOSING
131+ ZNOTHING ErrorCode = C.ZNOTHING
132+ ZSESSIONMOVED ErrorCode = C.ZSESSIONMOVED
133 )
134
135-func (error Error) Error() string {
136- return C.GoString(C.zerror(C.int(error))) // Static, no need to free it.
137+func (code ErrorCode) String() string {
138+ return C.GoString(C.zerror(C.int(code))) // Static, no need to free it.
139 }
140
141 // zkError creates an appropriate error return from
142 // a ZooKeeper status and the errno return from a C API
143 // call.
144-func zkError(rc C.int, cerr error) error {
145- code := Error(rc)
146- switch code {
147- case ZOK:
148+func zkError(rc C.int, cerr error, op, path string) error {
149+ code := ErrorCode(rc)
150+ if code == ZOK {
151 return nil
152+ }
153+ err := &Error{
154+ Op: op,
155+ Code: code,
156+ Path: path,
157+ }
158+ if code == ZSYSTEMERROR {
159+ err.SystemError = cerr
160+ }
161+ return err
162+}
163
164- case ZSYSTEMERROR:
165- // If a ZooKeeper call returns ZSYSTEMERROR, then
166- // errno becomes significant. If errno has not been
167- // set, then we will return ZSYSTEMERROR nonetheless.
168- if cerr != nil {
169- return cerr
170- }
171- }
172- return code
173+func closingError(op, path string) error {
174+ return zkError(C.int(ZCLOSING), nil, op, path)
175 }
176
177 // Constants for SetLogLevel.
178@@ -419,7 +451,7 @@
179 C.free(unsafe.Pointer(cservers))
180 if handle == nil {
181 conn.closeAllWatches()
182- return nil, nil, zkError(C.int(ZSYSTEMERROR), cerr)
183+ return nil, nil, zkError(C.int(ZSYSTEMERROR), cerr, "dial", "")
184 }
185 conn.handle = handle
186 runWatchLoop()
187@@ -444,7 +476,7 @@
188 if conn.handle == nil {
189 // ZooKeeper may hang indefinitely if a handler is closed twice,
190 // so we get in the way and prevent it from happening.
191- return ZCLOSING
192+ return closingError("close", "")
193 }
194 rc, cerr := C.zookeeper_close(conn.handle)
195
196@@ -454,7 +486,7 @@
197 // At this point, nothing else should need conn.handle.
198 conn.handle = nil
199
200- return zkError(rc, cerr)
201+ return zkError(rc, cerr, "close", "")
202 }
203
204 // Get returns the data and status from an existing node. err will be nil,
205@@ -464,7 +496,7 @@
206 conn.mutex.RLock()
207 defer conn.mutex.RUnlock()
208 if conn.handle == nil {
209- return "", nil, ZCLOSING
210+ return "", nil, closingError("get", path)
211 }
212
213 cpath := C.CString(path)
214@@ -476,7 +508,7 @@
215 var cstat Stat
216 rc, cerr := C.zoo_wget(conn.handle, cpath, nil, nil, cbuffer, &cbufferLen, &cstat.c)
217 if rc != C.ZOK {
218- return "", nil, zkError(rc, cerr)
219+ return "", nil, zkError(rc, cerr, "get", path)
220 }
221
222 result := C.GoStringN(cbuffer, cbufferLen)
223@@ -491,7 +523,7 @@
224 conn.mutex.RLock()
225 defer conn.mutex.RUnlock()
226 if conn.handle == nil {
227- return "", nil, nil, ZCLOSING
228+ return "", nil, nil, closingError("getw", path)
229 }
230
231 cpath := C.CString(path)
232@@ -506,7 +538,7 @@
233 rc, cerr := C.zoo_wget(conn.handle, cpath, C.watch_handler, unsafe.Pointer(watchId), cbuffer, &cbufferLen, &cstat.c)
234 if rc != C.ZOK {
235 conn.forgetWatch(watchId)
236- return "", nil, nil, zkError(rc, cerr)
237+ return "", nil, nil, zkError(rc, cerr, "getw", path)
238 }
239
240 result := C.GoStringN(cbuffer, cbufferLen)
241@@ -519,7 +551,7 @@
242 conn.mutex.RLock()
243 defer conn.mutex.RUnlock()
244 if conn.handle == nil {
245- return nil, nil, ZCLOSING
246+ return nil, nil, closingError("children", path)
247 }
248
249 cpath := C.CString(path)
250@@ -536,7 +568,7 @@
251 if rc == C.ZOK {
252 stat = &cstat
253 } else {
254- err = zkError(rc, cerr)
255+ err = zkError(rc, cerr, "children", path)
256 }
257 return
258 }
259@@ -549,7 +581,7 @@
260 conn.mutex.RLock()
261 defer conn.mutex.RUnlock()
262 if conn.handle == nil {
263- return nil, nil, nil, ZCLOSING
264+ return nil, nil, nil, closingError("childrenw", path)
265 }
266
267 cpath := C.CString(path)
268@@ -570,7 +602,7 @@
269 watch = watchChannel
270 } else {
271 conn.forgetWatch(watchId)
272- err = zkError(rc, cerr)
273+ err = zkError(rc, cerr, "childrenw", path)
274 }
275 return
276 }
277@@ -595,7 +627,7 @@
278 conn.mutex.RLock()
279 defer conn.mutex.RUnlock()
280 if conn.handle == nil {
281- return nil, ZCLOSING
282+ return nil, closingError("exists", path)
283 }
284
285 cpath := C.CString(path)
286@@ -610,7 +642,7 @@
287 if rc == C.ZOK {
288 stat = &cstat
289 } else if rc != C.ZNONODE {
290- err = zkError(rc, cerr)
291+ err = zkError(rc, cerr, "exists", path)
292 }
293 return
294 }
295@@ -624,7 +656,7 @@
296 conn.mutex.RLock()
297 defer conn.mutex.RUnlock()
298 if conn.handle == nil {
299- return nil, nil, ZCLOSING
300+ return nil, nil, closingError("existsw", path)
301 }
302
303 cpath := C.CString(path)
304@@ -638,7 +670,7 @@
305 // We diverge a bit from the usual here: a ZNONODE is not an error
306 // for an exists call, otherwise every Exists call would have to check
307 // for err != nil and err.Code() != ZNONODE.
308- switch Error(rc) {
309+ switch ErrorCode(rc) {
310 case ZOK:
311 stat = &cstat
312 watch = watchChannel
313@@ -646,7 +678,7 @@
314 watch = watchChannel
315 default:
316 conn.forgetWatch(watchId)
317- err = zkError(rc, cerr)
318+ err = zkError(rc, cerr, "existsw", path)
319 }
320 return
321 }
322@@ -664,7 +696,7 @@
323 conn.mutex.RLock()
324 defer conn.mutex.RUnlock()
325 if conn.handle == nil {
326- return "", ZCLOSING
327+ return "", closingError("close", path)
328 }
329
330 cpath := C.CString(path)
331@@ -684,7 +716,7 @@
332 if rc == C.ZOK {
333 pathCreated = C.GoString(cpathCreated)
334 } else {
335- err = zkError(rc, cerr)
336+ err = zkError(rc, cerr, "create", path)
337 }
338 return
339 }
340@@ -701,7 +733,7 @@
341 conn.mutex.RLock()
342 defer conn.mutex.RUnlock()
343 if conn.handle == nil {
344- return nil, ZCLOSING
345+ return nil, closingError("set", path)
346 }
347
348 cpath := C.CString(path)
349@@ -714,7 +746,7 @@
350 if rc == C.ZOK {
351 stat = &cstat
352 } else {
353- err = zkError(rc, cerr)
354+ err = zkError(rc, cerr, "set", path)
355 }
356 return
357 }
358@@ -726,13 +758,13 @@
359 conn.mutex.RLock()
360 defer conn.mutex.RUnlock()
361 if conn.handle == nil {
362- return ZCLOSING
363+ return closingError("delete", path)
364 }
365
366 cpath := C.CString(path)
367 defer C.free(unsafe.Pointer(cpath))
368 rc, cerr := C.zoo_delete(conn.handle, cpath, C.int(version))
369- return zkError(rc, cerr)
370+ return zkError(rc, cerr, "delete", path)
371 }
372
373 // AddAuth adds a new authentication certificate to the ZooKeeper
374@@ -744,7 +776,7 @@
375 conn.mutex.RLock()
376 defer conn.mutex.RUnlock()
377 if conn.handle == nil {
378- return ZCLOSING
379+ return closingError("addauth", "")
380 }
381
382 cscheme := C.CString(scheme)
383@@ -760,13 +792,13 @@
384
385 rc, cerr := C.zoo_add_auth(conn.handle, cscheme, ccert, C.int(len(cert)), C.handle_void_completion, unsafe.Pointer(data))
386 if rc != C.ZOK {
387- return zkError(rc, cerr)
388+ return zkError(rc, cerr, "addauth", "")
389 }
390
391 C.wait_for_completion(data)
392
393 rc = C.int(uintptr(data.data))
394- return zkError(rc, nil)
395+ return zkError(rc, nil, "addauth", "")
396 }
397
398 // ACL returns the access control list for path.
399@@ -774,7 +806,7 @@
400 conn.mutex.RLock()
401 defer conn.mutex.RUnlock()
402 if conn.handle == nil {
403- return nil, nil, ZCLOSING
404+ return nil, nil, closingError("acl", path)
405 }
406
407 cpath := C.CString(path)
408@@ -785,7 +817,7 @@
409 var cstat Stat
410 rc, cerr := C.zoo_get_acl(conn.handle, cpath, &caclv, &cstat.c)
411 if rc != C.ZOK {
412- return nil, nil, zkError(rc, cerr)
413+ return nil, nil, zkError(rc, cerr, "acl", path)
414 }
415
416 aclv := parseACLVector(&caclv)
417@@ -798,7 +830,7 @@
418 conn.mutex.RLock()
419 defer conn.mutex.RUnlock()
420 if conn.handle == nil {
421- return ZCLOSING
422+ return closingError("setacl", path)
423 }
424
425 cpath := C.CString(path)
426@@ -808,7 +840,7 @@
427 defer C.deallocate_ACL_vector(caclv)
428
429 rc, cerr := C.zoo_set_acl(conn.handle, cpath, C.int(version), caclv)
430- return zkError(rc, cerr)
431+ return zkError(rc, cerr, "setacl", path)
432 }
433
434 func parseACLVector(caclv *C.struct_ACL_vector) []ACL {
435@@ -890,7 +922,7 @@
436 func (conn *Conn) RetryChange(path string, flags int, acl []ACL, changeFunc ChangeFunc) error {
437 for {
438 oldValue, oldStat, err := conn.Get(path)
439- if err != nil && err != ZNONODE {
440+ if err != nil && !IsError(err, ZNONODE) {
441 return err
442 }
443 newValue, err := changeFunc(oldValue, oldStat)
444@@ -899,7 +931,7 @@
445 }
446 if oldStat == nil {
447 _, err := conn.Create(path, newValue, flags, acl)
448- if err == nil || err != ZNODEEXISTS {
449+ if err == nil || !IsError(err, ZNODEEXISTS) {
450 return err
451 }
452 continue
453@@ -908,7 +940,7 @@
454 return nil // Nothing to do.
455 }
456 _, err = conn.Set(path, newValue, oldStat.Version())
457- if err == nil || (err != ZBADVERSION && err != ZNONODE) {
458+ if err == nil || !IsError(err, ZBADVERSION) && !IsError(err, ZNONODE) {
459 return err
460 }
461 }
462
463=== modified file 'zk_test.go'
464--- zk_test.go 2012-02-27 17:15:50 +0000
465+++ zk_test.go 2012-03-15 15:14:18 +0000
466@@ -1,6 +1,7 @@
467 package zookeeper_test
468
469 import (
470+ "errors"
471 . "launchpad.net/gocheck"
472 zk "launchpad.net/gozk/zookeeper"
473 "time"
474@@ -25,7 +26,45 @@
475 }
476 c.Assert(conn, IsNil)
477 c.Assert(watch, IsNil)
478- c.Assert(err, ErrorMatches, "invalid argument")
479+ c.Assert(err, ErrorMatches, "zookeeper: dial: invalid argument")
480+}
481+
482+func (s *S) TestErrorMessages(c *C) {
483+ tests := []struct {
484+ err zk.Error
485+ msg string
486+ }{{
487+ zk.Error{
488+ Op: "foo",
489+ Code: zk.ZNONODE,
490+ Path: "/blah",
491+ },
492+ `zookeeper: foo "/blah": no node`,
493+ }, {
494+ zk.Error{
495+ Op: "foo",
496+ Code: zk.ZNONODE,
497+ },
498+ `zookeeper: foo: no node`,
499+ }, {
500+ zk.Error{
501+ Op: "foo",
502+ Code: zk.ZSYSTEMERROR,
503+ SystemError: errors.New("an error"),
504+ Path: "/blah",
505+ },
506+ `zookeeper: foo "/blah": an error`,
507+ }, {
508+ zk.Error{
509+ Op: "foo",
510+ Code: zk.ZSYSTEMERROR,
511+ Path: "/blah",
512+ },
513+ `zookeeper: foo "/blah": system error`,
514+ }}
515+ for _, t := range tests {
516+ c.Check(t.err.Error(), Equals, t.msg)
517+ }
518 }
519
520 func (s *S) TestRecvTimeoutInitParameter(c *C) {
521@@ -42,7 +81,7 @@
522 for i := 0; i != 1000; i++ {
523 _, _, err := conn.Get("/zookeeper")
524 if err != nil {
525- c.Assert(err, ErrorMatches, "operation timeout")
526+ c.Check(zk.IsError(err, zk.ZOPERATIONTIMEOUT), Equals, true, Commentf("%v", err))
527 c.SucceedNow()
528 }
529 }
530@@ -158,8 +197,8 @@
531
532 c.Assert(data, Equals, "")
533 c.Assert(stat, IsNil)
534- c.Assert(err, ErrorMatches, "no node")
535- c.Assert(err, Equals, zk.ZNONODE)
536+ c.Assert(err, ErrorMatches, `zookeeper: get "/non-existent": no node`)
537+ c.Check(zk.IsError(err, zk.ZNONODE), Equals, true, Commentf("%v", err))
538 }
539
540 func (s *S) TestCreateAndGet(c *C) {
541@@ -171,7 +210,7 @@
542
543 // Check the error condition from Create().
544 _, err = conn.Create(path, "", zk.EPHEMERAL, zk.WorldACL(zk.PERM_ALL))
545- c.Assert(err, ErrorMatches, "node exists")
546+ c.Check(zk.IsError(err, zk.ZNODEEXISTS), Equals, true, Commentf("%v", err))
547
548 data, _, err := conn.Get(path)
549 c.Assert(err, IsNil)
550@@ -270,7 +309,7 @@
551
552 _, _, watch, err := conn.GetW("/test")
553 c.Assert(err, NotNil)
554- c.Assert(err, Equals, zk.ZNONODE)
555+ c.Check(zk.IsError(err, zk.ZNONODE), Equals, true, Commentf("%v", err))
556 c.Assert(watch, IsNil)
557
558 c.Check(zk.CountPendingWatches(), Equals, 1)
559@@ -304,7 +343,7 @@
560 c.Assert(err, IsNil)
561 err = conn.Close()
562 c.Assert(err, NotNil)
563- c.Assert(err, Equals, zk.ZCLOSING)
564+ c.Check(zk.IsError(err, zk.ZCLOSING), Equals, true, Commentf("%v", err))
565 }
566
567 func (s *S) TestChildren(c *C) {
568@@ -316,7 +355,7 @@
569 c.Assert(stat.NumChildren(), Equals, 1)
570
571 children, stat, err = conn.Children("/non-existent")
572- c.Assert(err, Equals, zk.ZNONODE)
573+ c.Check(zk.IsError(err, zk.ZNONODE), Equals, true, Commentf("%v", err))
574 c.Assert(children, IsNil)
575 c.Assert(stat, IsNil)
576 }
577@@ -383,7 +422,7 @@
578
579 _, stat, watch, err := conn.ChildrenW("/test")
580 c.Assert(err, NotNil)
581- c.Assert(err, Equals, zk.ZNONODE)
582+ c.Check(zk.IsError(err, zk.ZNONODE), Equals, true, Commentf("%v", err))
583 c.Assert(watch, IsNil)
584 c.Assert(stat, IsNil)
585
586@@ -447,7 +486,7 @@
587
588 stat, watch, err := conn.ExistsW("///")
589 c.Assert(err, NotNil)
590- c.Assert(err, Equals, zk.ZBADARGUMENTS)
591+ c.Check(zk.IsError(err, zk.ZBADARGUMENTS), Equals, true, Commentf("%v", err))
592 c.Assert(stat, IsNil)
593 c.Assert(watch, IsNil)
594
595@@ -462,14 +501,14 @@
596
597 err = conn.Delete("/test", 5)
598 c.Assert(err, NotNil)
599- c.Assert(err, Equals, zk.ZBADVERSION)
600+ c.Check(zk.IsError(err, zk.ZBADVERSION), Equals, true, Commentf("%v", err))
601
602 err = conn.Delete("/test", -1)
603 c.Assert(err, IsNil)
604
605 err = conn.Delete("/test", -1)
606 c.Assert(err, NotNil)
607- c.Assert(err, Equals, zk.ZNONODE)
608+ c.Check(zk.IsError(err, zk.ZNONODE), Equals, true, Commentf("%v", err))
609 }
610
611 func (s *S) TestClientIdAndReInit(c *C) {
612@@ -518,7 +557,7 @@
613
614 acl, stat, err = conn.ACL("/non-existent")
615 c.Assert(err, NotNil)
616- c.Assert(err, Equals, zk.ZNONODE)
617+ c.Check(zk.IsError(err, zk.ZNONODE), Equals, true, Commentf("%v", err))
618 c.Assert(acl, IsNil)
619 c.Assert(stat, IsNil)
620 }
621@@ -531,7 +570,7 @@
622
623 err = conn.SetACL("/test", zk.WorldACL(zk.PERM_ALL), 5)
624 c.Assert(err, NotNil)
625- c.Assert(err, Equals, zk.ZBADVERSION)
626+ c.Check(zk.IsError(err, zk.ZBADVERSION), Equals, true, Commentf("%v", err))
627
628 err = conn.SetACL("/test", zk.WorldACL(zk.PERM_READ), -1)
629 c.Assert(err, IsNil)
630@@ -551,7 +590,7 @@
631
632 _, _, err = conn.Get("/test")
633 c.Assert(err, NotNil)
634- c.Assert(err, Equals, zk.ZNOAUTH)
635+ c.Check(zk.IsError(err, zk.ZNOAUTH), Equals, true, Commentf("%v", err))
636
637 err = conn.AddAuth("digest", "joe:passwd")
638 c.Assert(err, IsNil)

Subscribers

People subscribed via source and target branches