fixes

- gometalinter warnings: part 1
- memory leaks in selenium and wda clients

Author: Mikhail Podtserkovskiy

handlers/apiProxy.go

@@ -8,15 +8,15 @@ import (
 	"github.com/qa-dev/jsonwire-grid/pool"
 )
 
-type ApiProxy struct {
+type APIProxy struct {
 	Pool *pool.Pool
 }
 
-func (h *ApiProxy) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
+func (h *APIProxy) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	rw.Header().Add("Content-type", "application/json")
 
 	id := r.URL.Query().Get("id")
-	nodeUrl, err := url.Parse(id) //todo: обработка ошибок
+	nodeURL, err := url.Parse(id) //todo: обработка ошибок
 
 	if err != nil {
 		errorMessage := "Error get 'id' from url: " + r.URL.String()
@@ -25,17 +25,17 @@ func (h *ApiProxy) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	node, err := h.Pool.GetNodeByAddress(nodeUrl.Host)
+	_, err = h.Pool.GetNodeByAddress(nodeURL.Host)
 
 	//todo: хардкод для ткста, сделать нормальный респонз обжекты
-	if node == nil || err != nil {
-		errorMessage := "api/proxy: Can't get node"
-		if err != nil {
-			errorMessage = err.Error()
-			log.Warning(errorMessage)
-		}
-		rw.Write([]byte(`{"msg": "Cannot find proxy with ID =` + id + ` in the registry: ` + errorMessage + `", "success": false}`))
+	if err != nil {
+		errorMessage := "api/proxy: Can't get node, " + err.Error()
+		log.Warning(errorMessage)
+		_, err = rw.Write([]byte(`{"msg": "Cannot find proxy with ID =` + id + ` in the registry: ` + errorMessage + `", "success": false}`))
 	} else {
-		rw.Write([]byte(`{"id": "", "request": {}, "msg": "proxy found !", "success": true}`))
+		_, err = rw.Write([]byte(`{"id": "", "request": {}, "msg": "proxy found !", "success": true}`))
+	}
+	if err != nil {
+		log.Errorf("api/proxy: write response, %v", err)
 	}
 }

handlers/createSession.go

@@ -35,9 +35,7 @@ func (h *CreateSession) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	}
 	err = r.Body.Close()
 	if err != nil {
-		log.Error(err.Error())
-		http.Error(rw, err.Error(), http.StatusInternalServerError)
-		return
+		log.Errorf("create/session: close request body, %v", err)
 	}
 	rc := ioutil.NopCloser(bytes.NewReader(body))
 	r.Body = rc
@@ -63,7 +61,10 @@ func (h *CreateSession) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 		return
 	}
 	rw.WriteHeader(tw.StatusCode)
-	rw.Write(tw.Output)
+	_, err = rw.Write(tw.Output)
+	if err != nil {
+		log.Errorf("create/session: write response, %v", err)
+	}
 }
 
 func (h *CreateSession) tryCreateSession(r *http.Request, capabilities *capabilities.Capabilities) (*proxy.ResponseWriter, error) {

handlers/registerNode.go

@@ -24,7 +24,10 @@ func (h *RegisterNode) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 		http.Error(rw, errorMessage, http.StatusInternalServerError)
 		return
 	}
-	r.Body.Close()
+	err = r.Body.Close()
+	if err != nil {
+		log.Errorf("register/node: close request body, %v", err)
+	}
 	var register jsonwire.Register
 	err = json.Unmarshal(body, &register)
 	if err != nil {
@@ -59,5 +62,8 @@ func (h *RegisterNode) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	}
 
 	rw.WriteHeader(http.StatusOK)
-	rw.Write([]byte("ok"))
+	_, err = rw.Write([]byte("ok"))
+	if err != nil {
+		log.Errorf("register/node: write response, %v", err)
+	}
 }

handlers/useSession.go

@@ -16,22 +16,17 @@ type UseSession struct {
 
 func (h *UseSession) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	re := regexp.MustCompile(".*/session/([^/]+)(?:/([^/]+))?")
-	parsedUrl := re.FindStringSubmatch(r.URL.Path)
-	if len(parsedUrl) != 3 {
+	parsedURL := re.FindStringSubmatch(r.URL.Path)
+	if len(parsedURL) != 3 {
 		errorMessage := "url [" + r.URL.Path + "] parsing error"
 		log.Infof(errorMessage)
 		http.Error(rw, errorMessage, http.StatusBadRequest)
 		return
 	}
-	sessionId := re.FindStringSubmatch(r.URL.Path)[1]
-	targetNode, err := h.Pool.GetNodeBySessionId(sessionId)
-	if targetNode == nil || err != nil {
-		errorMessage := ""
-		if err != nil {
-			errorMessage = err.Error()
-		}
-		// посылаем сообщение о том что сессия не найдена
-		errorMessage = "session " + sessionId + " not found in node pool: " + errorMessage
+	sessionID := re.FindStringSubmatch(r.URL.Path)[1]
+	targetNode, err := h.Pool.GetNodeBySessionID(sessionID)
+	if err != nil {
+		errorMessage := "session " + sessionID + " not found in node pool: " + err.Error()
 		log.Infof(errorMessage)
 		http.Error(rw, errorMessage, http.StatusNotFound)
 		return
@@ -44,7 +39,7 @@ func (h *UseSession) ServeHTTP(rw http.ResponseWriter, r *http.Request) {
 	proxy.ServeHTTP(rw, r)
 
 	//// todo: заговнокодим пока не появилось понимание как лучше сделать ибо сраный rest
-	if parsedUrl[2] == "" && r.Method == http.MethodDelete {
+	if parsedURL[2] == "" && r.Method == http.MethodDelete {
 		err := h.Pool.CleanUpNode(targetNode)
 		if err != nil {
 			log.Error("Clanup node status error: " + err.Error())

jsonwire/interfaces.go

@@ -7,7 +7,7 @@ type ClientFactoryInterface interface {
 type ClientInterface interface {
 	Status() (*Message, error)
 	Sessions() (*Sessions, error)
-	CloseSession(sessionId string) (*Message, error)
+	CloseSession(sessionID string) (*Message, error)
 	Address() string
 }
 

jsonwire/jsonwire.go

@@ -5,22 +5,22 @@ import (
 )
 
 type Message struct {
-	SessionId string      `json:"sessionId"`
+	SessionID string      `json:"sessionId"`
 	Status    int         `json:"status"`
 	Value     interface{} `json:"value"`
 }
 
 type NewSession struct {
 	Message
 	Value struct {
-		SessionId string `json:"sessionId"`
+		SessionID string `json:"sessionId"`
 	} `json:"value"`
 }
 
 type Sessions struct {
 	Message
 	Value []struct {
-		Id           string          `json:"id"`
+		ID           string          `json:"id"`
 		Capabilities json.RawMessage `json:"capabilities"`
 	} `json:"value"`
 }
@@ -44,12 +44,12 @@ type Configuration struct {
 	HubHost          string
 	RegisterCycle    int
 	HubPort          int
-	Url              string
+	URL              string
 	Register         bool
 	CapabilitiesList []Capabilities `json:"capabilities"` // selenium 2
 }
 
-type ApiProxy struct {
+type APIProxy struct {
 	ID      string      `json:"id"`
 	Request interface{} `json:"request"` //todo: пока не ясно зачем он нужен
 	Msg     string      `json:"msg"`

jsonwire/mocks.go

@@ -27,8 +27,8 @@ func (c *ClientMock) Sessions() (*Sessions, error) {
 	return args.Get(0).(*Sessions), args.Error(1)
 }
 
-func (c *ClientMock) CloseSession(sessionId string) (*Message, error) {
-	args := c.Called(sessionId)
+func (c *ClientMock) CloseSession(sessionID string) (*Message, error) {
+	args := c.Called(sessionID)
 	return args.Get(0).(*Message), args.Error(1)
 }
 

jsonwire/node.go

@@ -1,7 +1,6 @@
 package jsonwire
 
 import (
-	"errors"
 	"fmt"
 )
 
@@ -16,27 +15,23 @@ func NewNode(abstractClient ClientInterface) *Node {
 func (n *Node) RemoveAllSessions() (int, error) {
 	message, err := n.client.Sessions()
 	if err != nil {
-		err = errors.New(fmt.Sprintf("Can't get sessions, %s", err.Error()))
-		return 0, err
+		return 0, fmt.Errorf("Can't get sessions, %s", err.Error())
 	}
 	if message.Status != 0 {
-		err = errors.New(fmt.Sprintf("client.Sessions: Not succcess response status node, %s", n.client.Address()))
-		return 0, err
+		return 0, fmt.Errorf("client.Sessions: Not succcess response status node, %s", n.client.Address())
 	}
 	// hasn't sessions
 	countSessions := len(message.Value)
 	if countSessions == 0 {
 		return 0, nil
 	}
 	for _, session := range message.Value {
-		message, err := n.client.CloseSession(session.Id)
+		message, err := n.client.CloseSession(session.ID)
 		if err != nil {
-			err = errors.New(fmt.Sprintf("Can't close session[%s], %s", session.Id, err.Error()))
-			return 0, err
+			return 0, fmt.Errorf("Can't close session[%s], %s", session.ID, err.Error())
 		}
 		if message.Status != 0 {
-			err = errors.New(fmt.Sprintf("client.CloseSession[%s]: Not succcess response node, %s", session.Id, n.client.Address()))
-			return 0, err
+			return 0, fmt.Errorf("client.CloseSession[%s]: Not succcess response node, %s", session.ID, n.client.Address())
 		}
 	}
 	return countSessions, nil

jsonwire/response.go

@@ -8,21 +8,21 @@ import (
 type ResponseStatus int
 
 const (
-	RESPONSE_STATUS_SUCCESS     ResponseStatus = 0
-	RESPONSE_STATUS_UNKNOWN_ERR ResponseStatus = 13
+	ResponseStatusSuccess    ResponseStatus = 0
+	ResponseStatusUnknownErr ResponseStatus = 13
 )
 
 type Response struct {
-	SessionId *string         `json:"sessionId"`
+	SessionID *string         `json:"sessionId"`
 	Status    ResponseStatus  `json:"status"`
 	Value     json.RawMessage `json:"value"`
 }
 
-func NewResponse(sessionId *string, status ResponseStatus, value json.RawMessage) *Response {
-	return &Response{sessionId, status, value}
+func NewResponse(sessionID *string, status ResponseStatus, value json.RawMessage) *Response {
+	return &Response{sessionID, status, value}
 }
 
-func JsonResponse(w http.ResponseWriter, data Response, code int) (int, error) {
+func JSONResponse(w http.ResponseWriter, data Response, code int) (int, error) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	w.WriteHeader(code)
 

main.go

@@ -81,22 +81,21 @@ func main() {
 	poolInstance.SetBusyNodeDuration(busyNodeDuration)
 	poolInstance.SetReservedNodeDuration(reservedNodeDuration)
 
-	//todo: сделать конфиг для пула, вынести duration туда
-	poolMetricsSender := poolMetrics.NewSender(statsdClient, poolInstance, time.Second*1) //todo: вынести в конфиг
+	poolMetricsSender := poolMetrics.NewSender(statsdClient, poolInstance, time.Second*1) // todo: move to config
 	go poolMetricsSender.SendAll()
 
 	go func() {
 		for {
 			poolInstance.FixNodeStatuses()
-			time.Sleep(time.Minute * 5) // todo: вынести в конфиг
+			time.Sleep(time.Minute * 5) // todo: move to config
 		}
 	}()
 
 	m := middleware.NewLogMiddleware(statsdClient)
 	http.Handle("/wd/hub/session", m.Log(&handlers.CreateSession{Pool: poolInstance, ClientFactory: clientFactory})) //selenium
 	http.Handle("/session", m.Log(&handlers.CreateSession{Pool: poolInstance, ClientFactory: clientFactory}))        //wda
 	http.Handle("/grid/register", m.Log(&handlers.RegisterNode{Pool: poolInstance}))
-	http.Handle("/grid/api/proxy", &handlers.ApiProxy{Pool: poolInstance})
+	http.Handle("/grid/api/proxy", &handlers.APIProxy{Pool: poolInstance})
 	http.HandleFunc("/_info", heartbeat)
 	http.Handle("/", m.Log(&handlers.UseSession{Pool: poolInstance}))
 
@@ -118,15 +117,21 @@ func main() {
 
 	log.Info("Shutting down the server...")
 
-	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) // todo: вынести в конфиг
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) // todo: move to config
 	defer cancel()
-	server.Shutdown(ctx)
+	err = server.Shutdown(ctx)
+	if err != nil {
+		log.Fatalf("graceful shutdown, %v", err)
+	}
 
 	log.Info("Server gracefully stopped")
 }
 
 func heartbeat(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json; charset=utf-8")
 	w.WriteHeader(http.StatusOK)
-	w.Write([]byte(`{"result": {"ok": true}}`))
+	_, err := w.Write([]byte(`{"result": {"ok": true}}`))
+	if err != nil {
+		log.Errorf("write response, %v", err)
+	}
 }

pool/mocks.go

@@ -19,8 +19,8 @@ func (s *StorageMock) ReserveAvailable(nodeList []Node) (Node, error) {
 	return args.Get(0).(Node), args.Error(1)
 }
 
-func (s *StorageMock) SetBusy(node Node, sessionId string) error {
-	args := s.Called(node, sessionId)
+func (s *StorageMock) SetBusy(node Node, sessionID string) error {
+	args := s.Called(node, sessionID)
 	return args.Error(0)
 }
 
@@ -34,8 +34,8 @@ func (s *StorageMock) GetCountWithStatus(nodeStatus *NodeStatus) (int, error) {
 	return args.Int(0), args.Error(1)
 }
 
-func (s *StorageMock) GetBySession(sessionId string) (Node, error) {
-	args := s.Called(sessionId)
+func (s *StorageMock) GetBySession(sessionID string) (Node, error) {
+	args := s.Called(sessionID)
 	return args.Get(0).(Node), args.Error(1)
 }
 

pool/pool.go

@@ -83,10 +83,10 @@ func (p *Pool) GetAll() ([]Node, error) {
 	return p.storage.GetAll()
 }
 
-func (p *Pool) GetNodeBySessionId(sessionId string) (*Node, error) {
-	node, err := p.storage.GetBySession(sessionId)
+func (p *Pool) GetNodeBySessionID(sessionID string) (*Node, error) {
+	node, err := p.storage.GetBySession(sessionID)
 	if err != nil {
-		err = errors.New(fmt.Sprintf("Can't find node by session[%s], %s", sessionId, err.Error()))
+		err = fmt.Errorf("Can't find node by session[%s], %s", sessionID, err.Error())
 		log.Error(err)
 		return nil, err
 	}
@@ -96,7 +96,7 @@ func (p *Pool) GetNodeBySessionId(sessionId string) (*Node, error) {
 func (p *Pool) GetNodeByAddress(address string) (*Node, error) {
 	node, err := p.storage.GetByAddress(address)
 	if err != nil {
-		err = errors.New(fmt.Sprintf("Can't find node by address[%s], %s", address, err.Error()))
+		err = fmt.Errorf("Can't find node by address[%s], %s", address, err.Error())
 		log.Error(err)
 		return nil, err
 	}
@@ -153,7 +153,7 @@ func (p *Pool) FixNodeStatuses() {
 }
 
 func (p *Pool) fixNodeStatus(node *Node) (bool, error) {
-	nodeStatusDuration := time.Since(time.Unix(int64(node.Updated), 0))
+	nodeStatusDuration := time.Since(time.Unix(node.Updated, 0))
 	switch node.Status {
 	case NodeStatusReserved:
 		if nodeStatusDuration < p.reservedNodeDuration {
@@ -168,7 +168,7 @@ func (p *Pool) fixNodeStatus(node *Node) (bool, error) {
 	}
 	err := p.strategyList.FixNodeStatus(*node)
 	if err != nil {
-		return false, errors.New(fmt.Sprintf("Can't fix node [%s] status, %s", node.Address, err.Error()))
+		return false, fmt.Errorf("Can't fix node [%s] status, %s", node.Address, err.Error())
 	}
 	return true, nil
 }

pool/pool_test.go

@@ -116,7 +116,7 @@ func TestPool_GetNodeBySessionId_Positive(t *testing.T) {
 	s := new(StorageMock)
 	s.On("GetBySession", mock.AnythingOfType("string")).Return(Node{}, nil)
 	p := NewPool(s, new(StrategyListMock))
-	node, err := p.GetNodeBySessionId("testSessId")
+	node, err := p.GetNodeBySessionID("testSessId")
 	a.NotNil(node)
 	a.Nil(err)
 }
@@ -127,7 +127,7 @@ func TestPool_GetNodeBySessionId_Negative(t *testing.T) {
 	eError := errors.New("Error")
 	s.On("GetBySession", mock.AnythingOfType("string")).Return(Node{}, eError)
 	p := NewPool(s, new(StrategyListMock))
-	node, err := p.GetNodeBySessionId("testSessId")
+	node, err := p.GetNodeBySessionID("testSessId")
 	a.Nil(node)
 	a.Error(err)
 }

pool/strategy/kubernetes/config.go

@@ -25,7 +25,7 @@ type nodeParams struct {
 	Port  string `json:"port"`
 }
 
-func NewConfig(cfg config.Strategy) (*strategyConfig, error) {
+func newConfig(cfg config.Strategy) (*strategyConfig, error) {
 	kubConfig := new(strategyConfig)
 	kubConfig.Type = cfg.Type
 	kubConfig.Limit = cfg.Limit