Merge pull request #1435 from xiangli-cmu/json

etcdhttp: make admin HTTP endpoint return json format error

etcdserver/etcdhttp/client.go

@@ -161,7 +161,7 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	switch r.Method {
 	case "GET":
 		if trimPrefix(r.URL.Path, adminMembersPrefix) != "" {
-			http.NotFound(w, r)
+			writeError(w, httptypes.NewHTTPError(http.StatusNotFound, "Not found"))
 			return
 		}
 		mc := newMemberCollection(h.clusterInfo.Members())
@@ -172,22 +172,22 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 	case "POST":
 		ctype := r.Header.Get("Content-Type")
 		if ctype != "application/json" {
-			http.Error(w, fmt.Sprintf("bad Content-Type %s, accept application/json", ctype), http.StatusBadRequest)
+			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Bad Content-Type %s, accept application/json", ctype)))
 			return
 		}
 		b, err := ioutil.ReadAll(r.Body)
 		if err != nil {
-			http.Error(w, err.Error(), http.StatusBadRequest)
+			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error()))
 			return
 		}
 		raftAttr := etcdserver.RaftAttributes{}
 		if err := json.Unmarshal(b, &raftAttr); err != nil {
-			http.Error(w, err.Error(), http.StatusBadRequest)
+			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error()))
 			return
 		}
 		validURLs, err := types.NewURLs(raftAttr.PeerURLs)
 		if err != nil {
-			http.Error(w, "bad peer urls", http.StatusBadRequest)
+			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, "Bad peer urls"))
 			return
 		}
 		now := h.clock.Now()
@@ -211,7 +211,7 @@ func (h *adminMembersHandler) ServeHTTP(w http.ResponseWriter, r *http.Request)
 		}
 		id, err := strconv.ParseUint(idStr, 16, 64)
 		if err != nil {
-			http.Error(w, err.Error(), http.StatusBadRequest)
+			writeError(w, httptypes.NewHTTPError(http.StatusBadRequest, err.Error()))
 			return
 		}
 		log.Printf("etcdhttp: remove node %x", id)

etcdserver/etcdhttp/client_test.go

@@ -575,8 +575,8 @@ func TestServeAdminMembers(t *testing.T) {
 	}{
 		{adminMembersPrefix, http.StatusOK, "application/json", wmc},
 		{adminMembersPrefix + "/", http.StatusOK, "application/json", wmc},
-		{path.Join(adminMembersPrefix, "100"), http.StatusNotFound, "text/plain; charset=utf-8", "404 page not found\n"},
-		{path.Join(adminMembersPrefix, "foobar"), http.StatusNotFound, "text/plain; charset=utf-8", "404 page not found\n"},
+		{path.Join(adminMembersPrefix, "100"), http.StatusNotFound, "application/json", `{"message":"Not found"}`},
+		{path.Join(adminMembersPrefix, "foobar"), http.StatusNotFound, "application/json", `{"message":"Not found"}`},
 	}
 
 	for i, tt := range tests {

etcdserver/etcdhttp/http.go

@@ -24,6 +24,7 @@ import (
 	"time"
 
 	etcdErr "github.com/coreos/etcd/error"
+	"github.com/coreos/etcd/etcdserver/etcdhttp/httptypes"
 )
 
 const (
@@ -44,9 +45,12 @@ func writeError(w http.ResponseWriter, err error) {
 		return
 	}
 	log.Println(err)
-	if e, ok := err.(*etcdErr.Error); ok {
+	switch e := err.(type) {
+	case *etcdErr.Error:
 		e.WriteTo(w)
-	} else {
+	case *httptypes.HTTPError:
+		e.WriteTo(w)
+	default:
 		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
 	}
 }

etcdserver/etcdhttp/httptypes/errors.go

@@ -0,0 +1,50 @@
+/*
+   Copyright 2014 CoreOS, Inc.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package httptypes
+
+import (
+	"encoding/json"
+	"net/http"
+)
+
+type HTTPError struct {
+	Message string `json:"message"`
+	// HTTP return code
+	Code int `json:"-"`
+}
+
+func (e HTTPError) Error() string {
+	return e.Message
+}
+
+// TODO(xiangli): handle http write errors
+func (e HTTPError) WriteTo(w http.ResponseWriter) {
+	w.WriteHeader(e.Code)
+	w.Header().Set("Content-Type", "application/json")
+	b, err := json.Marshal(e)
+	if err != nil {
+		panic("unexpected json marshal error")
+	}
+	w.Write(b)
+}
+
+func NewHTTPError(code int, m string) *HTTPError {
+	return &HTTPError{
+		Message: m,
+		Code:    code,
+	}
+}