Fix code path that could cause panic

http/service.go

@@ -26,6 +26,11 @@ import (
 	"github.com/rqlite/rqlite/store"
 )
 
+var (
+	// ErrLeaderNotFound is returned when a node cannot locate a leader
+	ErrLeaderNotFound = errors.New("leader not found")
+)
+
 // Database is the interface any queryable system must implement
 type Database interface {
 	// Execute executes a slice of queries, each of which is not expected
@@ -126,6 +131,7 @@ type Response struct {
 var stats *expvar.Map
 
 const (
+	numLeaderNotFound   = "leader_not_found"
 	numExecutions       = "executions"
 	numQueries          = "queries"
 	numRemoteExecutions = "remote_executions"
@@ -167,6 +173,7 @@ const (
 
 func init() {
 	stats = expvar.NewMap("http")
+	stats.Add(numLeaderNotFound, 0)
 	stats.Add(numExecutions, 0)
 	stats.Add(numQueries, 0)
 	stats.Add(numRemoteExecutions, 0)
@@ -375,7 +382,8 @@ func (s *Service) handleJoin(w http.ResponseWriter, r *http.Request) {
 		if err == store.ErrNotLeader {
 			leaderAPIAddr := s.LeaderAPIAddr()
 			if leaderAPIAddr == "" {
-				http.Error(w, err.Error(), http.StatusServiceUnavailable)
+				stats.Add(numLeaderNotFound, 1)
+				http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable)
 				return
 			}
 
@@ -427,7 +435,8 @@ func (s *Service) handleRemove(w http.ResponseWriter, r *http.Request) {
 		if err == store.ErrNotLeader {
 			leaderAPIAddr := s.LeaderAPIAddr()
 			if leaderAPIAddr == "" {
-				http.Error(w, err.Error(), http.StatusServiceUnavailable)
+				stats.Add(numLeaderNotFound, 1)
+				http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable)
 				return
 			}
 
@@ -470,7 +479,8 @@ func (s *Service) handleBackup(w http.ResponseWriter, r *http.Request) {
 		if err == store.ErrNotLeader {
 			leaderAPIAddr := s.LeaderAPIAddr()
 			if leaderAPIAddr == "" {
-				http.Error(w, err.Error(), http.StatusServiceUnavailable)
+				stats.Add(numLeaderNotFound, 1)
+				http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable)
 				return
 			}
 
@@ -522,7 +532,8 @@ func (s *Service) handleLoad(w http.ResponseWriter, r *http.Request) {
 		if err == store.ErrNotLeader {
 			leaderAPIAddr := s.LeaderAPIAddr()
 			if leaderAPIAddr == "" {
-				http.Error(w, err.Error(), http.StatusServiceUnavailable)
+				stats.Add(numLeaderNotFound, 1)
+				http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable)
 				return
 			}
 
@@ -767,7 +778,8 @@ func (s *Service) handleExecute(w http.ResponseWriter, r *http.Request) {
 		if redirect {
 			leaderAPIAddr := s.LeaderAPIAddr()
 			if leaderAPIAddr == "" {
-				http.Error(w, err.Error(), http.StatusServiceUnavailable)
+				stats.Add(numLeaderNotFound, 1)
+				http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable)
 				return
 			}
 			loc := s.FormRedirect(r, leaderAPIAddr)
@@ -779,6 +791,10 @@ func (s *Service) handleExecute(w http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 		}
+		if addr == "" {
+			stats.Add(numLeaderNotFound, 1)
+			http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable)
+		}
 		results, resultsErr = s.cluster.Execute(er, addr, timeout)
 		stats.Add(numRemoteExecutions, 1)
 		w.Header().Add(ServedByHTTPHeader, addr)
@@ -849,7 +865,8 @@ func (s *Service) handleQuery(w http.ResponseWriter, r *http.Request) {
 		if redirect {
 			leaderAPIAddr := s.LeaderAPIAddr()
 			if leaderAPIAddr == "" {
-				http.Error(w, err.Error(), http.StatusServiceUnavailable)
+				stats.Add(numLeaderNotFound, 1)
+				http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable)
 				return
 			}
 			loc := s.FormRedirect(r, leaderAPIAddr)
@@ -861,6 +878,10 @@ func (s *Service) handleQuery(w http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 		}
+		if addr == "" {
+			stats.Add(numLeaderNotFound, 1)
+			http.Error(w, ErrLeaderNotFound.Error(), http.StatusServiceUnavailable)
+		}
 		results, resultsErr = s.cluster.Query(qr, addr, timeout)
 		stats.Add(numRemoteQueries, 1)
 		w.Header().Add(ServedByHTTPHeader, addr)

http/service_test.go

@@ -677,7 +677,7 @@ func Test_Nodes(t *testing.T) {
 	}
 }
 
-func Test_ForwardingRedirectQueries(t *testing.T) {
+func Test_ForwardingRedirectQuery(t *testing.T) {
 	m := &MockStore{
 		leaderAddr: "foo:1234",
 	}
@@ -725,6 +725,16 @@ func Test_ForwardingRedirectQueries(t *testing.T) {
 	if resp.StatusCode != http.StatusMovedPermanently {
 		t.Fatalf("failed to get expected StatusMovedPermanently for query, got %d", resp.StatusCode)
 	}
+
+	// Check leader failure case.
+	m.leaderAddr = ""
+	resp, err = client.Get(host + "/db/query?pretty&timings&q=SELECT%20%2A%20FROM%20foo")
+	if err != nil {
+		t.Fatalf("failed to make query forwarded request")
+	}
+	if resp.StatusCode != http.StatusServiceUnavailable {
+		t.Fatalf("failed to get expected StatusServiceUnavailable for node with no leader, got %d", resp.StatusCode)
+	}
 }
 
 func Test_ForwardingRedirectExecute(t *testing.T) {
@@ -775,6 +785,16 @@ func Test_ForwardingRedirectExecute(t *testing.T) {
 	if resp.StatusCode != http.StatusMovedPermanently {
 		t.Fatalf("failed to get expected StatusMovedPermanently for execute, got %d", resp.StatusCode)
 	}
+
+	// Check leader failure case.
+	m.leaderAddr = ""
+	resp, err = client.Post(host+"/db/execute", "application/json", strings.NewReader(`["Some SQL"]`))
+	if err != nil {
+		t.Fatalf("failed to make execute request")
+	}
+	if resp.StatusCode != http.StatusServiceUnavailable {
+		t.Fatalf("failed to get expected StatusServiceUnavailable for node with no leader, got %d", resp.StatusCode)
+	}
 }
 
 func Test_TLSServce(t *testing.T) {