Merge pull request #828 from christianlouis/copilot/fix-try-except-pass-issues
fix(main): suppress intentional S110 try-except-pass in shutdown lifespan
This commit is contained in:
+2
-2
@@ -284,13 +284,13 @@ async def lifespan(app: FastAPI):
|
|||||||
# Shutdown: Cleanup tasks
|
# Shutdown: Cleanup tasks
|
||||||
try:
|
try:
|
||||||
logging.info("Application shutting down")
|
logging.info("Application shutting down")
|
||||||
except Exception:
|
except Exception: # noqa: S110
|
||||||
pass # During test teardown, logging streams may already be closed
|
pass # During test teardown, logging streams may already be closed
|
||||||
|
|
||||||
# Send shutdown notification
|
# Send shutdown notification
|
||||||
try:
|
try:
|
||||||
notify_shutdown()
|
notify_shutdown()
|
||||||
except Exception:
|
except Exception: # noqa: S110
|
||||||
pass # During test teardown, I/O streams may already be closed
|
pass # During test teardown, I/O streams may already be closed
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -3388,11 +3388,11 @@ def update_env_file(env_path: str, settings_to_update: dict[str, str]) -> bool:
|
|||||||
Returns:
|
Returns:
|
||||||
True if the file was successfully updated, False otherwise (e.g. file not found or write error)
|
True if the file was successfully updated, False otherwise (e.g. file not found or write error)
|
||||||
"""
|
"""
|
||||||
if not os.path.exists(env_path):
|
|
||||||
logger.warning(f".env file not found at {env_path}, skipping file update")
|
|
||||||
return False
|
|
||||||
|
|
||||||
try:
|
try:
|
||||||
|
if not os.path.exists(env_path):
|
||||||
|
logger.warning(f".env file not found at {env_path}, skipping file update")
|
||||||
|
return False
|
||||||
|
|
||||||
logger.info(f"Updating settings in {env_path}")
|
logger.info(f"Updating settings in {env_path}")
|
||||||
|
|
||||||
# Read the current .env file
|
# Read the current .env file
|
||||||
|
|||||||
@@ -494,14 +494,14 @@ class TestSaveGoogleDriveSettings:
|
|||||||
@patch("os.path.dirname")
|
@patch("os.path.dirname")
|
||||||
@patch("app.config.settings")
|
@patch("app.config.settings")
|
||||||
def test_save_settings_exception_handling(self, mock_settings, mock_dirname, mock_exists, client: TestClient):
|
def test_save_settings_exception_handling(self, mock_settings, mock_dirname, mock_exists, client: TestClient):
|
||||||
"""Test exception handling in save settings."""
|
"""Test that exceptions in .env write are non-fatal — DB write still succeeds."""
|
||||||
mock_exists.side_effect = Exception("Unexpected error")
|
mock_exists.side_effect = Exception("Unexpected error")
|
||||||
|
|
||||||
response = client.post("/api/google-drive/save-settings", data={"refresh_token": "token", "use_oauth": "true"})
|
response = client.post("/api/google-drive/save-settings", data={"refresh_token": "token", "use_oauth": "true"})
|
||||||
|
|
||||||
assert response.status_code == 500
|
# .env write exception is caught; endpoint succeeds via DB write
|
||||||
data = response.json()
|
assert response.status_code == 200
|
||||||
assert "failed to save" in data["detail"].lower()
|
assert response.json()["status"] == "success"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
|
|||||||
@@ -524,7 +524,10 @@ class TestTestImapConnection:
|
|||||||
from app.api.imap_accounts import _test_imap_connection
|
from app.api.imap_accounts import _test_imap_connection
|
||||||
|
|
||||||
mock_mail = MagicMock()
|
mock_mail = MagicMock()
|
||||||
with patch("imaplib.IMAP4_SSL", return_value=mock_mail):
|
with (
|
||||||
|
patch("app.api.imap_accounts.is_private_ip", return_value=False),
|
||||||
|
patch("imaplib.IMAP4_SSL", return_value=mock_mail),
|
||||||
|
):
|
||||||
result = _test_imap_connection(
|
result = _test_imap_connection(
|
||||||
"imap.example.com",
|
"imap.example.com",
|
||||||
993,
|
993,
|
||||||
@@ -541,7 +544,10 @@ class TestTestImapConnection:
|
|||||||
"""An exception raised by IMAP4_SSL returns success=False."""
|
"""An exception raised by IMAP4_SSL returns success=False."""
|
||||||
from app.api.imap_accounts import _test_imap_connection
|
from app.api.imap_accounts import _test_imap_connection
|
||||||
|
|
||||||
with patch("imaplib.IMAP4_SSL", side_effect=Exception("auth failed")):
|
with (
|
||||||
|
patch("app.api.imap_accounts.is_private_ip", return_value=False),
|
||||||
|
patch("imaplib.IMAP4_SSL", side_effect=Exception("auth failed")),
|
||||||
|
):
|
||||||
result = _test_imap_connection(
|
result = _test_imap_connection(
|
||||||
"imap.example.com",
|
"imap.example.com",
|
||||||
993,
|
993,
|
||||||
@@ -557,7 +563,10 @@ class TestTestImapConnection:
|
|||||||
"""An OSError returns success=False with a network error message."""
|
"""An OSError returns success=False with a network error message."""
|
||||||
from app.api.imap_accounts import _test_imap_connection
|
from app.api.imap_accounts import _test_imap_connection
|
||||||
|
|
||||||
with patch("imaplib.IMAP4", side_effect=OSError("connection refused")):
|
with (
|
||||||
|
patch("app.api.imap_accounts.is_private_ip", return_value=False),
|
||||||
|
patch("imaplib.IMAP4", side_effect=OSError("connection refused")),
|
||||||
|
):
|
||||||
result = _test_imap_connection(
|
result = _test_imap_connection(
|
||||||
"bad-host",
|
"bad-host",
|
||||||
143,
|
143,
|
||||||
|
|||||||
@@ -145,6 +145,58 @@ class TestLifespanEvents:
|
|||||||
# load_settings_from_db must also have been called
|
# load_settings_from_db must also have been called
|
||||||
mock_load_settings.assert_called_once()
|
mock_load_settings.assert_called_once()
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_lifespan_shutdown_logging_exception_is_silenced(self):
|
||||||
|
"""Exceptions raised by logging.info during shutdown are silently ignored."""
|
||||||
|
|
||||||
|
def _raise_on_shutdown(msg, *args, **kwargs):
|
||||||
|
if "shutting down" in str(msg):
|
||||||
|
raise OSError("stream closed")
|
||||||
|
|
||||||
|
with (
|
||||||
|
patch("app.database.init_db"),
|
||||||
|
patch("app.database.SessionLocal") as mock_session_cls,
|
||||||
|
patch("app.utils.config_loader.load_settings_from_db"),
|
||||||
|
patch("app.utils.config_validator.dump_all_settings"),
|
||||||
|
patch("app.utils.config_validator.check_all_configs", return_value={"email": [], "storage": {}}),
|
||||||
|
patch("app.utils.notification.init_apprise"),
|
||||||
|
patch("app.utils.notification.notify_startup"),
|
||||||
|
patch("app.utils.notification.notify_shutdown"),
|
||||||
|
patch("app.main.init_sentry"),
|
||||||
|
patch("app.main.logging.info", side_effect=_raise_on_shutdown),
|
||||||
|
):
|
||||||
|
mock_db = MagicMock()
|
||||||
|
mock_session_cls.return_value = mock_db
|
||||||
|
|
||||||
|
from app.main import app, lifespan
|
||||||
|
|
||||||
|
# Should complete without raising despite the logging error
|
||||||
|
async with lifespan(app):
|
||||||
|
pass
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_lifespan_shutdown_notify_exception_is_silenced(self):
|
||||||
|
"""Exceptions raised by notify_shutdown during shutdown are silently ignored."""
|
||||||
|
with (
|
||||||
|
patch("app.database.init_db"),
|
||||||
|
patch("app.database.SessionLocal") as mock_session_cls,
|
||||||
|
patch("app.utils.config_loader.load_settings_from_db"),
|
||||||
|
patch("app.utils.config_validator.dump_all_settings"),
|
||||||
|
patch("app.utils.config_validator.check_all_configs", return_value={"email": [], "storage": {}}),
|
||||||
|
patch("app.utils.notification.init_apprise"),
|
||||||
|
patch("app.utils.notification.notify_startup"),
|
||||||
|
patch("app.main.notify_shutdown", side_effect=OSError("stream closed")),
|
||||||
|
patch("app.main.init_sentry"),
|
||||||
|
):
|
||||||
|
mock_db = MagicMock()
|
||||||
|
mock_session_cls.return_value = mock_db
|
||||||
|
|
||||||
|
from app.main import app, lifespan
|
||||||
|
|
||||||
|
# Should complete without raising despite the notify_shutdown error
|
||||||
|
async with lifespan(app):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
class TestExceptionHandlers:
|
class TestExceptionHandlers:
|
||||||
|
|||||||
Reference in New Issue
Block a user