test/pylib: scylla_cluster: return error details from test framework endpoints

If an endpoint handler throws an exception, the details of the exception
are not returned to the client. Normally this is desirable so that
information is not leaked, but in this test framework we do want to
return the details to the client so it can log a useful error message.

Do it by wrapping every handler into a catch clause that returns
the exception message.

Also modify a bit how HTTPErrors are rendered so it's easier to discern
the actual body of the error from other details (such as the params used
to make the request etc.)

Before:
```
E test.pylib.rest_client.HTTPError: HTTP error 500: 500 Internal Server Error
E
E Server got itself in trouble, params None, json None, uri http+unix://api/cluster/before-test/test_stuff
```

After:
```
E test.pylib.rest_client.HTTPError: HTTP error 500, uri: http+unix://api/cluster/before-test/test_stuff, params: None, json: None, body:
E Failed to start server at host 127.155.129.1.
E Check the log files:
E /home/kbraun/dev/scylladb/testlog/test.py.dev.log
E /home/kbraun/dev/scylladb/testlog/dev/scylla-1.log
```

Closes #12563
This commit is contained in:
Kamil Braun
2023-01-18 16:50:26 +01:00
committed by Botond Dénes
parent 3ed3966f13
commit 2f84e820fd
2 changed files with 46 additions and 24 deletions

View File

@@ -21,14 +21,17 @@ logger = logging.getLogger(__name__)
class HTTPError(Exception):
def __init__(self, uri, code, message):
def __init__(self, uri, code, params, json, message):
super().__init__(message)
self.uri = uri
self.code = code
self.params = params
self.json = json
self.message = message
def __str__(self):
return f"HTTP error {self.code}: {self.message}, uri {self.uri}"
return f"HTTP error {self.code}, uri: {self.uri}, " \
f"params: {self.params}, json: {self.json}, body:\n{self.message}"
# TODO: support ssl and verify_ssl
@@ -63,7 +66,7 @@ class RESTClient(metaclass=ABCMeta):
params = params, json = json, timeout = client_timeout) as resp:
if resp.status != 200:
text = await resp.text()
raise HTTPError(uri, resp.status, f"{text}, params {params}, json {json}")
raise HTTPError(uri, resp.status, params, json, text)
if response_type is not None:
# Return response.text() or response.json()
return await getattr(resp, response_type)()

View File

@@ -17,6 +17,7 @@ import pathlib
import shutil
import tempfile
import time
import traceback
from typing import Optional, Dict, List, Set, Tuple, Callable, AsyncIterator, NamedTuple, Union
import uuid
from io import BufferedWriter
@@ -884,27 +885,45 @@ class ScyllaClusterManager:
def _setup_routes(self, app: aiohttp.web.Application) -> None:
app.router.add_get('/up', self._manager_up)
app.router.add_get('/cluster/up', self._cluster_up)
app.router.add_get('/cluster/is-dirty', self._is_dirty)
app.router.add_get('/cluster/replicas', self._cluster_replicas)
app.router.add_get('/cluster/running-servers', self._cluster_running_servers)
app.router.add_get('/cluster/host-ip/{server_id}', self._cluster_server_ip_addr)
app.router.add_get('/cluster/host-id/{server_id}', self._cluster_host_id)
app.router.add_get('/cluster/before-test/{test_case_name}', self._before_test_req)
app.router.add_get('/cluster/after-test', self._after_test)
app.router.add_get('/cluster/mark-dirty', self._mark_dirty)
app.router.add_get('/cluster/server/{server_id}/stop', self._cluster_server_stop)
app.router.add_get('/cluster/server/{server_id}/stop_gracefully',
self._cluster_server_stop_gracefully)
app.router.add_get('/cluster/server/{server_id}/start', self._cluster_server_start)
app.router.add_get('/cluster/server/{server_id}/restart', self._cluster_server_restart)
app.router.add_put('/cluster/addserver', self._cluster_server_add)
app.router.add_put('/cluster/remove-node/{initiator}', self._cluster_remove_node)
app.router.add_get('/cluster/decommission-node/{server_id}',
self._cluster_decommission_node)
app.router.add_get('/cluster/server/{server_id}/get_config', self._server_get_config)
app.router.add_put('/cluster/server/{server_id}/update_config', self._server_update_config)
def make_catching_handler(handler: Callable) -> Callable:
async def catching_handler(request) -> aiohttp.web.Response:
"""Catch all exceptions and return them to the client.
Without this, the client would get an 'Internal server error' message
without any details. Thanks to this the test log shows the actual error.
"""
try:
return await handler(request)
except Exception as e:
tb = traceback.format_exc()
self.logger.error(f'Exception when executing {handler.__name__}: {e}\n{tb}')
return aiohttp.web.Response(status=500, text=str(e))
return catching_handler
def add_get(route: str, handler: Callable):
app.router.add_get(route, make_catching_handler(handler))
def add_put(route: str, handler: Callable):
app.router.add_put(route, make_catching_handler(handler))
add_get('/up', self._manager_up)
add_get('/cluster/up', self._cluster_up)
add_get('/cluster/is-dirty', self._is_dirty)
add_get('/cluster/replicas', self._cluster_replicas)
add_get('/cluster/running-servers', self._cluster_running_servers)
add_get('/cluster/host-ip/{server_id}', self._cluster_server_ip_addr)
add_get('/cluster/host-id/{server_id}', self._cluster_host_id)
add_get('/cluster/before-test/{test_case_name}', self._before_test_req)
add_get('/cluster/after-test', self._after_test)
add_get('/cluster/mark-dirty', self._mark_dirty)
add_get('/cluster/server/{server_id}/stop', self._cluster_server_stop)
add_get('/cluster/server/{server_id}/stop_gracefully', self._cluster_server_stop_gracefully)
add_get('/cluster/server/{server_id}/start', self._cluster_server_start)
add_get('/cluster/server/{server_id}/restart', self._cluster_server_restart)
add_put('/cluster/addserver', self._cluster_server_add)
add_put('/cluster/remove-node/{initiator}', self._cluster_remove_node)
add_get('/cluster/decommission-node/{server_id}', self._cluster_decommission_node)
add_get('/cluster/server/{server_id}/get_config', self._server_get_config)
add_put('/cluster/server/{server_id}/update_config', self._server_update_config)
async def _manager_up(self, _request) -> aiohttp.web.Response:
return aiohttp.web.Response(text=f"{self.is_running}")