always return json in responses

src/ahriman/web/middlewares/exception_handler.py

@@ -18,8 +18,8 @@
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
 from aiohttp.web import middleware, Request
-from aiohttp.web_exceptions import HTTPException
+from aiohttp.web_exceptions import HTTPClientError, HTTPException, HTTPServerError
-from aiohttp.web_response import StreamResponse
+from aiohttp.web_response import json_response, StreamResponse
 from logging import Logger
 
 from ahriman.web.middlewares import HandlerType, MiddlewareType
@@ -35,10 +35,15 @@ def exception_handler(logger: Logger) -> MiddlewareType:
     async def handle(request: Request, handler: HandlerType) -> StreamResponse:
         try:
             return await handler(request)
+        except HTTPClientError as e:
+            return json_response(data={"error": e.reason}, status=e.status_code)
+        except HTTPServerError as e:
+            logger.exception("server exception during performing request to %s", request.path)
+            return json_response(data={"error": e.reason}, status=e.status_code)
         except HTTPException:
-            raise  # we do not raise 5xx exceptions actually so it should be fine
+            raise  # just raise 2xx and 3xx codes
-        except Exception:
+        except Exception as e:
-            logger.exception("exception during performing request to %s", request.path)
+            logger.exception("unknown exception during performing request to %s", request.path)
-            raise
+            return json_response(data={"error": str(e)}, status=500)
 
     return handle

src/ahriman/web/views/service/add.py

@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-from aiohttp.web import HTTPFound, Response, json_response
+from aiohttp.web import HTTPBadRequest, HTTPFound, Response
 
 from ahriman.models.user_access import UserAccess
 from ahriman.web.views.base import BaseView
@@ -42,12 +42,11 @@ class AddView(BaseView):
 
         :return: redirect to main page on success
         """
-        data = await self.extract_data(["packages"])
-
         try:
+            data = await self.extract_data(["packages"])
             packages = data["packages"]
         except Exception as e:
-            return json_response(data=str(e), status=400)
+            raise HTTPBadRequest(reason=str(e))
 
         self.spawner.packages_add(packages, now=True)
 

src/ahriman/web/views/service/remove.py

@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-from aiohttp.web import HTTPFound, Response, json_response
+from aiohttp.web import HTTPBadRequest, HTTPFound, Response
 
 from ahriman.models.user_access import UserAccess
 from ahriman.web.views.base import BaseView
@@ -42,12 +42,11 @@ class RemoveView(BaseView):
 
         :return: redirect to main page on success
         """
-        data = await self.extract_data(["packages"])
-
         try:
+            data = await self.extract_data(["packages"])
             packages = data["packages"]
         except Exception as e:
-            return json_response(data=str(e), status=400)
+            raise HTTPBadRequest(reason=str(e))
 
         self.spawner.packages_remove(packages)
 

src/ahriman/web/views/service/request.py

@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-from aiohttp.web import HTTPFound, Response, json_response
+from aiohttp.web import HTTPBadRequest, HTTPFound, Response
 
 from ahriman.models.user_access import UserAccess
 from ahriman.web.views.base import BaseView
@@ -42,12 +42,11 @@ class RequestView(BaseView):
 
         :return: redirect to main page on success
         """
-        data = await self.extract_data(["packages"])
-
         try:
+            data = await self.extract_data(["packages"])
             packages = data["packages"]
         except Exception as e:
-            return json_response(data=str(e), status=400)
+            raise HTTPBadRequest(reason=str(e))
 
         self.spawner.packages_add(packages, now=False)
 

src/ahriman/web/views/service/search.py

@@ -19,7 +19,7 @@
 #
 import aur  # type: ignore
 
-from aiohttp.web import Response, json_response
+from aiohttp.web import HTTPBadRequest, Response, json_response
 from typing import Callable, Iterator
 
 from ahriman.models.user_access import UserAccess
@@ -47,7 +47,7 @@ class SearchView(BaseView):
         search_string = " ".join(search)
 
         if not search_string:
-            return json_response(data="Search string must not be empty", status=400)
+            raise HTTPBadRequest(reason="Search string must not be empty")
         packages = aur.search(search_string)
 
         comparator: Callable[[aur.Package], str] = lambda item: str(item.package_base)

src/ahriman/web/views/status/ahriman.py

@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-from aiohttp.web import HTTPNoContent, Response, json_response
+from aiohttp.web import HTTPBadRequest, HTTPNoContent, Response, json_response
 
 from ahriman.models.build_status import BuildStatusEnum
 from ahriman.models.user_access import UserAccess
@@ -53,12 +53,11 @@ class AhrimanView(BaseView):
 
         :return: 204 on success
         """
-        data = await self.extract_data()
-
         try:
+            data = await self.extract_data()
             status = BuildStatusEnum(data["status"])
         except Exception as e:
-            return json_response(data=str(e), status=400)
+            raise HTTPBadRequest(reason=str(e))
 
         self.service.update_self(status)
 

src/ahriman/web/views/status/package.py

@@ -17,7 +17,7 @@
 # You should have received a copy of the GNU General Public License
 # along with this program. If not, see <http://www.gnu.org/licenses/>.
 #
-from aiohttp.web import HTTPNoContent, HTTPNotFound, Response, json_response
+from aiohttp.web import HTTPBadRequest, HTTPNoContent, HTTPNotFound, Response, json_response
 
 from ahriman.core.exceptions import UnknownPackage
 from ahriman.models.build_status import BuildStatusEnum
@@ -88,11 +88,11 @@ class PackageView(BaseView):
             package = Package.from_json(data["package"]) if "package" in data else None
             status = BuildStatusEnum(data["status"])
         except Exception as e:
-            return json_response(data=str(e), status=400)
+            raise HTTPBadRequest(reason=str(e))
 
         try:
             self.service.update(base, status, package)
         except UnknownPackage:
-            return json_response(data=f"Package {base} is unknown, but no package body set", status=400)
+            raise HTTPBadRequest(reason=f"Package {base} is unknown, but no package body set")
 
         raise HTTPNoContent()

tests/ahriman/web/middlewares/test_exception_handler.py

@@ -1,13 +1,24 @@
+import json
 import logging
 import pytest
 
-from aiohttp.web_exceptions import HTTPBadRequest, HTTPNoContent
+from aiohttp.web_exceptions import HTTPBadRequest, HTTPInternalServerError, HTTPNoContent
 from pytest_mock import MockerFixture
+from typing import Any
 from unittest.mock import AsyncMock
 
 from ahriman.web.middlewares.exception_handler import exception_handler
 
 
+def _extract_body(response: Any) -> Any:
+    """
+    extract json body from given object
+    :param response: response (any actually) object
+    :return: body key from the object converted to json
+    """
+    return json.loads(getattr(response, "body"))
+
+
 async def test_exception_handler(mocker: MockerFixture) -> None:
     """
     must pass success response
@@ -23,7 +34,7 @@ async def test_exception_handler(mocker: MockerFixture) -> None:
 
 async def test_exception_handler_success(mocker: MockerFixture) -> None:
     """
-    must pass client exception
+    must pass 2xx and 3xx codes
     """
     request = pytest.helpers.request("", "", "")
     request_handler = AsyncMock(side_effect=HTTPNoContent())
@@ -37,27 +48,41 @@ async def test_exception_handler_success(mocker: MockerFixture) -> None:
 
 async def test_exception_handler_client_error(mocker: MockerFixture) -> None:
     """
-    must pass client exception
+    must handle client exception
     """
     request = pytest.helpers.request("", "", "")
     request_handler = AsyncMock(side_effect=HTTPBadRequest())
     logging_mock = mocker.patch("logging.Logger.exception")
 
     handler = exception_handler(logging.getLogger())
-    with pytest.raises(HTTPBadRequest):
+    response = await handler(request, request_handler)
-        await handler(request, request_handler)
+    assert _extract_body(response) == {"error": HTTPBadRequest().reason}
     logging_mock.assert_not_called()
 
 
 async def test_exception_handler_server_error(mocker: MockerFixture) -> None:
     """
-    must log server exception and re-raise it
+    must handle server exception
     """
     request = pytest.helpers.request("", "", "")
-    request_handler = AsyncMock(side_effect=Exception())
+    request_handler = AsyncMock(side_effect=HTTPInternalServerError())
     logging_mock = mocker.patch("logging.Logger.exception")
 
     handler = exception_handler(logging.getLogger())
-    with pytest.raises(Exception):
+    response = await handler(request, request_handler)
-        await handler(request, request_handler)
+    assert _extract_body(response) == {"error": HTTPInternalServerError().reason}
+    logging_mock.assert_called_once()
+
+
+async def test_exception_handler_unknown_error(mocker: MockerFixture) -> None:
+    """
+    must log server exception and re-raise it
+    """
+    request = pytest.helpers.request("", "", "")
+    request_handler = AsyncMock(side_effect=Exception("An error"))
+    logging_mock = mocker.patch("logging.Logger.exception")
+
+    handler = exception_handler(logging.getLogger())
+    response = await handler(request, request_handler)
+    assert _extract_body(response) == {"error": "An error"}
     logging_mock.assert_called_once()