consul-api: don't conflate network errors with API errors

[rboyer]

The HTTP api documentation has many endpoints that have some informational failures. For example if you attempt to renew a Session that does not exist or has expired it returns 404.

The go consul-api wrapper unfortunately uses the requireOK() wrapper function to mask non-200 error codes behind an opaque string.

This makes programmatic access to handle API errors (like 404s) from network hiccups difficult.

It would be nice if the go API returned an error struct instead of an error string (similar to os.PathError). That way advanced clients could do a type-assertion rather than string comparisons to properly handle the different kind of failures.

I already found one case out in the wild that has resorted to having broken code: [https://github.com/docker/swarm/blob/master/pkg/store/consul.go#L193]. In this example the authors likely thought that err != nil meant "session expired, got 404" when it could mean that or it could be any HTTP error. I believe swarm does the wrong thing and thinks that the session is invalid when it's still completely valid.

[rboyer]

This may actually only apply to Session.Renew and Session.RenewPeriodic.

[rboyer]

Arguably, acquiring a KV lock with a stale session should be distinguishable from other errors. Currently consul-server does this:

$ curl -sLi -XPUT "http://localhost:8500/v1/kv/foo/bar?acquire=$STALE_SESSION" -d"MYDATA"
HTTP/1.1 500 Internal Server Error
Date: Wed, 17 Jun 2015 16:35:52 GMT
Content-Length: 15
Content-Type: text/plain; charset=utf-8

Invalid session

The go api doesn't do anything here to help you determine if the lock acquire failed for network reasons or simply because your session expired. If the consul-server API were tweaked the go-api could be similarly tweaked.

[rboyer]

Do I have to do anything else to help move this along?

[kisunji]

@FFMMM Is this related to the work you've been doing?

[acpana]

It is! Since this should be fixed as per #11054 I'll be closing this.