Skip to content

Commit

Permalink
etcdserver, client: propagate and print detailed error message
Browse files Browse the repository at this point in the history
Current etcdserver just returns an error message "Internal Server
Error" in a case of internal error. This message isn't friendly and
not so useful for diagnosis. In addition, etcdctl just reports
"client: etcd cluster is unavailable or misconfigured" in such a case.

This commit improves the error message. The new body of error response
is now generated based on an error code of etcdserver. The client
constructs more friendly error message based on the response.

Below is an example:

Before:
 $ etcdctl member add infra6 http://127.0.0.1:32338
 client: etcd cluster is unavailable or misconfigured

After:
 $ etcdctl member add infra6 http://127.0.0.1:32338
 error #0: client: etcd member http://127.0.0.1:12379: etcdserver: re-configuration failed due to not enough started members
 error etcd-io#1: client: etcd member http://127.0.0.1:22379: etcdserver: re-configuration failed due to not enough started members
 error etcd-io#2: client: etcd member http://127.0.0.1:32379: etcdserver: re-configuration failed due to not enough started members
  • Loading branch information
mitake committed Nov 5, 2015
1 parent 2fd2f17 commit 6603a69
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 2 deletions.
14 changes: 13 additions & 1 deletion client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"errors"
"fmt"
"io/ioutil"
"encoding/json"
"math/rand"
"net"
"net/http"
Expand Down Expand Up @@ -282,10 +283,21 @@ func (c *httpClusterClient) Do(ctx context.Context, act httpAction) (*http.Respo
continue
}
if resp.StatusCode/100 == 5 {
type errMessage struct {
Message string
}

var msg errMessage
uerr := json.Unmarshal(body, &msg)
if uerr != nil {
// TODO: how to handle this case?
fmt.Errorf("json.Unmarshal() failed: %s", uerr)
}

switch resp.StatusCode {
case http.StatusInternalServerError, http.StatusServiceUnavailable:
// TODO: make sure this is a no leader response
cerr.Errors = append(cerr.Errors, fmt.Errorf("client: etcd member %s has no leader", eps[k].String()))
cerr.Errors = append(cerr.Errors, fmt.Errorf("client: etcd member %s: %s", eps[k].String(), msg.Message))

This comment has been minimized.

Copy link
@yichengq

yichengq Nov 6, 2015

we need to ensure has no leader error returns correctly before moving forward IMO, or it may break current working cases :(

This comment has been minimized.

Copy link
@mitake

mitake Nov 6, 2015

Author Owner

@yichengq thanks for your comment. Actually, this change is breaking a test case, sorry for that.

default:
cerr.Errors = append(cerr.Errors, fmt.Errorf("client: etcd member %s returns server error [%s]", eps[k].String(), http.StatusText(resp.StatusCode)))
}
Expand Down
2 changes: 1 addition & 1 deletion etcdserver/etcdhttp/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func writeError(w http.ResponseWriter, r *http.Request, err error) {
default:
plog.Errorf("got unexpected response error (%v)", err)
}
herr := httptypes.NewHTTPError(http.StatusInternalServerError, "Internal Server Error")
herr := httptypes.NewHTTPError(http.StatusInternalServerError, err.Error())
if et := herr.WriteTo(w); et != nil {
plog.Debugf("error writing HTTPError (%v) to %s", et, r.RemoteAddr)
}
Expand Down

0 comments on commit 6603a69

Please sign in to comment.