Better client health check and error handling

command/agent/agent_endpoint.go

@@ -1,6 +1,7 @@
 package agent
 
 import (
+	"encoding/json"
 	"net"
 	"net/http"
 	"sort"
@@ -338,11 +339,18 @@ func (s *HTTPServer) HealthRequest(resp http.ResponseWriter, req *http.Request)
 
 	// If we should check the client and it exists assume it's healthy
 	if client := s.agent.Client(); getClient && client != nil {
+		if len(client.GetServers()) == 0 {
+			health.Client = &healthResponseAgent{
+				Ok:      false,
+				Message: "no known servers",
+			}
+		} else {
 			health.Client = &healthResponseAgent{
 				Ok:      true,
 				Message: "ok",
 			}
 		}
+	}
 
 	// If we should check the server and it exists, see if there's a leader
 	if server := s.agent.Server(); getServer && server != nil {
@@ -361,13 +369,17 @@ func (s *HTTPServer) HealthRequest(resp http.ResponseWriter, req *http.Request)
 		}
 	}
 
-	if !health.ok() {
-		// At least one not-ok response, set failing status code
-		resp.WriteHeader(500)
-	}
+	if health.ok() {
 		return &health, nil
 	}
 
+	jsonResp, err := json.Marshal(&health)
+	if err != nil {
+		return nil, err
+	}
+	return nil, CodedError(500, string(jsonResp))
+}
+
 type healthResponse struct {
 	Client *healthResponseAgent `json:"client,omitempty"`
 	Server *healthResponseAgent `json:"server,omitempty"`

command/agent/agent_endpoint_test.go

@@ -746,14 +746,12 @@ func TestHTTP_AgentHealth_BadServer(t *testing.T) {
 			assert.Nil(err)
 
 			respW := httptest.NewRecorder()
-			healthI, err := s.Server.HealthRequest(respW, req)
-			assert.Nil(err)
-			assert.Equal(500, respW.Code)
-			assert.NotNil(healthI)
-			health := healthI.(*healthResponse)
-			assert.NotNil(health.Server)
-			assert.False(health.Server.Ok)
-			assert.Equal("server not enabled", health.Server.Message)
+			_, err = s.Server.HealthRequest(respW, req)
+			assert.NotNil(err)
+			httpErr, ok := err.(HTTPCodedError)
+			assert.True(ok)
+			assert.Equal(500, httpErr.Code())
+			assert.Equal(`{"server":{"ok":false,"message":"server not enabled"}}`, err.Error())
 		}
 	})
 }
@@ -791,14 +789,12 @@ func TestHTTP_AgentHealth_BadClient(t *testing.T) {
 			assert.Nil(err)
 
 			respW := httptest.NewRecorder()
-			healthI, err := s.Server.HealthRequest(respW, req)
-			assert.Nil(err)
-			assert.Equal(500, respW.Code)
-			assert.NotNil(healthI)
-			health := healthI.(*healthResponse)
-			assert.NotNil(health.Client)
-			assert.False(health.Client.Ok)
-			assert.Equal("client not enabled", health.Client.Message)
+			_, err = s.Server.HealthRequest(respW, req)
+			assert.NotNil(err)
+			httpErr, ok := err.(HTTPCodedError)
+			assert.True(ok)
+			assert.Equal(500, httpErr.Code())
+			assert.Equal(`{"client":{"ok":false,"message":"client not enabled"}}`, err.Error())
 		}
 	})
 }