add api exception handler (#74)

* try to add exception handler

* improve server error test

* fix lint

* add build_uri util

* fix header file path

---------

Co-authored-by: Dmitry Afanasyev <afanasiev@litres.ru>

bot_microservice/api/base_schemas.py

@@ -0,0 +1,27 @@
+from typing import Any
+
+from fastapi import status as status_code
+from pydantic import BaseModel, Field
+
+
+class BaseError(BaseModel):
+    """Error response as defined in
+    https://datatracker.ietf.org/doc/html/rfc7807.
+
+    One difference is that the member "type" is not a URI
+    """
+
+    type: str | None = Field(title="The name of the class of the error")
+    title: str | None = Field(
+        title="A short, human-readable summary of the problem that does not change from occurence to occurence",
+    )
+    detail: str | None = Field(
+        default=None, title="А human-readable explanation specific to this occurrence of the problem"
+    )
+    instance: Any | None = Field(default=None, title="Identifier for this specific occurrence of the problem")
+
+
+class BaseResponse(BaseModel):
+    status: int = Field(..., title="Status code of request.", example=status_code.HTTP_200_OK)  # type: ignore[call-arg]
+    error: dict[Any, Any] | BaseError | None = Field(default=None, title="Errors")
+    payload: Any | None = Field(default_factory=dict, title="Payload data.")

bot_microservice/api/exceptions.py

@@ -1,2 +1,34 @@
+from fastapi.responses import ORJSONResponse
+from starlette.requests import Request
+
+from api.base_schemas import BaseError, BaseResponse
+
+
 class BaseAPIException(Exception):
     pass
+
+
+class InternalServerError(BaseError):
+    pass
+
+
+class InternalServerErrorResponse(BaseResponse):
+    error: InternalServerError
+
+    class Config:
+        json_schema_extra = {
+            "example": {
+                "status": 500,
+                "error": {
+                    "type": "InternalServerError",
+                    "title": "Server encountered an unexpected error that prevented it from fulfilling the request",
+                    "detail": "error when adding send message",
+                },
+            },
+        }
+
+
+async def internal_server_error_handler(_request: Request, _exception: Exception) -> ORJSONResponse:
+    error = InternalServerError(title="Something went wrong!", type="InternalServerError")
+    response = InternalServerErrorResponse(status=500, error=error).model_dump(exclude_unset=True)
+    return ORJSONResponse(status_code=500, content=response)

bot_microservice/core/utils.py

@@ -1,3 +1,4 @@
+import os
 from datetime import datetime, timedelta
 from functools import cache, wraps
 from inspect import cleandoc
@@ -39,3 +40,9 @@ def clean_doc(cls: Any) -> str | None:
     if cls.__doc__ is None:
         return None
     return cleandoc(cls.__doc__)
+
+
+def build_uri(uri_parts: list[str]) -> str:
+    parts = [part.strip("/") for part in uri_parts]
+    uri = str(os.path.join(*parts)).strip("/")
+    return f"/{uri}"

bot_microservice/infra/admin.py

@@ -1,9 +1,9 @@
-import os
 from typing import TYPE_CHECKING
 
 from sqladmin import Admin, ModelView
 
 from core.bot.models.chatgpt import ChatGptModels
+from core.utils import build_uri
 from settings.config import settings
 
 if TYPE_CHECKING:
@@ -21,12 +21,11 @@ class ChatGptAdmin(ModelView, model=ChatGptModels):
 
 
 def create_admin(application: "Application") -> Admin:
-    base_url = os.path.join(settings.URL_PREFIX, "admin")
     admin = Admin(
         title="Chat GPT admin",
         app=application.fastapi_app,
         engine=application.db.async_engine,
-        base_url=base_url if base_url.startswith("/") else "/" + base_url,
+        base_url=build_uri([settings.URL_PREFIX, "admin"]),
         authentication_backend=None,
     )
     admin.add_view(ChatGptAdmin)

bot_microservice/main.py

@@ -5,9 +5,11 @@ import sentry_sdk
 from fastapi import FastAPI
 from fastapi.responses import UJSONResponse
 
+from api.exceptions import internal_server_error_handler
 from core.bot.app import BotApplication, BotQueue
 from core.bot.handlers import bot_event_handlers
 from core.lifetime import shutdown, startup
+from core.utils import build_uri
 from infra.admin import create_admin
 from infra.database.db_adapter import Database
 from infra.logging_conf import configure_logging
@@ -21,11 +23,14 @@ class Application:
             title="Chat gpt bot",
             description="Bot for proxy to chat gpt in telegram",
             version="0.0.3",
-            docs_url="/" + "/".join([settings.api_prefix.strip("/"), "docs"]),
-            redoc_url="/" + "/".join([settings.api_prefix.strip("/"), "redocs"]),
-            openapi_url="/" + "/".join([settings.api_prefix.strip("/"), "openapi.json"]),
+            docs_url=build_uri([settings.api_prefix, "docs"]),
+            redoc_url=build_uri([settings.api_prefix, "redocs"]),
+            openapi_url=build_uri([settings.api_prefix, "openapi.json"]),
             debug=settings.DEBUG,
             default_response_class=UJSONResponse,
+            exception_handlers={
+                Exception: internal_server_error_handler,
+            },
         )
         self.bot_app = bot_app
         self.db = Database(settings)

bot_microservice/settings/config.py

@@ -9,6 +9,7 @@ from pydantic_settings import BaseSettings
 from yarl import URL
 
 from constants import API_PREFIX, CHATGPT_BASE_URI, LogLevelEnum
+from core.utils import build_uri
 
 BASE_DIR = Path(__file__).parent.parent
 SHARED_DIR = BASE_DIR.resolve().joinpath("shared")
@@ -109,7 +110,7 @@ class AppSettings(SentrySettings, LoggingSettings, BaseSettings):
     @cached_property
     def api_prefix(self) -> str:
         if self.URL_PREFIX:
-            return "/" + "/".join([self.URL_PREFIX.strip("/"), API_PREFIX.strip("/")])
+            return build_uri([self.URL_PREFIX, API_PREFIX])
         return API_PREFIX
 
     @cached_property
@@ -126,7 +127,7 @@ class AppSettings(SentrySettings, LoggingSettings, BaseSettings):
 
     @cached_property
     def bot_webhook_url(self) -> str:
-        return "/".join([self.api_prefix, self.token_part])
+        return build_uri([self.api_prefix, self.token_part])
 
     @cached_property
     def db_file(self) -> Path:

bot_microservice/tests/conftest.py

@@ -24,6 +24,20 @@ def test_settings() -> AppSettings:
     return get_settings()
 
 
+# Redefine the event_loop fixture to have a session scope. Otherwise `bot` fixture can't be
+# session. See https://github.com/pytest-dev/pytest-asyncio/issues/68 for more details.
+@pytest.fixture(scope="session", autouse=True)
+def event_loop() -> AbstractEventLoop:
+    """
+    Пересоздаем луп для изоляции тестов. В основном нужно для запуска юнит тестов
+    в связке с интеграционными, т.к. без этого pytest зависает.
+    Для интеграционных тестов фикстура определяется дополнительная фикстура на всю сессию.
+    """
+    loop = asyncio.get_event_loop_policy().new_event_loop()
+    asyncio.set_event_loop(loop)
+    return loop
+
+
 @pytest.fixture(scope="session")
 def engine(test_settings: AppSettings) -> Generator[Engine, None, None]:
     """
@@ -99,21 +113,6 @@ class PytestApplication(Application):  # type: ignore
     pass
 
 
-def make_bot(bot_info: dict[str, Any] | None = None, **kwargs: Any) -> PytestExtBot:
-    """
-    Tests are executed on tg.ext.ExtBot, as that class only extends the functionality of tg.bot
-    """
-    token = kwargs.pop("token", (bot_info or {}).get("token"))
-    kwargs.pop("token", None)
-    return PytestExtBot(
-        token=token,
-        private_key=None,
-        request=NonchalantHttpxRequest(connection_pool_size=8),
-        get_updates_request=NonchalantHttpxRequest(connection_pool_size=1),
-        **kwargs,
-    )
-
-
 async def _mocked_get_me(bot: Bot) -> User:
     if bot._bot_user is None:
         bot._bot_user = _get_bot_user(bot.token)
@@ -145,20 +144,6 @@ def _get_bot_user(token: str) -> User:
     )
 
 
-# Redefine the event_loop fixture to have a session scope. Otherwise `bot` fixture can't be
-# session. See https://github.com/pytest-dev/pytest-asyncio/issues/68 for more details.
-@pytest.fixture(scope="session", autouse=True)
-def event_loop() -> AbstractEventLoop:
-    """
-    Пересоздаем луп для изоляции тестов. В основном нужно для запуска юнит тестов
-    в связке с интеграционными, т.к. без этого pytest зависает.
-    Для интеграционных тестов фикстура определяется дополнительная фикстура на всю сессию.
-    """
-    loop = asyncio.get_event_loop_policy().new_event_loop()
-    asyncio.set_event_loop(loop)
-    return loop
-
-
 @pytest.fixture(scope="session")
 def bot_info() -> dict[str, Any]:
     return BotInfoFactory()
@@ -183,14 +168,16 @@ async def bot(bot_info: dict[str, Any], bot_application: Any) -> AsyncGenerator[
         yield _bot
 
 
+@pytest.fixture(scope="session")
+async def bot_app(test_settings: AppSettings) -> BotApplication:
+    return BotApplication(settings=test_settings, handlers=bot_event_handlers.handlers)
+
+
 @pytest.fixture(scope="session")
 async def main_application(
-    bot_application: PytestApplication, test_settings: AppSettings
+    test_settings: AppSettings,
+    bot_app: BotApplication,
 ) -> AsyncGenerator[AppApplication, None]:
-    bot_app = BotApplication(
-        settings=test_settings,
-        handlers=bot_event_handlers.handlers,
-    )
     bot_app.application._initialized = True
     bot_app.application.bot = make_bot(BotInfoFactory())
     bot_app.application.bot._bot_user = BotUserFactory()
@@ -213,3 +200,18 @@ async def rest_client(main_application: AppApplication) -> AsyncGenerator[AsyncC
         headers={"Content-Type": "application/json"},
     ) as client:
         yield client
+
+
+def make_bot(bot_info: dict[str, Any] | None = None, **kwargs: Any) -> PytestExtBot:
+    """
+    Tests are executed on tg.ext.ExtBot, as that class only extends the functionality of tg.bot
+    """
+    token = kwargs.pop("token", (bot_info or {}).get("token"))
+    kwargs.pop("token", None)
+    return PytestExtBot(
+        token=token,
+        private_key=None,
+        request=NonchalantHttpxRequest(connection_pool_size=8),
+        get_updates_request=NonchalantHttpxRequest(connection_pool_size=1),
+        **kwargs,
+    )

bot_microservice/tests/integration/system/test_system.py

@@ -1,11 +1,15 @@
 import httpx
 import pytest
 from faker import Faker
-from httpx import AsyncClient, Response
+from fastapi.responses import ORJSONResponse
+from httpx import ASGITransport, AsyncClient, Response
 from sqlalchemy.ext.asyncio import AsyncSession
 from sqlalchemy.orm import Session
+from starlette import status
 
 from api.exceptions import BaseAPIException
+from core.bot.app import BotApplication
+from main import Application as AppApplication
 from settings.config import AppSettings
 from tests.integration.factories.bot import ChatGptModelFactory
 from tests.integration.utils import mocked_ask_question_api
@@ -63,3 +67,32 @@ async def test_bot_healthcheck_not_ok(
     ):
         response = await rest_client.get("/api/bot-healthcheck")
         assert response.status_code == httpx.codes.INTERNAL_SERVER_ERROR
+
+
+async def test_server_error_handler_returns_500_without_traceback_when_debug_disabled(
+    test_settings: AppSettings,
+    bot_app: BotApplication,
+) -> None:
+    settings = test_settings.model_copy(update={"DEBUG": False})
+    fastapi_app = AppApplication(settings=settings, bot_app=bot_app).fastapi_app
+
+    route = "/server-error"
+
+    @fastapi_app.get(route, response_model=None, status_code=status.HTTP_500_INTERNAL_SERVER_ERROR)
+    async def controller() -> ORJSONResponse:
+        result = 1 / 0
+        return ORJSONResponse(content=result, status_code=status.HTTP_200_OK)
+
+    async with AsyncClient(
+        transport=ASGITransport(app=fastapi_app, raise_app_exceptions=False),  # type: ignore[arg-type]
+        base_url="http://test",
+        headers={"Content-Type": "application/json"},
+    ) as client:
+        response = await client.get(route)
+    assert response.status_code == 500
+    data = response.json()
+    assert data == {"error": {"title": "Something went wrong!", "type": "InternalServerError"}, "status": 500}
+    replaced_oauth_route = next(
+        filter(lambda r: r.path == route, fastapi_app.routes)  # type: ignore[arg-type, attr-defined]
+    )
+    fastapi_app.routes.remove(replaced_oauth_route)

bot_microservice/tests/unit/test_system_utils.py

@@ -1,7 +1,9 @@
 import time
 from typing import Callable
 
-from core.utils import timed_lru_cache
+import pytest
+
+from core.utils import build_uri, timed_lru_cache
 
 
 class TestTimedLruCache:
@@ -58,3 +60,21 @@ class TestTimedLruCache:
     ) -> None:
         for _ in range(call_times):
             assert func(first, second) == result
+
+
+@pytest.mark.parametrize(
+    "uri_parts, expected_result",
+    [
+        (["", "admin"], "/admin"),
+        (["/gpt", "admin"], "/gpt/admin"),
+        (["/gpt", "/chat"], "/gpt/chat"),
+        (["gpt", ""], "/gpt"),
+        (["gpt"], "/gpt"),
+        (["gpt", "chat"], "/gpt/chat"),
+        (["", ""], "/"),
+        (["gpt", "/chat", "/admin"], "/gpt/chat/admin"),
+        (["gpt/", "/chat/", "/admin"], "/gpt/chat/admin"),
+    ],
+)
+def test_build_uri_with_slash_prefix(uri_parts: list[str], expected_result: str) -> None:
+    assert build_uri(uri_parts) == expected_result

chatgpt_microservice/src/free_gpt.cpp

@@ -7,7 +7,7 @@
 #include <tuple>
 #include <vector>
 
-#include <concurrentqueue/concurrentqueue.h>
+#include <concurrentqueue.h>
 #include <curl/curl.h>
 #include <openssl/md5.h>
 #include <spdlog/spdlog.h>