Fix async generator exception handling and add comprehensive tests
This commit is contained in:
@@ -119,56 +119,86 @@ For each task completed:
|
|||||||
|
|
||||||
## TODO List:
|
## TODO List:
|
||||||
|
|
||||||
✅ **Task Completed Successfully**
|
fix:
|
||||||
|
|
||||||
### Issue Fixed: TypeError: 'async for' requires an object with **aiter** method
|
INFO: 127.0.0.1:34916 - "POST /api/anime/search HTTP/1.1" 200
|
||||||
|
INFO: 127.0.0.1:34916 - "POST /api/anime/add HTTP/1.1" 500
|
||||||
|
ERROR: Exception in ASGI application
|
||||||
|
Traceback (most recent call last):
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/anyio/streams/memory.py", line 98, in receive
|
||||||
|
return self.receive_nowait()
|
||||||
|
~~~~~~~~~~~~~~~~~~~^^
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/anyio/streams/memory.py", line 93, in receive_nowait
|
||||||
|
raise WouldBlock
|
||||||
|
anyio.WouldBlock
|
||||||
|
|
||||||
**Problem:**
|
During handling of the above exception, another exception occurred:
|
||||||
The BackgroundLoaderService was crashing when trying to load series data with the error:
|
|
||||||
|
|
||||||
```
|
Traceback (most recent call last):
|
||||||
TypeError: 'async for' requires an object with __aiter__ method, got _AsyncGeneratorContextManager
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/base.py", line 78, in call_next
|
||||||
```
|
message = await recv_stream.receive()
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/anyio/streams/memory.py", line 118, in receive
|
||||||
|
raise EndOfStream
|
||||||
|
anyio.EndOfStream
|
||||||
|
|
||||||
This error occurred in `_load_series_data` method at line 282 of [background_loader_service.py](src/server/services/background_loader_service.py).
|
During handling of the above exception, another exception occurred:
|
||||||
|
|
||||||
**Root Cause:**
|
Traceback (most recent call last):
|
||||||
The code was incorrectly using `async for db in get_db_session():` to get a database session. However, `get_db_session()` is decorated with `@asynccontextmanager`, which returns an async context manager (not an async iterator). Async context managers must be used with `async with`, not `async for`.
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/uvicorn/protocols/http/httptools_impl.py", line 426, in run_asgi
|
||||||
|
result = await app( # type: ignore[func-returns-value]
|
||||||
**Solution:**
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
Changed the database session acquisition from:
|
self.scope, self.receive, self.send
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
```python
|
)
|
||||||
async for db in get_db_session():
|
^
|
||||||
# ... code ...
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/uvicorn/middleware/proxy_headers.py", line 84, in **call**
|
||||||
break # Exit loop after first iteration
|
return await self.app(scope, receive, send)
|
||||||
```
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/fastapi/applications.py", line 1106, in **call**
|
||||||
To the correct pattern:
|
await super().**call**(scope, receive, send)
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/applications.py", line 122, in **call**
|
||||||
```python
|
await self.middleware_stack(scope, receive, send)
|
||||||
async with get_db_session() as db:
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/errors.py", line 184, in **call**
|
||||||
# ... code ...
|
raise exc
|
||||||
```
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/errors.py", line 162, in **call**
|
||||||
|
await self.app(scope, receive, \_send)
|
||||||
**Files Modified:**
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/base.py", line 108, in **call**
|
||||||
|
response = await self.dispatch_func(request, call_next)
|
||||||
1. [src/server/services/background_loader_service.py](src/server/services/background_loader_service.py) - Fixed async context manager usage
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
2. [tests/unit/test_background_loader_session.py](tests/unit/test_background_loader_session.py) - Created comprehensive unit tests
|
File "/home/lukas/Volume/repo/Aniworld/src/server/middleware/auth.py", line 209, in dispatch
|
||||||
|
return await call_next(request)
|
||||||
**Tests:**
|
^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/base.py", line 84, in call_next
|
||||||
- ✅ 5 new unit tests for background loader database session handling (all passing)
|
raise app_exc
|
||||||
- ✅ Tests verify proper async context manager usage
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/base.py", line 70, in coro
|
||||||
- ✅ Tests verify error handling and progress tracking
|
await self.app(scope, receive_or_disconnect, send_no_error)
|
||||||
- ✅ Tests verify episode and NFO loading logic
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/base.py", line 108, in **call**
|
||||||
- ✅ Includes test demonstrating the difference between `async with` and `async for`
|
response = await self.dispatch_func(request, call_next)
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
**Verification:**
|
File "/home/lukas/Volume/repo/Aniworld/src/server/middleware/setup_redirect.py", line 120, in dispatch
|
||||||
The fix allows the background loader service to properly load series data including episodes, NFO files, logos, and images without crashing.
|
return await call_next(request)
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^^^
|
||||||
---
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/base.py", line 84, in call_next
|
||||||
|
raise app_exc
|
||||||
## Next Tasks:
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/base.py", line 70, in coro
|
||||||
|
await self.app(scope, receive_or_disconnect, send_no_error)
|
||||||
No pending tasks at this time.
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/cors.py", line 91, in **call**
|
||||||
|
await self.simple_response(scope, receive, send, request_headers=headers)
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/cors.py", line 146, in simple_response
|
||||||
|
await self.app(scope, receive, send)
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/exceptions.py", line 79, in **call**
|
||||||
|
raise exc
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/starlette/middleware/exceptions.py", line 68, in **call**
|
||||||
|
await self.app(scope, receive, sender)
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/site-packages/fastapi/middleware/asyncexitstack.py", line 14, in **call**
|
||||||
|
async with AsyncExitStack() as stack:
|
||||||
|
~~~~~~~~~~~~~~^^
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/contextlib.py", line 768, in **aexit**
|
||||||
|
raise exc
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/contextlib.py", line 751, in **aexit**
|
||||||
|
cb_suppress = await cb(\*exc_details)
|
||||||
|
^^^^^^^^^^^^^^^^^^^^^^
|
||||||
|
File "/home/lukas/miniconda3/envs/AniWorld/lib/python3.13/contextlib.py", line 271, in **aexit**
|
||||||
|
raise RuntimeError("generator didn't stop after athrow()")
|
||||||
|
RuntimeError: generator didn't stop after athrow()
|
||||||
|
|||||||
@@ -170,12 +170,7 @@ async def get_optional_database_session() -> AsyncGenerator:
|
|||||||
from src.server.database import get_db_session
|
from src.server.database import get_db_session
|
||||||
|
|
||||||
async with get_db_session() as session:
|
async with get_db_session() as session:
|
||||||
try:
|
|
||||||
yield session
|
yield session
|
||||||
except Exception:
|
|
||||||
# Re-raise the exception to let FastAPI handle it
|
|
||||||
# This prevents "generator didn't stop after athrow()" error
|
|
||||||
raise
|
|
||||||
except (ImportError, RuntimeError):
|
except (ImportError, RuntimeError):
|
||||||
# Database not available - yield None
|
# Database not available - yield None
|
||||||
yield None
|
yield None
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ from src.server.utils.dependencies import (
|
|||||||
common_parameters,
|
common_parameters,
|
||||||
get_current_user,
|
get_current_user,
|
||||||
get_database_session,
|
get_database_session,
|
||||||
|
get_optional_database_session,
|
||||||
get_series_app,
|
get_series_app,
|
||||||
log_request_dependency,
|
log_request_dependency,
|
||||||
optional_auth,
|
optional_auth,
|
||||||
@@ -291,6 +292,156 @@ class TestUtilityDependencies:
|
|||||||
# Assert - no exception should be raised
|
# Assert - no exception should be raised
|
||||||
|
|
||||||
|
|
||||||
|
class TestOptionalDatabaseSession:
|
||||||
|
"""Test cases for optional database session dependency."""
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_optional_database_session_success(self):
|
||||||
|
"""Test successful database session creation."""
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
# Mock the database session
|
||||||
|
mock_session = AsyncMock()
|
||||||
|
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||||
|
mock_session.__aexit__ = AsyncMock(return_value=None)
|
||||||
|
|
||||||
|
mock_get_db = Mock(return_value=mock_session)
|
||||||
|
|
||||||
|
with patch('src.server.database.get_db_session', mock_get_db):
|
||||||
|
# Act
|
||||||
|
gen = get_optional_database_session()
|
||||||
|
session = await gen.__anext__()
|
||||||
|
|
||||||
|
# Assert
|
||||||
|
assert session is mock_session
|
||||||
|
mock_get_db.assert_called_once()
|
||||||
|
|
||||||
|
# Cleanup
|
||||||
|
try:
|
||||||
|
await gen.aclose()
|
||||||
|
except StopAsyncIteration:
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_optional_database_session_not_available(self):
|
||||||
|
"""Test when database is not available (ImportError)."""
|
||||||
|
# Mock ImportError when trying to import get_db_session
|
||||||
|
with patch(
|
||||||
|
'src.server.database.get_db_session',
|
||||||
|
side_effect=ImportError("No module named 'database'")
|
||||||
|
):
|
||||||
|
# Act
|
||||||
|
gen = get_optional_database_session()
|
||||||
|
session = await gen.__anext__()
|
||||||
|
|
||||||
|
# Assert - should return None when database not available
|
||||||
|
assert session is None
|
||||||
|
|
||||||
|
# Cleanup
|
||||||
|
try:
|
||||||
|
await gen.aclose()
|
||||||
|
except StopAsyncIteration:
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_optional_database_session_runtime_error(self):
|
||||||
|
"""Test when database raises RuntimeError."""
|
||||||
|
# Mock RuntimeError when trying to get database session
|
||||||
|
with patch(
|
||||||
|
'src.server.database.get_db_session',
|
||||||
|
side_effect=RuntimeError("Database connection failed")
|
||||||
|
):
|
||||||
|
# Act
|
||||||
|
gen = get_optional_database_session()
|
||||||
|
session = await gen.__anext__()
|
||||||
|
|
||||||
|
# Assert - should return None on RuntimeError
|
||||||
|
assert session is None
|
||||||
|
|
||||||
|
# Cleanup
|
||||||
|
try:
|
||||||
|
await gen.aclose()
|
||||||
|
except StopAsyncIteration:
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_optional_database_session_exception_during_use(self):
|
||||||
|
"""
|
||||||
|
Test that exceptions during database operations are properly propagated.
|
||||||
|
|
||||||
|
This test specifically addresses the "generator didn't stop after athrow()"
|
||||||
|
error that occurred when exceptions were caught and re-raised within the
|
||||||
|
yield block of an async context manager.
|
||||||
|
"""
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
# Create a mock session that will raise an exception when used
|
||||||
|
mock_session = AsyncMock()
|
||||||
|
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||||
|
mock_session.__aexit__ = AsyncMock(return_value=None)
|
||||||
|
|
||||||
|
# Mock the get_db_session to return our mock session
|
||||||
|
mock_get_db = Mock(return_value=mock_session)
|
||||||
|
|
||||||
|
with patch('src.server.database.get_db_session', mock_get_db):
|
||||||
|
# Act & Assert
|
||||||
|
gen = get_optional_database_session()
|
||||||
|
session = await gen.__anext__()
|
||||||
|
|
||||||
|
assert session is mock_session
|
||||||
|
|
||||||
|
# Simulate an exception being thrown into the generator
|
||||||
|
# This mimics what happens when an endpoint raises an exception
|
||||||
|
# after the dependency has yielded
|
||||||
|
test_exception = ValueError("Database operation failed")
|
||||||
|
|
||||||
|
try:
|
||||||
|
# This should not cause "generator didn't stop after athrow()" error
|
||||||
|
await gen.athrow(test_exception)
|
||||||
|
# If we get here, the exception was swallowed (shouldn't happen)
|
||||||
|
pytest.fail("Exception should have been propagated")
|
||||||
|
except ValueError as e:
|
||||||
|
# Exception should be properly propagated
|
||||||
|
assert str(e) == "Database operation failed"
|
||||||
|
except StopAsyncIteration:
|
||||||
|
# Generator stopped normally after exception
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_optional_database_session_cleanup_on_exception(self):
|
||||||
|
"""Test that database session is properly cleaned up when exception occurs."""
|
||||||
|
from unittest.mock import AsyncMock
|
||||||
|
|
||||||
|
# Track cleanup
|
||||||
|
cleanup_called = []
|
||||||
|
|
||||||
|
async def mock_exit(*args):
|
||||||
|
cleanup_called.append(True)
|
||||||
|
return None
|
||||||
|
|
||||||
|
# Create a mock session with tracked cleanup
|
||||||
|
mock_session = AsyncMock()
|
||||||
|
mock_session.__aenter__ = AsyncMock(return_value=mock_session)
|
||||||
|
mock_session.__aexit__ = mock_exit
|
||||||
|
|
||||||
|
mock_get_db = Mock(return_value=mock_session)
|
||||||
|
|
||||||
|
with patch('src.server.database.get_db_session', mock_get_db):
|
||||||
|
gen = get_optional_database_session()
|
||||||
|
session = await gen.__anext__()
|
||||||
|
|
||||||
|
assert session is mock_session
|
||||||
|
|
||||||
|
# Throw an exception to simulate endpoint failure
|
||||||
|
try:
|
||||||
|
await gen.athrow(RuntimeError("Simulated endpoint error"))
|
||||||
|
except (RuntimeError, StopAsyncIteration):
|
||||||
|
pass
|
||||||
|
|
||||||
|
# Assert cleanup was called
|
||||||
|
assert len(cleanup_called) > 0, "Session cleanup should have been called"
|
||||||
|
|
||||||
|
|
||||||
class TestIntegrationScenarios:
|
class TestIntegrationScenarios:
|
||||||
"""Integration test scenarios for dependency injection."""
|
"""Integration test scenarios for dependency injection."""
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user