etcdserver: switch to using etcd.Error

error/error.go

@@ -46,6 +46,7 @@ var errors = map[int]string{
 	EcodeIndexOrValueRequired: "Index or value is required",
 	EcodeIndexValueMutex:      "Index and value cannot both be specified",
 	EcodeInvalidField:         "Invalid field",
+	EcodeInvalidForm:          "Invalid POST form",
 
 	// raft related errors
 	EcodeRaftInternal: "Raft Internal Error",
@@ -84,6 +85,7 @@ const (
 	EcodeIndexOrValueRequired = 207
 	EcodeIndexValueMutex      = 208
 	EcodeInvalidField         = 209
+	EcodeInvalidForm          = 210
 
 	EcodeRaftInternal = 300
 	EcodeLeaderElect  = 301
@@ -104,6 +106,10 @@ type Error struct {
 	Index     uint64 `json:"index"`
 }
 
+func NewRequestError(errorCode int, cause string) *Error {
+	return NewError(errorCode, cause, 0)
+}
+
 func NewError(errorCode int, cause string, index uint64) *Error {
 	return &Error{
 		ErrorCode: errorCode,

etcdserver/etcdhttp/http.go

@@ -180,13 +180,13 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 func (h Handler) serveKeys(ctx context.Context, w http.ResponseWriter, r *http.Request) {
 	rr, err := parseRequest(r, genId())
 	if err != nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
+		writeError(w, err)
 		return
 	}
 
 	resp, err := h.Server.Do(ctx, rr)
 	if err != nil {
-		writeInternalError(w, err)
+		writeError(w, err)
 		return
 	}
 
@@ -200,7 +200,7 @@ func (h Handler) serveKeys(ctx context.Context, w http.ResponseWriter, r *http.R
 			return
 		}
 	default:
-		writeInternalError(w, errors.New("received response with no Event/Watcher!"))
+		writeError(w, errors.New("received response with no Event/Watcher!"))
 		return
 	}
 
@@ -257,11 +257,17 @@ func parseRequest(r *http.Request, id int64) (etcdserverpb.Request, error) {
 	var err error
 
 	if err = r.ParseForm(); err != nil {
-		return emptyReq, err
+		return emptyReq, etcdErr.NewRequestError(
+			etcdErr.EcodeInvalidForm,
+			err.Error(),
+		)
 	}
 
 	if !strings.HasPrefix(r.URL.Path, keysPrefix) {
-		return emptyReq, errors.New("unexpected key prefix!")
+		return emptyReq, etcdErr.NewRequestError(
+			etcdErr.EcodeInvalidForm,
+			"incorrect key prefix",
+		)
 	}
 	path := r.URL.Path[len(keysPrefix):]
 
@@ -269,24 +275,39 @@ func parseRequest(r *http.Request, id int64) (etcdserverpb.Request, error) {
 
 	var pIdx, wIdx, ttl uint64
 	if pIdx, err = parseUint64(q.Get("prevIndex")); err != nil {
-		return emptyReq, errors.New("invalid value for prevIndex")
+		return emptyReq, etcdErr.NewRequestError(
+			etcdErr.EcodeIndexNaN,
+			"invalid value for prevIndex",
+		)
 	}
 	if wIdx, err = parseUint64(q.Get("waitIndex")); err != nil {
-		return emptyReq, errors.New("invalid value for waitIndex")
+		return emptyReq, etcdErr.NewRequestError(
+			etcdErr.EcodeIndexNaN,
+			"invalid value for waitIndex",
+		)
 	}
 	if ttl, err = parseUint64(q.Get("ttl")); err != nil {
-		return emptyReq, errors.New("invalid value for ttl")
+		return emptyReq, etcdErr.NewRequestError(
+			etcdErr.EcodeTTLNaN,
+			"invalid value for ttl",
+		)
 	}
 
 	var rec, sort, wait bool
 	if rec, err = parseBool(q.Get("recursive")); err != nil {
-		return emptyReq, errors.New("invalid value for recursive")
+		return emptyReq, etcdErr.NewRequestError(
+			etcdErr.EcodeInvalidField,
+			"invalid value for recursive")
 	}
 	if sort, err = parseBool(q.Get("sorted")); err != nil {
-		return emptyReq, errors.New("invalid value for sorted")
+		return emptyReq, etcdErr.NewRequestError(
+			etcdErr.EcodeInvalidField,
+			"invalid value for sorted")
 	}
 	if wait, err = parseBool(q.Get("wait")); err != nil {
-		return emptyReq, errors.New("invalid value for wait")
+		return emptyReq, etcdErr.NewRequestError(
+			etcdErr.EcodeInvalidField,
+			"invalid value for wait")
 	}
 
 	rr := etcdserverpb.Request{
@@ -332,9 +353,10 @@ func parseUint64(s string) (uint64, error) {
 	return strconv.ParseUint(s, 10, 64)
 }
 
-// writeInternalError logs and writes the given Error to the ResponseWriter
+// writeError logs and writes the given Error to the ResponseWriter
 // If Error is an etcdErr, it is rendered to the ResponseWriter
-func writeInternalError(w http.ResponseWriter, err error) {
+// Otherwise, it is assumed to be an InternalServerError
+func writeError(w http.ResponseWriter, err error) {
 	if err == nil {
 		return
 	}

etcdserver/etcdhttp/http_test.go

@@ -56,7 +56,8 @@ func TestParseUint64(t *testing.T) {
 
 func TestBadParseRequest(t *testing.T) {
 	tests := []struct {
-		in *http.Request
+		in    *http.Request
+		wcode int
 	}{
 		{
 			// parseForm failure
@@ -64,36 +65,86 @@ func TestBadParseRequest(t *testing.T) {
 				Body:   nil,
 				Method: "PUT",
 			},
+			etcdErr.EcodeInvalidForm,
 		},
 		{
 			// bad key prefix
 			&http.Request{
 				URL: mustNewURL(t, "/badprefix/"),
 			},
+			etcdErr.EcodeInvalidForm,
 		},
 		// bad values for prevIndex, waitIndex, ttl
-		{mustNewRequest(t, "?prevIndex=foo")},
+		{
-		{mustNewRequest(t, "?prevIndex=1.5")},
+			mustNewRequest(t, "?prevIndex=foo"),
-		{mustNewRequest(t, "?prevIndex=-1")},
+			etcdErr.EcodeIndexNaN,
-		{mustNewRequest(t, "?waitIndex=garbage")},
+		},
-		{mustNewRequest(t, "?waitIndex=??")},
+		{
-		{mustNewRequest(t, "?ttl=-1")},
+			mustNewRequest(t, "?prevIndex=1.5"),
+			etcdErr.EcodeIndexNaN,
+		},
+		{
+			mustNewRequest(t, "?prevIndex=-1"),
+			etcdErr.EcodeIndexNaN,
+		},
+		{
+			mustNewRequest(t, "?waitIndex=garbage"),
+			etcdErr.EcodeIndexNaN,
+		},
+		{
+			mustNewRequest(t, "?waitIndex=??"),
+			etcdErr.EcodeIndexNaN,
+		},
+		{
+			mustNewRequest(t, "?ttl=-1"),
+			etcdErr.EcodeTTLNaN,
+		},
 		// bad values for recursive, sorted, wait
-		{mustNewRequest(t, "?recursive=hahaha")},
+		{
-		{mustNewRequest(t, "?recursive=1234")},
+			mustNewRequest(t, "?recursive=hahaha"),
-		{mustNewRequest(t, "?recursive=?")},
+			etcdErr.EcodeInvalidField,
-		{mustNewRequest(t, "?sorted=hahaha")},
+		},
-		{mustNewRequest(t, "?sorted=!!")},
+		{
-		{mustNewRequest(t, "?wait=notreally")},
+			mustNewRequest(t, "?recursive=1234"),
-		{mustNewRequest(t, "?wait=what!")},
+			etcdErr.EcodeInvalidField,
+		},
+		{
+			mustNewRequest(t, "?recursive=?"),
+			etcdErr.EcodeInvalidField,
+		},
+		{
+			mustNewRequest(t, "?sorted=hahaha"),
+			etcdErr.EcodeInvalidField,
+		},
+		{
+			mustNewRequest(t, "?sorted=!!"),
+			etcdErr.EcodeInvalidField,
+		},
+		{
+			mustNewRequest(t, "?wait=notreally"),
+			etcdErr.EcodeInvalidField,
+		},
+		{
+			mustNewRequest(t, "?wait=what!"),
+			etcdErr.EcodeInvalidField,
+		},
 	}
 	for i, tt := range tests {
 		got, err := parseRequest(tt.in, 1234)
 		if err == nil {
-			t.Errorf("case %d: unexpected nil error!", i)
+			t.Errorf("#%d: unexpected nil error!", i)
+			continue
+		}
+		ee, ok := err.(*etcdErr.Error)
+		if !ok {
+			t.Errorf("#%d: err is not etcd.Error!", i)
+			continue
+		}
+		if ee.ErrorCode != tt.wcode {
+			t.Errorf("#%d: code=%d, want %v", i, ee.ErrorCode, tt.wcode)
 		}
 		if !reflect.DeepEqual(got, etcdserverpb.Request{}) {
-			t.Errorf("case %d: unexpected non-empty Request: %#v", i, got)
+			t.Errorf("#%d: unexpected non-empty Request: %#v", i, got)
 		}
 	}
 }
@@ -205,10 +256,10 @@ func (w *eventingWatcher) EventChan() chan *store.Event {
 
 func (w *eventingWatcher) Remove() {}
 
-func TestWriteInternalError(t *testing.T) {
+func TestWriteError(t *testing.T) {
 	// nil error should not panic
 	rw := httptest.NewRecorder()
-	writeInternalError(rw, nil)
+	writeError(rw, nil)
 	h := rw.Header()
 	if len(h) > 0 {
 		t.Fatalf("unexpected non-empty headers: %#v", h)
@@ -241,7 +292,7 @@ func TestWriteInternalError(t *testing.T) {
 
 	for i, tt := range tests {
 		rw := httptest.NewRecorder()
-		writeInternalError(rw, tt.err)
+		writeError(rw, tt.err)
 		if code := rw.Code; code != tt.wcode {
 			t.Errorf("#%d: code=%d, want %d", i, code, tt.wcode)
 		}