diff --git a/app/main.py b/app/main.py index afa66525..5c9f4fc9 100644 --- a/app/main.py +++ b/app/main.py @@ -284,13 +284,13 @@ async def lifespan(app: FastAPI): # Shutdown: Cleanup tasks try: logging.info("Application shutting down") - except Exception: + except Exception: # noqa: S110 pass # During test teardown, logging streams may already be closed # Send shutdown notification try: notify_shutdown() - except Exception: + except Exception: # noqa: S110 pass # During test teardown, I/O streams may already be closed diff --git a/app/utils/settings_service.py b/app/utils/settings_service.py index cec83c24..75c64481 100644 --- a/app/utils/settings_service.py +++ b/app/utils/settings_service.py @@ -3388,11 +3388,11 @@ def update_env_file(env_path: str, settings_to_update: dict[str, str]) -> bool: Returns: 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: + 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}") # Read the current .env file diff --git a/tests/test_api_google_drive_comprehensive.py b/tests/test_api_google_drive_comprehensive.py index 6275602f..abd54707 100644 --- a/tests/test_api_google_drive_comprehensive.py +++ b/tests/test_api_google_drive_comprehensive.py @@ -494,14 +494,14 @@ class TestSaveGoogleDriveSettings: @patch("os.path.dirname") @patch("app.config.settings") 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") response = client.post("/api/google-drive/save-settings", data={"refresh_token": "token", "use_oauth": "true"}) - assert response.status_code == 500 - data = response.json() - assert "failed to save" in data["detail"].lower() + # .env write exception is caught; endpoint succeeds via DB write + assert response.status_code == 200 + assert response.json()["status"] == "success" @pytest.mark.unit diff --git a/tests/test_api_imap_accounts.py b/tests/test_api_imap_accounts.py index d872547b..90381cdc 100644 --- a/tests/test_api_imap_accounts.py +++ b/tests/test_api_imap_accounts.py @@ -524,7 +524,10 @@ class TestTestImapConnection: from app.api.imap_accounts import _test_imap_connection 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( "imap.example.com", 993, @@ -541,7 +544,10 @@ class TestTestImapConnection: """An exception raised by IMAP4_SSL returns success=False.""" 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( "imap.example.com", 993, @@ -557,7 +563,10 @@ class TestTestImapConnection: """An OSError returns success=False with a network error message.""" 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( "bad-host", 143, diff --git a/tests/test_main.py b/tests/test_main.py index b59c2c93..015c499f 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -145,6 +145,58 @@ class TestLifespanEvents: # load_settings_from_db must also have been called 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 class TestExceptionHandlers: