From f47772d528d9b2a02a81d5e2064459f5ba2964ff Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Wed, 14 Dec 2022 14:18:42 +0100 Subject: [PATCH 1/6] fix: fastapi exception handling and printing --- lnbits/app.py | 37 ++++++------------------------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/lnbits/app.py b/lnbits/app.py index 075828ef0..c293a4453 100644 --- a/lnbits/app.py +++ b/lnbits/app.py @@ -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,25 +170,22 @@ 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) + exc_str = 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": err} + "error.html", {"request": request, "err": exc_str} ) return JSONResponse( - status_code=HTTPStatus.NO_CONTENT, - content={"detail": err}, + status_code=HTTPStatus.BAD_REQUEST, + content={"detail": exc_str}, ) From b5eb8b7ee8c1c995faedc05b688edd6ce10d0b84 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Wed, 14 Dec 2022 14:33:13 +0100 Subject: [PATCH 2/6] add handler for RequestValidationError --- lnbits/app.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/lnbits/app.py b/lnbits/app.py index c293a4453..b688ec8d6 100644 --- a/lnbits/app.py +++ b/lnbits/app.py @@ -184,10 +184,32 @@ def register_exception_handlers(app: FastAPI): ) return JSONResponse( - status_code=HTTPStatus.BAD_REQUEST, + status_code=HTTPStatus.NO_CONTENT, content={"detail": exc_str}, ) + @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()}, + ) + def configure_logger() -> None: logger.remove() From fa7bbb62e2f7c1b656aea84a6b7bfd636fca27dd Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Wed, 14 Dec 2022 14:59:11 +0100 Subject: [PATCH 3/6] correct error codes in tests --- lnbits/app.py | 5 +++-- tests/core/views/test_generic.py | 16 ++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/lnbits/app.py b/lnbits/app.py index b688ec8d6..6efd82feb 100644 --- a/lnbits/app.py +++ b/lnbits/app.py @@ -184,7 +184,7 @@ def register_exception_handlers(app: FastAPI): ) return JSONResponse( - status_code=HTTPStatus.NO_CONTENT, + status_code=HTTPStatus.INTERNAL_SERVER_ERROR, content={"detail": exc_str}, ) @@ -192,6 +192,7 @@ def register_exception_handlers(app: FastAPI): async def validation_exception_handler( request: Request, exc: RequestValidationError ): + logger.error(str(exc)) # Only the browser sends "text/html" request # not fail proof, but everything else get's a JSON response @@ -206,7 +207,7 @@ def register_exception_handlers(app: FastAPI): ) return JSONResponse( - status_code=HTTPStatus.NO_CONTENT, + status_code=HTTPStatus.BAD_REQUEST, content={"detail": exc.errors()}, ) diff --git a/tests/core/views/test_generic.py b/tests/core/views/test_generic.py index 4300b78b9..e8fc6fcc0 100644 --- a/tests/core/views/test_generic.py +++ b/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) ) From 4a060baa752d6a3e0ffcb4ca72ac002b0efa63a2 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Wed, 14 Dec 2022 18:40:07 +0100 Subject: [PATCH 4/6] feat: error responses work --- lnbits/app.py | 42 +++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/lnbits/app.py b/lnbits/app.py index 6efd82feb..9ed246548 100644 --- a/lnbits/app.py +++ b/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 RequestValidationError, HTTPException from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.gzip import GZipMiddleware from fastapi.responses import JSONResponse @@ -173,26 +173,28 @@ def register_exception_handlers(app: FastAPI): async def exception_handler(request: Request, exc: Exception): etype, _, tb = sys.exc_info() traceback.print_exception(etype, exc, tb) - exc_str = str(exc) + 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": exc_str} + "error.html", {"request": request, "err": f"Error: {str(exc)}"} ) return JSONResponse( status_code=HTTPStatus.INTERNAL_SERVER_ERROR, - content={"detail": exc_str}, + content={"detail": str(exc)}, ) @app.exception_handler(RequestValidationError) async def validation_exception_handler( request: Request, exc: RequestValidationError ): - logger.error(str(exc)) + 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 @@ -203,12 +205,38 @@ def register_exception_handlers(app: FastAPI): ): return template_renderer().TemplateResponse( "error.html", - {"request": request, "err": f"{exc.errors()} is not a valid UUID."}, + {"request": request, "err": f"Error: {str(exc)}"}, ) return JSONResponse( status_code=HTTPStatus.BAD_REQUEST, - content={"detail": exc.errors()}, + 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}, ) From a7fe203abd6d238ec88d9c76a17928d3869c86fc Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Wed, 14 Dec 2022 18:40:23 +0100 Subject: [PATCH 5/6] fix: regtest error expectations fixed --- tests/extensions/boltz/test_api.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/extensions/boltz/test_api.py b/tests/extensions/boltz/test_api.py index 90ce6ec16..0266f9400 100644 --- a/tests/extensions/boltz/test_api.py +++ b/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 From 317c17c1b3b00921fde74e1eebe2bbee47743a97 Mon Sep 17 00:00:00 2001 From: callebtc <93376500+callebtc@users.noreply.github.com> Date: Wed, 14 Dec 2022 18:42:45 +0100 Subject: [PATCH 6/6] make format --- lnbits/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lnbits/app.py b/lnbits/app.py index 9ed246548..060e07e38 100644 --- a/lnbits/app.py +++ b/lnbits/app.py @@ -8,7 +8,7 @@ import warnings from http import HTTPStatus from fastapi import FastAPI, Request -from fastapi.exceptions import RequestValidationError, HTTPException +from fastapi.exceptions import HTTPException, RequestValidationError from fastapi.middleware.cors import CORSMiddleware from fastapi.middleware.gzip import GZipMiddleware from fastapi.responses import JSONResponse