From 891afee6dff62060fa4be27178745276cc62ee49 Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 17 Oct 2024 00:30:54 -0700 Subject: [PATCH] fix(spotlight): More defensive Django spotlight middleware injection (#3665) Turns out `settings.MIDDLEWARE` does not have to be a `list`. This causes issues as not all iterables support appending items to them. This PR leverages `itertools.chain` along with `type(settings.MIDDLEWARE)` to extend the middleware list while keeping its original type. It also adds a try-except block around the injection code to make sure it doesn't block anything further down in the unexpected case that it fails. --- sentry_sdk/spotlight.py | 18 ++++++++++++++---- tests/integrations/django/test_basic.py | 4 ++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/spotlight.py b/sentry_sdk/spotlight.py index e21bf56545..b1ebf847ab 100644 --- a/sentry_sdk/spotlight.py +++ b/sentry_sdk/spotlight.py @@ -5,6 +5,8 @@ import urllib.error import urllib3 +from itertools import chain + from typing import TYPE_CHECKING if TYPE_CHECKING: @@ -13,11 +15,12 @@ from typing import Dict from typing import Optional -from sentry_sdk.utils import logger, env_to_bool +from sentry_sdk.utils import logger, env_to_bool, capture_internal_exceptions from sentry_sdk.envelope import Envelope DEFAULT_SPOTLIGHT_URL = "http://localhost:8969/stream" +DJANGO_SPOTLIGHT_MIDDLEWARE_PATH = "sentry_sdk.spotlight.SpotlightMiddleware" class SpotlightClient: @@ -112,9 +115,16 @@ def setup_spotlight(options): else: return None - if settings is not None and env_to_bool( - os.environ.get("SENTRY_SPOTLIGHT_ON_ERROR", "1") + if ( + settings is not None + and settings.DEBUG + and env_to_bool(os.environ.get("SENTRY_SPOTLIGHT_ON_ERROR", "1")) ): - settings.MIDDLEWARE.append("sentry_sdk.spotlight.SpotlightMiddleware") + with capture_internal_exceptions(): + middleware = settings.MIDDLEWARE + if DJANGO_SPOTLIGHT_MIDDLEWARE_PATH not in middleware: + settings.MIDDLEWARE = type(middleware)( + chain(middleware, (DJANGO_SPOTLIGHT_MIDDLEWARE_PATH,)) + ) return SpotlightClient(url) diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index a8cc02fda5..c8282412ea 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -1247,6 +1247,7 @@ def test_ensures_spotlight_middleware_when_spotlight_is_enabled(sentry_init, set Test that ensures if Spotlight is enabled, relevant SpotlightMiddleware is added to middleware list in settings. """ + settings.DEBUG = True original_middleware = frozenset(settings.MIDDLEWARE) sentry_init(integrations=[DjangoIntegration()], spotlight=True) @@ -1263,6 +1264,7 @@ def test_ensures_no_spotlight_middleware_when_env_killswitch_is_false( Test that ensures if Spotlight is enabled, but is set to a falsy value the relevant SpotlightMiddleware is NOT added to middleware list in settings. """ + settings.DEBUG = True monkeypatch.setenv("SENTRY_SPOTLIGHT_ON_ERROR", "no") original_middleware = frozenset(settings.MIDDLEWARE) @@ -1281,6 +1283,8 @@ def test_ensures_no_spotlight_middleware_when_no_spotlight( Test that ensures if Spotlight is not enabled the relevant SpotlightMiddleware is NOT added to middleware list in settings. """ + settings.DEBUG = True + # We should NOT have the middleware even if the env var is truthy if Spotlight is off monkeypatch.setenv("SENTRY_SPOTLIGHT_ON_ERROR", "1")