fix(sentry): configure sentry before app instantiation (#258)

* 🐛 fix(sentry): call configure before starlite init

* chore: changes for rebase on 0.29

Co-authored-by: Peter Schutt <peter.github@proton.me>

src/starlite_saqlalchemy/exceptions.py

@@ -30,6 +30,8 @@
 __all__ = (
     "AuthorizationError",
     "ConflictError",
+    "HealthCheckConfigurationError",
+    "MissingDependencyError",
     "NotFoundError",
     "StarliteSaqlalchemyError",
     "after_exception_hook_handler",
@@ -60,6 +62,12 @@ class MissingDependencyError(StarliteSaqlalchemyError, ValueError):
     """A required dependency is not installed."""
 
     def __init__(self, module: str, config: str | None = None) -> None:
+        """
+
+        Args:
+            module: name of the package that should be installed
+            config: name of the extra to install the package
+        """
         config = config if config else module
         super().__init__(
             f"You enabled {config} configuration but package {module!r} is not installed. "

src/starlite_saqlalchemy/init_plugin.py

@@ -211,6 +211,9 @@ def __init__(self, config: PluginConfig | None = None) -> None:
             config: Plugin configuration object.
         """
         self.config = config if config is not None else PluginConfig()
+        # We must configure sentry before app is instantiated
+        # because sentry integration replaces Starlite.__init__
+        self.configure_sentry()
 
     def __call__(self, app_config: AppConfig) -> AppConfig:
         """Entrypoint to the app config plugin.
@@ -231,7 +234,6 @@ def __call__(self, app_config: AppConfig) -> AppConfig:
         self.configure_exception_handlers(app_config)
         self.configure_logging(app_config)
         self.configure_openapi(app_config)
-        self.configure_sentry(app_config)
         self.configure_sqlalchemy_plugin(app_config)
         self.configure_type_encoders(app_config)
         self.configure_worker(app_config)
@@ -368,7 +370,7 @@ def configure_openapi(self, app_config: AppConfig) -> None:
         if self.config.do_openapi and app_config.openapi_config == DEFAULT_OPENAPI_CONFIG:
             app_config.openapi_config = openapi.config
 
-    def configure_sentry(self, app_config: AppConfig) -> None:
+    def configure_sentry(self) -> None:
         """Add handler to configure Sentry integration.
 
         Args:
@@ -377,7 +379,7 @@ def configure_sentry(self, app_config: AppConfig) -> None:
         if self.config.do_sentry:
             from starlite_saqlalchemy import sentry
 
-            app_config.on_startup.append(sentry.configure)
+            sentry.configure()
 
     def configure_sqlalchemy_plugin(self, app_config: AppConfig) -> None:
         """Configure `SQLAlchemy` for the application.

src/starlite_saqlalchemy/sentry.py

@@ -37,7 +37,7 @@ def configure() -> None:
 
     See [SentrySettings][starlite_saqlalchemy.settings.SentrySettings].
     """
-    sentry_sdk.init(  # pragma: no cover
+    sentry_sdk.init(
         dsn=settings.sentry.DSN,
         environment=settings.app.ENVIRONMENT,
         release=settings.app.BUILD_NUMBER,

tests/unit/no_extras/test_init_plugin.py → tests/unit/no_extras/test_init_plugin_for_no_extras.py

@@ -1,4 +1,7 @@
 """Tests for init_plugin.py when no extra dependencies are installed."""
+# pylint:disable=duplicate-code
+
+from __future__ import annotations
 
 import pytest
 from pydantic import ValidationError

tests/unit/redis/worker/test_init_plugin_for_worker.py

@@ -0,0 +1,27 @@
+"""Tests for init_plugin.py."""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+from unittest.mock import MagicMock
+
+from starlite import Starlite
+
+from starlite_saqlalchemy import init_plugin, worker
+
+if TYPE_CHECKING:
+
+    from pytest import MonkeyPatch
+
+
+def test_do_worker_but_not_logging(monkeypatch: MonkeyPatch) -> None:
+    """Tests branch where we can have the worker enabled, but logging
+    disabled."""
+    mock = MagicMock()
+    monkeypatch.setattr(worker, "create_worker_instance", mock)
+    config = init_plugin.PluginConfig(do_logging=False)
+    Starlite(route_handlers=[], on_app_init=[init_plugin.ConfigureApp(config=config)])
+    mock.assert_called_once()
+    call = mock.mock_calls[0]
+    assert "before_process" not in call.kwargs
+    assert "after_process" not in call.kwargs

tests/unit/sentry/test_init_plugin_for_sentry.py

@@ -0,0 +1,60 @@
+"""Tests for init_plugin.py."""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+from unittest.mock import MagicMock
+
+import pytest
+from sentry_sdk.integrations.starlite import SentryStarliteASGIMiddleware
+from sentry_sdk.integrations.starlite import (
+    exception_handler as sentry_after_exception_handler,
+)
+from starlite import Starlite
+from starlite.handlers import BaseRouteHandler
+from starlite.routes.http import HTTPRoute
+
+from starlite_saqlalchemy import init_plugin, sentry
+
+if TYPE_CHECKING:
+
+    from pytest import MonkeyPatch
+
+
+@pytest.mark.parametrize(
+    ("env", "exp"), [("dev", True), ("prod", True), ("local", False), ("test", False)]
+)
+def test_sentry_environment_gate(env: str, exp: bool, monkeypatch: MonkeyPatch) -> None:
+    """Test that the sentry integration is configured under different
+    environment names."""
+    monkeypatch.setattr(init_plugin, "IS_LOCAL_ENVIRONMENT", env == "local")
+    monkeypatch.setattr(init_plugin, "IS_TEST_ENVIRONMENT", env == "test")
+    mock = MagicMock()
+    monkeypatch.setattr(sentry, "configure", mock)
+    Starlite(route_handlers=[], on_app_init=[init_plugin.ConfigureApp()])
+    assert mock.call_count == int(exp)
+
+
+def test_do_sentry() -> None:
+    """Test that do_sentry flag correctly patch Starlite."""
+    old_init = Starlite.__init__
+    old_route_handle = HTTPRoute.handle
+    old_resolve_middleware = BaseRouteHandler.resolve_middleware
+
+    app = Starlite(
+        route_handlers=[],
+        on_app_init=[init_plugin.ConfigureApp(config=init_plugin.PluginConfig(do_sentry=True))],
+    )
+    after_exception_handlers = []
+    for handler in app.after_exception:
+        handler_fn = handler.ref.value
+        # If wrapped with `starlite.utils.async_partial`
+        if hasattr(handler_fn, "func"):
+            handler_fn = handler_fn.func
+        after_exception_handlers.append(handler_fn)
+    assert SentryStarliteASGIMiddleware in app.middleware  # type: ignore
+    assert sentry_after_exception_handler in after_exception_handlers  # type: ignore
+
+    Starlite.__init__ = old_init  # type: ignore
+    HTTPRoute.handle = old_route_handle  # type: ignore
+    BaseRouteHandler.resolve_middleware = old_resolve_middleware  # type: ignore

tests/unit/test_init_plugin.py

@@ -1,36 +1,59 @@
 """Tests for init_plugin.py."""
-# pylint:disable=import-outside-toplevel
+# pylint:disable=duplicate-code
+
 from __future__ import annotations
 
 from typing import TYPE_CHECKING
-from unittest.mock import MagicMock
 
 import pytest
 from starlite import Starlite
+from starlite.cache import SimpleCacheBackend
 
 from starlite_saqlalchemy import init_plugin
-from starlite_saqlalchemy.constants import IS_SAQ_INSTALLED, IS_SENTRY_SDK_INSTALLED
+from starlite_saqlalchemy.constants import IS_REDIS_INSTALLED
 
 if TYPE_CHECKING:
     from typing import Any
 
-    from pytest import MonkeyPatch
-
 
-@pytest.mark.skipif(not IS_SAQ_INSTALLED, reason="saq is not installed")
-def test_do_worker_but_not_logging(monkeypatch: MonkeyPatch) -> None:
-    """Tests branch where we can have the worker enabled, but logging
-    disabled."""
-    from starlite_saqlalchemy import worker
-
-    mock = MagicMock()
-    monkeypatch.setattr(worker, "create_worker_instance", mock)
-    config = init_plugin.PluginConfig(do_logging=False, do_worker=True)
-    Starlite(route_handlers=[], on_app_init=[init_plugin.ConfigureApp(config=config)])
-    mock.assert_called_once()
-    call = mock.mock_calls[0]
-    assert "before_process" not in call.kwargs
-    assert "after_process" not in call.kwargs
+def test_config_switches() -> None:
+    """Tests that the app produced with all config switches off is as we
+    expect."""
+    config = init_plugin.PluginConfig(
+        do_after_exception=False,
+        do_cache=False,
+        do_compression=False,
+        # pyright reckons this parameter doesn't exist, I beg to differ
+        do_collection_dependencies=False,  # pyright:ignore
+        do_exception_handlers=False,
+        do_health_check=False,
+        do_logging=False,
+        do_openapi=False,
+        do_sentry=False,
+        do_set_debug=False,
+        do_sqlalchemy_plugin=False,
+        do_type_encoders=False,
+        do_worker=False,
+    )
+    app = Starlite(
+        route_handlers=[],
+        openapi_config=None,
+        on_app_init=[init_plugin.ConfigureApp(config=config)],
+    )
+    assert app.compression_config is None
+    assert app.debug is False
+    assert app.logging_config is None
+    assert app.openapi_config is None
+    assert app.response_class is None
+    assert isinstance(app.cache.backend, SimpleCacheBackend)
+    # client.close goes in there unconditionally atm
+    assert len(app.on_shutdown) == 1 if IS_REDIS_INSTALLED is False else 2
+    assert not app.after_exception
+    assert not app.dependencies
+    assert not app.exception_handlers
+    assert not app.on_startup
+    assert not app.plugins
+    assert not app.routes
 
 
 @pytest.mark.parametrize(
@@ -46,18 +69,3 @@ def test_ensure_list(in_: Any, out: Any) -> None:
     """Test _ensure_list() functionality."""
     # pylint: disable=protected-access
     assert init_plugin.ConfigureApp._ensure_list(in_) == out
-
-
-@pytest.mark.skipif(not IS_SENTRY_SDK_INSTALLED, reason="sentry_sdk is not installed")
-@pytest.mark.parametrize(
-    ("env", "exp"), [("dev", True), ("prod", True), ("local", False), ("test", False)]
-)
-def test_sentry_environment_gate(env: str, exp: bool, monkeypatch: MonkeyPatch) -> None:
-    """Test that the sentry integration is configured under different
-    environment names."""
-    from starlite_saqlalchemy import sentry
-
-    monkeypatch.setattr(init_plugin, "IS_LOCAL_ENVIRONMENT", env == "local")
-    monkeypatch.setattr(init_plugin, "IS_TEST_ENVIRONMENT", env == "test")
-    app = Starlite(route_handlers=[], on_app_init=[init_plugin.ConfigureApp()])
-    assert bool(sentry.configure in app.on_startup) is exp  # noqa: SIM901