Fix last_event_id() in nested scopes #3114
Conversation
|
|
| # self._last_event_id is only applicable to isolation scopes | ||
| self._last_event_id = None # type: Optional[str] | ||
|
|
There was a problem hiding this comment.
While I agree that it likely makes sense to move these lines to the clear function, I believe this change should be considered in a separate PR, since it does not appear to be needed to fix #3113; the tests you added pass even if I undo this change.
| # self._last_event_id is only applicable to isolation scopes | ||
| self._last_event_id = None # type: Optional[str] | ||
|
|
There was a problem hiding this comment.
Likely belongs in separate PR, see previous comment.
| # Check all attributes are copied | ||
| for name in set(Scope.__slots__) - {"client"}: | ||
| getattr(s2, name) | ||
|
|
There was a problem hiding this comment.
I would prefer to add this check to a separate unit test, since it is somewhat unrelated to the tag copying checks that this existing test is checking. The comment should also be updated to something like "Check that all attributes, except client, are present on the new scope", as the check only verifies the attributes' presence, and does not check whether the values were actually copied.
| sentry_init(enable_tracing=True) | ||
|
|
||
| # Should not crash | ||
| with push_scope() as scope: |
There was a problem hiding this comment.
| with push_scope() as scope: | |
| with isolation_scope() as scope: |
push_scope is deprecated since version 2.0.0, so let's use the new isolation_scope function here, instead. Using isolation_scope also fails on master but passes on this branch.
|
@sl0thentr0py, removing @antonpirker's assignment since I have already reviewed – I wrote the original Scope-based |
|
Closing in favor of #3123, which is based on this PR, but includes the changes I requested here |
|
Also, I split your changes to clear |
|
Thank you for the fix. I didn’t have to look at this because I wasn’t working on the related contract. |
Fixes #3113.
I added two tests, one that generally checks
Scope.__copy__()copies everything that it should, and one top-level one that reproduces the bug seen. Onmaster, both fail with:@szokeasaurusrex please take a look :)