-
Notifications
You must be signed in to change notification settings - Fork 544
fix: RuntimeError: dictionary changed size during iteration
raised within extract_locals
#298
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
Conversation
shenek
commented
Mar 20, 2019
- affects only python3 because items() in python3 doesn't return list
- note that this situation is actually a simplified example how environ is treated in Bottle framework
That's messed up. Can you link me to the code in bottle that changes the environ during |
tests/integrations/wsgi/test_wsgi.py
Outdated
@@ -30,3 +51,15 @@ def test_basic(sentry_init, crashing_app, capture_events): | |||
"query_string": "", | |||
"url": "http://localhost/", | |||
} | |||
|
|||
|
|||
def test_env_modifing_(sentry_init, crashing_env_modifing_app, capture_events): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you fix this name to something that doesn't have an underscore at the end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, sorry I missed that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There supposed to be _app
d267e6e
to
ae8e794
Compare
…raised in extract_locals phase * affects only python3 because items() in python3 doesn't return list * note that this situation is actually a simplified example how environ is treated in Bottle framework
ae8e794
to
30f1ec6
Compare
The code itself in bottle is not very 'visible'. And it took a quite amount of time to debug. But I can at least show you a part of my trace. So lets start with |
Amazing. I'll include this in 0.7.7. Thanks! |
You're welcomed. |
FYI, with #368 this will soon spit out a placeholder instead of the value. Using |
I'm not sure, but perhaps something like this could help: diff --git a/sentry_sdk/_compat.py b/sentry_sdk/_compat.py
index 871995e..5dbf158 100644
--- a/sentry_sdk/_compat.py
+++ b/sentry_sdk/_compat.py
@@ -1,14 +1,28 @@
import sys
+import copy
if False:
from typing import Optional
from typing import Tuple
from typing import Any
from typing import Type
+ from typing import Callable
PY2 = sys.version_info[0] == 2
+
+def safe_iteritems_wrapper(unsafe_iter):
+ # type: (Callable[[dict], None]) -> Callable[[dict], None]
+ def wrapped(x):
+ try:
+ return unsafe_iter(x)
+ except RuntimeError:
+ return copy.copy(x).items()
+
+ return wrapped
+
+
if PY2:
import urlparse # noqa
@@ -52,6 +66,9 @@ else:
raise value
+iteritems = safe_iteritems_wrapper(iteritems)
+
+
def with_metaclass(meta, *bases):
class metaclass(type):
def __new__(cls, name, this_bases, d): |
Do you think an easy fix for this could be that we just serialize the event breadth-first? |
I would doubt that. The problem is that you are modifying the dict which are you currently iterating.
I guess that it would only have failed sooner. I'll try to run some test for that later. |
Nevermind, |