Merge pull request #1197 from lnbits/fix/fastapi_exception_handling

Fix: FastApi exception handling and logging

lnbits/app.py

@@ -8,7 +8,7 @@ import warnings
 from http import HTTPStatus
 
 from fastapi import FastAPI, Request
-from fastapi.exceptions import RequestValidationError
+from fastapi.exceptions import HTTPException, RequestValidationError
 from fastapi.middleware.cors import CORSMiddleware
 from fastapi.middleware.gzip import GZipMiddleware
 from fastapi.responses import JSONResponse
@@ -68,28 +68,6 @@ def create_app(config_object="lnbits.settings") -> FastAPI:
     g().config = lnbits.settings
     g().base_url = f"http://{lnbits.settings.HOST}:{lnbits.settings.PORT}"
 
-    @app.exception_handler(RequestValidationError)
-    async def validation_exception_handler(
-        request: Request, exc: RequestValidationError
-    ):
-        # Only the browser sends "text/html" request
-        # not fail proof, but everything else get's a JSON response
-
-        if (
-            request.headers
-            and "accept" in request.headers
-            and "text/html" in request.headers["accept"]
-        ):
-            return template_renderer().TemplateResponse(
-                "error.html",
-                {"request": request, "err": f"{exc.errors()} is not a valid UUID."},
-            )
-
-        return JSONResponse(
-            status_code=HTTPStatus.NO_CONTENT,
-            content={"detail": exc.errors()},
-        )
-
     app.add_middleware(GZipMiddleware, minimum_size=1000)
 
     check_funding_source(app)
@@ -192,12 +170,33 @@ def register_async_tasks(app):
 
 def register_exception_handlers(app: FastAPI):
     @app.exception_handler(Exception)
-    async def basic_error(request: Request, err):
-        logger.error("handled error", traceback.format_exc())
-        logger.error("ERROR:", err)
+    async def exception_handler(request: Request, exc: Exception):
         etype, _, tb = sys.exc_info()
-        traceback.print_exception(etype, err, tb)
-        exc = traceback.format_exc()
+        traceback.print_exception(etype, exc, tb)
+        logger.error(f"Exception: {str(exc)}")
+        if (
+            request.headers
+            and "accept" in request.headers
+            and "text/html" in request.headers["accept"]
+        ):
+            return template_renderer().TemplateResponse(
+                "error.html", {"request": request, "err": f"Error: {str(exc)}"}
+            )
+
+        return JSONResponse(
+            status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
+            content={"detail": str(exc)},
+        )
+
+    @app.exception_handler(RequestValidationError)
+    async def validation_exception_handler(
+        request: Request, exc: RequestValidationError
+    ):
+        etype, _, tb = sys.exc_info()
+        traceback.print_exception(etype, exc, tb)
+        logger.error(f"RequestValidationError: {str(exc)}")
+        # Only the browser sends "text/html" request
+        # not fail proof, but everything else get's a JSON response
 
         if (
             request.headers
@@ -205,12 +204,39 @@ def register_exception_handlers(app: FastAPI):
             and "text/html" in request.headers["accept"]
         ):
             return template_renderer().TemplateResponse(
-                "error.html", {"request": request, "err": err}
+                "error.html",
+                {"request": request, "err": f"Error: {str(exc)}"},
             )
 
         return JSONResponse(
-            status_code=HTTPStatus.NO_CONTENT,
-            content={"detail": err},
+            status_code=HTTPStatus.BAD_REQUEST,
+            content={"detail": str(exc)},
+        )
+
+    @app.exception_handler(HTTPException)
+    async def http_exception_handler(request: Request, exc: HTTPException):
+        etype, _, tb = sys.exc_info()
+        traceback.print_exception(etype, exc, tb)
+        logger.error(f"HTTPException {exc.status_code}: {exc.detail}")
+        # Only the browser sends "text/html" request
+        # not fail proof, but everything else get's a JSON response
+
+        if (
+            request.headers
+            and "accept" in request.headers
+            and "text/html" in request.headers["accept"]
+        ):
+            return template_renderer().TemplateResponse(
+                "error.html",
+                {
+                    "request": request,
+                    "err": f"HTTP Error {exc.status_code}: {exc.detail}",
+                },
+            )
+
+        return JSONResponse(
+            status_code=exc.status_code,
+            content={"detail": exc.detail},
         )
 
 

tests/core/views/test_generic.py

@@ -46,11 +46,11 @@ async def test_get_wallet_no_redirect(client):
         i += 1
 
 
-# check GET /wallet: wrong user, expect 204
+# check GET /wallet: wrong user, expect 400
 @pytest.mark.asyncio
 async def test_get_wallet_with_nonexistent_user(client):
     response = await client.get("wallet", params={"usr": "1"})
-    assert response.status_code == 204, (
+    assert response.status_code == 400, (
         str(response.url) + " " + str(response.status_code)
     )
 
@@ -91,11 +91,11 @@ async def test_get_wallet_with_user_and_wallet(client, to_user, to_wallet):
     )
 
 
-# check GET /wallet: wrong wallet and user, expect 204
+# check GET /wallet: wrong wallet and user, expect 400
 @pytest.mark.asyncio
 async def test_get_wallet_with_user_and_wrong_wallet(client, to_user):
     response = await client.get("wallet", params={"usr": to_user.id, "wal": "1"})
-    assert response.status_code == 204, (
+    assert response.status_code == 400, (
         str(response.url) + " " + str(response.status_code)
     )
 
@@ -109,20 +109,20 @@ async def test_get_extensions(client, to_user):
     )
 
 
-# check GET /extensions: extensions list wrong user, expect 204
+# check GET /extensions: extensions list wrong user, expect 400
 @pytest.mark.asyncio
 async def test_get_extensions_wrong_user(client, to_user):
     response = await client.get("extensions", params={"usr": "1"})
-    assert response.status_code == 204, (
+    assert response.status_code == 400, (
         str(response.url) + " " + str(response.status_code)
     )
 
 
-# check GET /extensions: no user given, expect code 204 no content
+# check GET /extensions: no user given, expect code 400 bad request
 @pytest.mark.asyncio
 async def test_get_extensions_no_user(client):
     response = await client.get("extensions")
-    assert response.status_code == 204, (  # no content
+    assert response.status_code == 400, (  # bad request
         str(response.url) + " " + str(response.status_code)
     )
 

tests/extensions/boltz/test_api.py

@@ -61,21 +61,21 @@ async def test_endpoints_inkey(client, inkey_headers_to):
 
 @pytest.mark.asyncio
 @pytest.mark.skipif(is_fake, reason="this test is only passes with regtest")
-async def test_endpoints_adminkey_nocontent(client, adminkey_headers_to):
+async def test_endpoints_adminkey_badrequest(client, adminkey_headers_to):
     response = await client.post("/boltz/api/v1/swap", headers=adminkey_headers_to)
-    assert response.status_code == 204
+    assert response.status_code == 400
     response = await client.post(
         "/boltz/api/v1/swap/reverse", headers=adminkey_headers_to
     )
-    assert response.status_code == 204
+    assert response.status_code == 400
     response = await client.post(
         "/boltz/api/v1/swap/refund", headers=adminkey_headers_to
     )
-    assert response.status_code == 204
+    assert response.status_code == 400
     response = await client.post(
         "/boltz/api/v1/swap/status", headers=adminkey_headers_to
     )
-    assert response.status_code == 204
+    assert response.status_code == 400
 
 
 @pytest.mark.asyncio