fix: rollback logs warning on clean sessions [DIA-40867] (#52)

fastapi_sqla/__init__.py

@@ -166,12 +166,13 @@ def get_users(session: fastapi_sqla.Session = Depends()):
 
         response = await call_next(request)
 
+        is_dirty = bool(session.dirty or session.deleted or session.new)
+
         loop = asyncio.get_running_loop()
 
         # try to commit after response, so that we can return a proper 500 response
         # and not raise a true internal server error
         if response.status_code < 400:
-
             try:
                 await loop.run_in_executor(None, session.commit)
             except Exception:
@@ -183,7 +184,13 @@ def get_users(session: fastapi_sqla.Session = Depends()):
         if response.status_code >= 400:
             # If ever a route handler returns an http exception, we do not want the
             # session opened by current context manager to commit anything in db.
-            logger.warning("http error, rolling back", status_code=response.status_code)
+            if is_dirty:
+                # optimistically only log if there were uncommitted changes
+                logger.warning(
+                    "http error, rolling back possibly uncommitted changes",
+                    status_code=response.status_code,
+                )
+            # since this is no-op if session is not dirty, we can always call it
             await loop.run_in_executor(None, session.rollback)
 
     return response

tests/test_middleware.py

@@ -159,7 +159,7 @@ async def test_commit_error_returns_500(client, user_1, mock_middleware):
     } in caplog
 
     assert {
-        "event": "http error, rolling back",
+        "event": "http error, rolling back possibly uncommitted changes",
         "log_level": "warning",
         "status_code": 500,
     } in caplog
@@ -175,3 +175,16 @@ async def test_rollback_on_http_exception(client, mock_middleware):
 
         session.rollback.assert_called_once_with()
         mock_middleware.assert_called_once()
+
+
+async def test_rollback_on_http_exception_silent(client, mock_middleware):
+    with capture_logs() as caplog:
+        await client.get("/404")
+
+    mock_middleware.assert_called_once()
+
+    assert {
+        "event": "http error, rolling back possibly uncommitted changes",
+        "log_level": "warning",
+        "status_code": 404,
+    } not in caplog