From 0e50ab255dd3ad77710a1780f99d804dffdd6151 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Wed, 8 Sep 2021 10:30:03 -0400 Subject: [PATCH 1/5] Better HTTP logging during end-to-end tests --- http/service.go | 15 ++++++++++----- system_test/full_system_test.py | 23 +++++++++++++++-------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/http/service.go b/http/service.go index 1b983d60..9a0edada 100644 --- a/http/service.go +++ b/http/service.go @@ -565,13 +565,15 @@ func (s *Service) handleStatus(w http.ResponseWriter, r *http.Request) { storeStatus, err := s.store.Stats() if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("store stats: %s", err.Error()), + http.StatusInternalServerError) return } clusterStatus, err := s.cluster.Stats() if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("cluster stats: %s", err.Error()), + http.StatusInternalServerError) return } @@ -616,7 +618,8 @@ func (s *Service) handleStatus(w http.ResponseWriter, r *http.Request) { for k, v := range s.statuses { stat, err := v.Stats() if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("registered stats: %s", err.Error()), + http.StatusInternalServerError) return } status[k] = stat @@ -631,12 +634,14 @@ func (s *Service) handleStatus(w http.ResponseWriter, r *http.Request) { b, err = json.Marshal(status) } if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("JSON marshal: %s", err.Error()), + http.StatusInternalServerError) return } _, err = w.Write([]byte(b)) if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("write: %s", err.Error()), + http.StatusInternalServerError) return } } diff --git a/system_test/full_system_test.py b/system_test/full_system_test.py index 20b53031..919298be 100755 --- a/system_test/full_system_test.py +++ b/system_test/full_system_test.py @@ -123,17 +123,17 @@ class Node(object): def status(self): r = requests.get(self._status_url()) - r.raise_for_status() + raise_for_status(r) return r.json() def nodes(self): r = requests.get(self._nodes_url()) - r.raise_for_status() + raise_for_status(r) return r.json() def expvar(self): r = requests.get(self._expvar_url()) - r.raise_for_status() + raise_for_status(r) return r.json() def is_leader(self): @@ -245,7 +245,7 @@ class Node(object): if pretty: reqParams['pretty'] = "yes" r = requests.post(self._query_url(), params=reqParams, data=json.dumps(body)) - r.raise_for_status() + raise_for_status(r) if text: return r.text return r.json() @@ -258,26 +258,26 @@ class Node(object): def execute_raw(self, body): r = requests.post(self._execute_url(), data=body) - r.raise_for_status() + raise_for_status(r) return r.json() def backup(self, file): with open(file, 'w') as fd: r = requests.get(self._backup_url()) - r.raise_for_status() + raise_for_status(r) fd.write(r.content) def restore(self, file): # This is the one API that doesn't expect JSON. conn = sqlite3.connect(file) r = requests.post(self._load_url(), data='\n'.join(conn.iterdump())) - r.raise_for_status() + raise_for_status(r) conn.close() return r.json() def redirect_addr(self): r = requests.post(self._execute_url(redirect=True), data=json.dumps(['nonsense']), allow_redirects=False) - r.raise_for_status() + raise_for_status(r) if r.status_code == 301: return "%s://%s" % (urlparse(r.headers['Location']).scheme, urlparse(r.headers['Location']).netloc) @@ -309,6 +309,13 @@ class Node(object): self.stdout_fd.close() self.stderr_fd.close() +def raise_for_status(r): + try: + r.raise_for_status() + except requests.exceptions.HTTPError as e: + print(e) + print(r.text) + def random_addr(): s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.bind(('localhost', 0)) From 875c09d9cae2ef019c9125dbcc96f9a261a2c6ad Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Wed, 8 Sep 2021 10:35:07 -0400 Subject: [PATCH 2/5] More improved responses on HTTP 500 --- CHANGELOG.md | 4 ++++ http/service.go | 12 ++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d09c549..6acee70f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +## 6.4.4 (unreleased) +### Implementation changes and bug fixes +- [PR #885](https://github.com/rqlite/rqlite/pull/885): Improved responses when `status/` returns HTTP 500. + ## 6.4.3 (September 9th 2021) ### Implementation changes and bug fixes - [PR #882](https://github.com/rqlite/rqlite/pull/882): Some minor improvements related to on-disk SQLite use. diff --git a/http/service.go b/http/service.go index 9a0edada..e191f8f7 100644 --- a/http/service.go +++ b/http/service.go @@ -677,7 +677,8 @@ func (s *Service) handleNodes(w http.ResponseWriter, r *http.Request) { // Get nodes in the cluster, and possibly filter out non-voters. nodes, err := s.store.Nodes() if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("store nodes: %s", err.Error()), + http.StatusInternalServerError) return } @@ -691,13 +692,15 @@ func (s *Service) handleNodes(w http.ResponseWriter, r *http.Request) { lAddr, err := s.store.LeaderAddr() if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("leader address: %s", err.Error()), + http.StatusInternalServerError) return } nodesResp, err := s.checkNodes(filteredNodes, t) if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("check nodes: %s", err.Error()), + http.StatusInternalServerError) return } @@ -798,7 +801,8 @@ func (s *Service) handleExecute(w http.ResponseWriter, r *http.Request) { addr, err := s.store.LeaderAddr() if err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) + http.Error(w, fmt.Sprintf("leader address: %s", err.Error()), + http.StatusInternalServerError) return } if addr == "" { From fd6c850ff365d90ba8d6ef38512c83e4c8c96b19 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Wed, 8 Sep 2021 10:35:40 -0400 Subject: [PATCH 3/5] Update CHANGELOG --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6acee70f..d7c5354a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ ## 6.4.4 (unreleased) ### Implementation changes and bug fixes -- [PR #885](https://github.com/rqlite/rqlite/pull/885): Improved responses when `status/` returns HTTP 500. +- [PR #885](https://github.com/rqlite/rqlite/pull/885): Improved responses on HTTP 500. ## 6.4.3 (September 9th 2021) ### Implementation changes and bug fixes From 15265136d56ea218725a8c8a159ba9302ae5de76 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Wed, 8 Sep 2021 10:48:30 -0400 Subject: [PATCH 4/5] Fix racy WaitGroup usage during tests --- tcp/pool/channel_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tcp/pool/channel_test.go b/tcp/pool/channel_test.go index 232dd986..c3a7e9cd 100644 --- a/tcp/pool/channel_test.go +++ b/tcp/pool/channel_test.go @@ -224,9 +224,9 @@ func TestPoolConcurrent2(t *testing.T) { var wg sync.WaitGroup + wg.Add(10) go func() { for i := 0; i < 10; i++ { - wg.Add(1) go func(i int) { conn, _ := p.Get() time.Sleep(time.Millisecond * time.Duration(rand.Intn(100))) @@ -236,8 +236,8 @@ func TestPoolConcurrent2(t *testing.T) { } }() + wg.Add(10) for i := 0; i < 10; i++ { - wg.Add(1) go func(i int) { conn, _ := p.Get() time.Sleep(time.Millisecond * time.Duration(rand.Intn(100))) From 4eb743b259dc9a455d06591b8b39aaa289fc7331 Mon Sep 17 00:00:00 2001 From: Philip O'Toole Date: Wed, 8 Sep 2021 10:50:20 -0400 Subject: [PATCH 5/5] Revert one unneeded change --- tcp/pool/channel_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tcp/pool/channel_test.go b/tcp/pool/channel_test.go index c3a7e9cd..13b8cb1e 100644 --- a/tcp/pool/channel_test.go +++ b/tcp/pool/channel_test.go @@ -236,8 +236,8 @@ func TestPoolConcurrent2(t *testing.T) { } }() - wg.Add(10) for i := 0; i < 10; i++ { + wg.Add(1) go func(i int) { conn, _ := p.Get() time.Sleep(time.Millisecond * time.Duration(rand.Intn(100)))