Skip to content

Commit

Permalink
fix(spotlight): More defensive Django spotlight middleware injection (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
BYK authored Oct 17, 2024
1 parent f493057 commit 891afee
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
18 changes: 14 additions & 4 deletions sentry_sdk/spotlight.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import urllib.error
import urllib3

from itertools import chain

from typing import TYPE_CHECKING

if TYPE_CHECKING:
Expand All @@ -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:
Expand Down Expand Up @@ -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)
4 changes: 4 additions & 0 deletions tests/integrations/django/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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")

Expand Down

0 comments on commit 891afee

Please sign in to comment.