8000 fix: Fix confusing/buggy push_scope behavior with callbacks by untitaker · Pull Request #156 · getsentry/sentry-python · GitHub
[go: up one dir, main page]

Skip to content

fix: Fix confusing/buggy push_scope behavior with callbacks #156

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 31, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
8000 Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 10 additions & 18 deletions sentry_sdk/hub.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ def __init__(self, hub, layer):

def __enter__(self):
scope = self._layer[1]
if scope is None:
scope = Scope()
assert scope is not None
return scope

def __exit__(self, exc_type, exc_value, tb):
Expand Down Expand Up @@ -271,21 +270,24 @@ def push_scope(self, callback=None):
that should be used to pop the scope again. Alternatively a callback
can be provided that is executed in the context of the scope.
"""
if callback is not None:
with self.push_scope() as scope:
callback(scope)
return

client, scope = self._stack[-1]
new_layer = (client, copy.copy(scope))
self._stack.append(new_layer)

if callback is not None:
if client is not None:
callback(scope)
else:
return _ScopeManager(self, new_layer)
return _ScopeManager(self, new_layer)

scope = push_scope

def pop_scope_unsafe(self):
"""Pops a scope layer from the stack. Try to use the context manager
`push_scope()` instead."""
rv = self._stack.pop()
assert self._stack
assert self._stack, "stack must have at least one layer"
return rv

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

return inner()

def scope(self, callback=None):
"""Pushes a new scope and yields it for configuration.

The scope is dropped at the end of the with statement. Alternatively
a callback can be provided similar to `configure_scope`.
"""
with self.push_scope():
client, scope = self._stack[-1]
return self.configure_scope(callback)


GLOBAL_HUB = Hub()
_local.set(GLOBAL_HUB)
29 changes: 29 additions & 0 deletions tests/test_basics.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
import logging

import pytest

from sentry_sdk import (
Client,
push_scope,
Expand Down Expand Up @@ -144,6 +147,32 @@ def test_push_scope_null_client(sentry_init, capture_events):
assert len(events) == 0


@pytest.mark.parametrize("null_client", (True, False))
def test_push_scope_callback(sentry_init, null_client, capture_events):
sentry_init()

if null_client:
Hub.current.bind_client(None)

outer_scope = Hub.current._stack[-1][1]

calls = []

@push_scope
def _(scope):
assert scope is Hub.current._stack[-1][1]
assert scope is not outer_scope
calls.append(1)

# push_scope always needs to execute the callback regardless of
# client state, because that actually runs usercode in it, not
# just scope config code
assert calls == [1]

# Assert scope gets popped correctly
assert Hub.current._stack[-1][1] is outer_scope


def test_breadcrumbs(sentry_init, capture_events):
sentry_init(max_breadcrumbs=10)
events = capture_events()
Expand Down
0