Merge pull request #49 from aandryashin/master

Added JSON error resposes
aerokube · Apr 15, 2017 · f82759e · f82759e
2 parents 466292f + 79c8328
commit f82759e
Show file tree

Hide file tree

Showing 3 changed files with 123 additions and 52 deletions.
diff --git a/proxy.go b/proxy.go
@@ -110,10 +110,10 @@ func (h *Host) session(ctx context.Context, header http.Header, c caps) (map[str
 	return reply, browserStarted
 }
 
-func reply(w http.ResponseWriter, msg map[string]interface{}) {
-	reply, _ := json.Marshal(msg)
+func reply(w http.ResponseWriter, msg map[string]interface{}, status int) {
 	w.Header().Set("Content-Type", "application/json")
-	w.Write(reply)
+	w.WriteHeader(status)
+	json.NewEncoder(w).Encode(msg)
 }
 
 func serial() uint64 {
@@ -159,14 +159,13 @@ func browserErrMsg(js map[string]interface{}) string {
 	return msg
 }
 
-func jsonErrMsg(msg string) string {
-	message := make(map[string]string)
-	message["message"] = msg
-	value := make(map[string]interface{})
-	value["value"] = message
-	value["status"] = 13
-	result, _ := json.Marshal(value)
-	return string(result)
+func errMsg(msg string) map[string]interface{} {
+	return map[string]interface{}{
+		"value": map[string]string{
+			"message": msg,
+		},
+		"status": 13,
+	}
 }
 
 func route(w http.ResponseWriter, r *http.Request) {
@@ -176,13 +175,13 @@ func route(w http.ResponseWriter, r *http.Request) {
 	var c caps
 	err := json.NewDecoder(r.Body).Decode(&c)
 	if err != nil {
-		http.Error(w, fmt.Sprintf("bad json format: %s", err.Error()), http.StatusBadRequest)
+		reply(w, errMsg(fmt.Sprintf("bad json format: %s", err.Error())), http.StatusBadRequest)
 		log.Printf("[%d] [BAD_JSON] [%s] [%s] [%v]\n", id, user, remote, err)
 		return
 	}
 	browser, version := c.browser(), c.version()
 	if browser == "" {
-		http.Error(w, "browser not set", http.StatusBadRequest)
+		reply(w, errMsg("browser not set"), http.StatusBadRequest)
 		log.Printf("[%d] [BROWSER_NOT_SET] [%s] [%s]\n", id, user, remote)
 		return
 	}
@@ -193,7 +192,7 @@ func route(w http.ResponseWriter, r *http.Request) {
 	hosts, version, excludedRegions := browsers.find(browser, version, excludedHosts, newSet())
 	confLock.RUnlock()
 	if len(hosts) == 0 {
-		http.Error(w, fmt.Sprintf("unsupported browser: %s", fmtBrowser(browser, version)), http.StatusNotFound)
+		reply(w, errMsg(fmt.Sprintf("unsupported browser: %s", fmtBrowser(browser, version))), http.StatusNotFound)
 		log.Printf("[%d] [UNSUPPORTED_BROWSER] [%s] [%s] [%s]\n", id, user, remote, fmtBrowser(browser, version))
 		return
 	}
@@ -216,7 +215,7 @@ loop:
 		case browserStarted:
 			sess := resp["sessionId"].(string)
 			resp["sessionId"] = h.sum() + sess
-			reply(w, resp)
+			reply(w, resp, http.StatusOK)
 			log.Printf("[%d] [%.2fs] [SESSION_CREATED] [%s] [%s] [%s] [%s] [%s] [%d]\n", id, float64(time.Now().Sub(start).Seconds()), user, remote, fmtBrowser(browser, version), h.net(), sess, count)
 			return
 		case browserFailed:
@@ -231,7 +230,7 @@ loop:
 			break loop
 		}
 	}
-	http.Error(w, jsonErrMsg(fmt.Sprintf("cannot create session %s on any hosts after %d attempt(s)", fmtBrowser(browser, version), count)), http.StatusInternalServerError)
+	reply(w, errMsg(fmt.Sprintf("cannot create session %s on any hosts after %d attempt(s)", fmtBrowser(browser, version), count)), http.StatusInternalServerError)
 	log.Printf("[%d] [SESSION_NOT_CREATED] [%s] [%s] [%s]\n", id, user, remote, fmtBrowser(browser, version))
 }
 
@@ -279,13 +278,13 @@ func ping(w http.ResponseWriter, r *http.Request) {
 }
 
 func err(w http.ResponseWriter, r *http.Request) {
-	http.Error(w, "route not found", http.StatusNotFound)
+	reply(w, errMsg("route not found"), http.StatusNotFound)
 }
 
 func postOnly(handler http.HandlerFunc) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if r.Method != http.MethodPost {
-			http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+			reply(w, errMsg("method not allowed"), http.StatusMethodNotAllowed)
 			return
 		}
 		handler(w, r)

diff --git a/proxy_test.go b/proxy_test.go
@@ -10,7 +10,6 @@ import (
 	"net/http/httptest"
 	"net/url"
 	"strconv"
-	"strings"
 	"sync"
 	"testing"
 
@@ -32,21 +31,6 @@ const (
 	password = "test"
 )
 
-type Body struct {
-	B string
-}
-
-func (m Body) Match(i interface{}) bool {
-	rsp := i.(*http.Response)
-	body, _ := ioutil.ReadAll(rsp.Body)
-	rsp.Body.Close()
-	return EqualTo{m.B}.Match(strings.TrimSpace(string(body)))
-}
-
-func (m Body) String() string {
-	return fmt.Sprintf("response body %v", m.B)
-}
-
 type Message struct {
 	B string
 }
@@ -98,14 +82,14 @@ func TestPing(t *testing.T) {
 	rsp, err := http.Get(gridrouter("/ping"))
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, Body{"Ok"}})
+	AssertThat(t, rsp, Code{http.StatusOK})
 }
 
 func TestErr(t *testing.T) {
 	rsp, err := http.Get(gridrouter("/err"))
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Body{"route not found"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Message{"route not found"}})
 }
 
 func TestCreateSessionGet(t *testing.T) {
@@ -115,7 +99,7 @@ func TestCreateSessionGet(t *testing.T) {
 	rsp, err := client.Do(req)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusMethodNotAllowed}, Body{"method not allowed"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusMethodNotAllowed}, Message{"method not allowed"}})
 }
 
 func TestUnauthorized(t *testing.T) {
@@ -129,49 +113,49 @@ func TestCreateSessionEmptyBody(t *testing.T) {
 	rsp, err := createSessionFromReader(nil)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Body{"bad json format: EOF"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Message{"bad json format: EOF"}})
 }
 
 func TestCreateSessionBadJson(t *testing.T) {
 	rsp, err := createSession("")
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Body{"bad json format: EOF"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Message{"bad json format: EOF"}})
 }
 
 func TestCreateSessionCapsNotSet(t *testing.T) {
 	rsp, err := createSession("{}")
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Body{"browser not set"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Message{"browser not set"}})
 }
 
 func TestCreateSessionBrowserNotSet(t *testing.T) {
 	rsp, err := createSession(`{"desiredCapabilities":{}}`)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Body{"browser not set"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Message{"browser not set"}})
 }
 
 func TestCreateSessionBadBrowserName(t *testing.T) {
 	rsp, err := createSession(`{"desiredCapabilities":{"browserName":false}}`)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Body{"browser not set"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusBadRequest}, Message{"browser not set"}})
 }
 
 func TestCreateSessionUnsupportedBrowser(t *testing.T) {
 	rsp, err := createSession(`{"desiredCapabilities":{"browserName":"mosaic"}}`)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Body{"unsupported browser: mosaic"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Message{"unsupported browser: mosaic"}})
 }
 
 func TestCreateSessionUnsupportedBrowserVersion(t *testing.T) {
 	rsp, err := createSession(`{"desiredCapabilities":{"browserName":"mosaic", "version":"1.0"}}`)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Body{"unsupported browser: mosaic-1.0"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Message{"unsupported browser: mosaic-1.0"}})
 }
 
 func createSession(capabilities string) (*http.Response, error) {
@@ -232,14 +216,14 @@ func TestSessionEmptyHash(t *testing.T) {
 	rsp, err := http.Get(gridrouter("/wd/hub/session/"))
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Body{"route not found"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Message{"route not found"}})
 }
 
 func TestSessionWrongHash(t *testing.T) {
 	rsp, err := http.Get(gridrouter("/wd/hub/session/012345678901234567890123456789012"))
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Body{"route not found"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusNotFound}, Message{"route not found"}})
 }
 
 func TestStartSession(t *testing.T) {
@@ -271,9 +255,10 @@ func TestStartSession(t *testing.T) {
 	}()
 
 	rsp, err := createSession(`{"desiredCapabilities":{"browserName":"browser", "version":"1.0"}}`)
-
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, Body{`{"sessionId":"` + node.sum() + `123"}`}})
+	var sess map[string]string
+	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, IsJson{&sess}})
+	AssertThat(t, sess["sessionId"], EqualTo{fmt.Sprintf("%s123", node.sum())})
 }
 
 func TestStartSessionWithJsonSpecChars(t *testing.T) {
@@ -303,7 +288,9 @@ func TestStartSessionWithJsonSpecChars(t *testing.T) {
 	rsp, err := createSession(`{"desiredCapabilities":{"browserName":"{browser}", "version":"1.0"}}`)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, Body{`{"sessionId":"` + node.sum() + `123"}`}})
+	var sess map[string]string
+	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, IsJson{&sess}})
+	AssertThat(t, sess["sessionId"], EqualTo{fmt.Sprintf("%s123", node.sum())})
 }
 
 func TestStartSessionWithPrefixVersion(t *testing.T) {
@@ -608,7 +595,7 @@ func TestDeleteSession(t *testing.T) {
 func TestProxyRequest(t *testing.T) {
 	mux := http.NewServeMux()
 	mux.HandleFunc("/wd/hub/session/", func(w http.ResponseWriter, r *http.Request) {
-		w.Write([]byte("response"))
+		w.Write([]byte(`{"value":{"message":"response"}}`))
 	})
 	selenium := httptest.NewServer(mux)
 	defer selenium.Close()
@@ -634,7 +621,7 @@ func TestProxyRequest(t *testing.T) {
 	rsp, err := http.DefaultClient.Do(r)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, Body{"response"}})
+	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, Message{"response"}})
 }
 
 func TestProxyJsonRequest(t *testing.T) {
@@ -776,5 +763,7 @@ func TestStartSessionProxyHeaders(t *testing.T) {
 	rsp, err := createSession(`{"desiredCapabilities":{"browserName":"browser", "version":"1.0"}}`)
 
 	AssertThat(t, err, Is{nil})
-	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, Body{`{"sessionId":"` + node.sum() + `123"}`}})
+	var sess map[string]string
+	AssertThat(t, rsp, AllOf{Code{http.StatusOK}, IsJson{&sess}})
+	AssertThat(t, sess["sessionId"], EqualTo{fmt.Sprintf("%s123", node.sum())})
 }
diff --git a/quota/test.xml b/quota/test.xml
@@ -0,0 +1,83 @@
+<qa:browsers xmlns:qa="urn:config.gridrouter.qatools.ru">
+    <browser name="firefox" defaultVersion="52.0">
+        <version number="52.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="5"/>
+            </region>
+        </version>
+        <version number="51.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="5"/>
+            </region>
+        </version>
+        <version number="50.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="5"/>
+            </region>
+        </version>
+        <version number="49.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="5"/>
+            </region>
+        </version>
+        <version number="48.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="5"/>
+            </region>
+        </version>
+    </browser>
+    <browser name="chrome" defaultVersion="57.0">
+        <version number="57.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+        <version number="56.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+        <version number="55.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+        <version number="54.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+        <version number="53.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+    </browser>
+    <browser name="opera" defaultVersion="44.0">
+        <version number="44.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+        <version number="43.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+        <version number="42.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+        <version number="41.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+        <version number="40.0">
+            <region name="local">
+                <host name="localhost" port="5555" count="1"/>
+            </region>
+        </version>
+    </browser>
+</qa:browsers>