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:
committed by
Botond Dénes
parent
3ed3966f13
commit
2f84e820fd
@@ -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)()
|
||||
|
||||
@@ -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}")
|
||||
|
||||
Reference in New Issue
Block a user