8000 fix: Fix confusing/buggy push_scope behavior with callbacks (#156) · etherscan-io/sentry-python@3fe8712 · GitHub
[go: up one dir, main page]

Skip to content

Commit 3fe8712

Browse files
authored
fix: Fix confusing/buggy push_scope behavior with callbacks (getsentry#156)
* fix: Fix confusing/buggy push_scope behavior * fix: Styling * fix: Add assertion instead of hopefully noop conditional
1 parent 5b15aaa commit 3fe8712

File tree

2 files changed

+39
-18
lines changed

2 files changed

+39
-18
lines changed

sentry_sdk/hub.py

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ def __init__(self, hub, layer):
8686

8787
def __enter__(self):
8888
scope = self._layer[1]
89-
if scope is None:
90-
scope = Scope()
89+
assert scope is not None
9190
return scope
9291

9392
def __exit__(self, exc_type, exc_value, tb):
@@ -271,21 +270,24 @@ def push_scope(self, callback=None):
271270
that should be used to pop the scope again. Alternatively a callback
272271
can be provided that is executed in the context of the scope.
273272
"""
273+
if callback is not None:
274+
with self.push_scope() as scope:
275+
callback(scope)
276+
return
277+
274278
client, scope = self._stack[-1]
275279
new_layer = (client, copy.copy(scope))
276280
self._stack.append(new_layer)
277281

278-
if callback is not None:
279-
if client is not None:
280-
callback(scope)
281-
else:
282-
return _ScopeManager(self, new_layer)
282+
return _ScopeManager(self, new_layer)
283+
284+
scope = push_scope
283285

284286
def pop_scope_unsafe(self):
285287
"""Pops a scope layer from the stack. Try to use the context manager
286288
`push_scope()` instead."""
287289
rv = self._stack.pop()
288-
assert self._stack
290+
assert self._stack, "stack must have at least one layer"
289291
return rv
290292

291293
def configure_scope(self, callback=None):
@@ -305,16 +307,6 @@ def inner():
305307

306308
return inner()
307309

308-
def scope(self, callback=None):
309-
"""Pushes a new scope and yields it for configuration.
310-
311-
The scope is dropped at the end of the with statement. Alternatively
312-
a callback can be provided similar to `configure_scope`.
313-
"""
314-
with self.push_scope():
315-
client, scope = self._stack[-1]
316-
return self.configure_scope(callback)
317-
318310

319311
GLOBAL_HUB = Hub()
320312
_local.set(GLOBAL_HUB)

tests/test_basics.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
import logging
2+
3+
import pytest
4+
25
from sentry_sdk import (
36
Client,
47
push_scope,
@@ -144,6 +147,32 @@ def test_push_scope_null_client(sentry_init, capture_events):
144147
assert len(events) == 0
145148

146149

150+
@pytest.mark.parametrize("null_client", (True, False))
151+
def test_push_scope_callback(sentry_init, null_client, capture_events):
152+
sentry_init()
153+
154+
if null_client:
155+
Hub.current.bind_client(None)
156+
157+
outer_scope = Hub.current._stack[-1][1]
158+
159+
calls = []
160+
161+
@push_scope
162+
def _(scope):
163+
assert scope is Hub.current._stack[-1][1]
164+
assert scope is not outer_scope
165+
calls.append(1)
166+
167+
# push_scope always needs to execute the callback regardless of
168+
# client state, because that actually runs usercode in it, not
169+
# just scope config code
170+
assert calls == [1]
171+
172+
# Assert scope gets popped correctly
173+
assert Hub.current._stack[-1][1] is outer_scope
174+
175+
147176
def test_breadcrumbs(sentry_init, capture_events):
148177
sentry_init(max_breadcrumbs=10)
149178
events = capture_events()

0 commit comments

Comments
 (0)
0