bugfix: refactor event metadata and fix request field (getsentry#10)

untitaker · web-flow · commit 8b2a727f49b7 · 2018-07-22T21:25:03.000+02:00
diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py
@@ -4,7 +4,7 @@
 from .utils import Dsn
 from .transport import Transport
 from .consts import DEFAULT_OPTIONS, SDK_INFO
-from .stripping import strip_event
+from .stripping import strip_event, flatten_metadata
 
 
 NO_DSN = object()
@@ -58,15 +58,15 @@ def _prepare_event(self, event, scope):
         if event.get('platform') is None:
             event['platform'] = 'python'
 
-        if event.get('') is None:
-            event, event_meta = strip_event(event)
-            event[""] = event_meta
+        event = strip_event(event)
+        event = flatten_metadata(event)
+        return event
 
     def capture_event(self, event, scope=None):
         """Captures an event."""
         if self._transport is None:
             return
-        self._prepare_event(event, scope)
+        event = self._prepare_event(event, scope)
         self._transport.capture_event(event)
 
     def drain_events(self, timeout=None):
diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py
@@ -138,7 +138,7 @@ def capture_exception(self, error=None):
             }
             return self.capture_event(event)
         except Exception:
-            capture_internal_exception()
+            self.capture_internal_exception()
 
     def capture_internal_exception(self, error=None):
         """Capture an exception that is likely caused by a bug in the SDK
diff --git a/sentry_sdk/integrations/flask.py b/sentry_sdk/integrations/flask.py
@@ -63,7 +63,6 @@ def _before_request(*args, **kwargs):
             try:
                 scope.user = _get_user_info()
             except Exception:
-                raise
                 get_current_hub().capture_internal_exception()
     except Exception:
         get_current_hub().capture_internal_exception()
diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py
@@ -65,6 +65,10 @@ def apply_to_event(self, event):
         if tags:
             event.setdefault('tags', {}).update(tags)
 
+        tags = self._data.get('request')
+        if tags:
+            event.setdefault('request', {}).update(tags)
+
         contexts = self._data.get('contexts')
         if contexts:
             event.setdefault('contexts', {}).update(contexts)
diff --git a/sentry_sdk/stripping.py b/sentry_sdk/stripping.py
@@ -2,60 +2,87 @@
 
 from ._compat import text_type
 
+
+class AnnotatedValue(object):
+    def __init__(self, value, metadata):
+        self.value = value
+        self.metadata = metadata
+
+
+def flatten_metadata(obj):
+    def inner(obj):
+        if isinstance(obj, Mapping):
+            assert "" not in obj, "Merging metadata not supported"
+            rv = {}
+            meta = {}
+            for k, v in obj.items():
+                rv[k], meta[k] = inner(v)
+                if meta[k] is None:
+                    del meta[k]
+            return rv, (meta or None)
+        if isinstance(obj, Sequence) and not isinstance(obj, (text_type, bytes)):
+            rv = []
+            meta = {}
+            for i, v in enumerate(obj):
+                new_v, meta[i] = inner(v)
+                rv.append(new_v)
+                if meta[i] is None:
+                    del meta[i]
+            return rv, (meta or None)
+        if isinstance(obj, AnnotatedValue):
+            return obj.value, {"": obj.metadata}
+        return obj, None
+
+    obj, meta = inner(obj)
+    if meta is not None:
+        obj[''] = meta
+    return obj
+
+
 def strip_event(event):
-    assert "" not in event, "Merging metadata not supported"
-    old_frames = event.get('stacktrace', {}).get('frames', [])
-    new_frames = []
-    frames_meta = {}
-    meta = {"stacktrace": {"frames": frames_meta}}
+    old_frames = event.get('stacktrace', {}).get('frames', None)
+    if old_frames:
+        event['stacktrace']['frames'] = [strip_frame(frame)
+                                         for frame in old_frames]
 
-    for i, frame in old_frames:
-        frame_meta, new_frame = strip_frame(frame)
-        new_frames.append(new_frame)
-        if frame_meta is not None:
-            frames_meta[str(i)] = frame_meta
+    old_request_data = event.get('request', {}).get('data', None)
+    if old_request_data:
+        event['request']['data'] = strip_databag(old_request_data)
 
-    return event, meta
+    return event
 
 def strip_frame(frame):
     frame['vars'], meta = strip_databag(frame.get('vars'))
     return frame, ({"vars": meta} if meta is not None else None)
 
 def strip_databag(obj, remaining_depth=20):
+    assert not isinstance(obj, bytes), 'bytes should have been normalized before'
     if remaining_depth <= 0:
-        return None, {"": {"rem": [["!dep", "x"]]}}
+        return AnnotatedValue(None, {"": {"rem": [["!dep", "x"]]}})
     if isinstance(obj, text_type):
         return strip_string(obj)
     if isinstance(obj, Mapping):
-        rv = {}
-        meta = {}
-        for k, v in obj.items():
-            rv[k], v_meta = strip_databag(v, remaining_depth - 1)
-            if v_meta is not None:
-                meta[k] = v_meta
-        return rv, meta
+        return {k: strip_databag(v, remaining_depth - 1)
+                for k, v in obj.items()}
     if isinstance(obj, Sequence):
-        rv = []
-        meta = {}
-        for i, v in enumerate(obj):
-            new_v, v_meta = strip_databag(obj, remaining_depth - 1)
-            rv.append(new_v)
-            if v_meta is not None:
-                meta[str(i)] = v_meta
-        return rv, meta
+        return [strip_databag(v, remaining_depth - 1)
+                for v in obj]
+    return obj
 
-    return obj, None
 
-
-def strip_string(value, length=512):
+def strip_string(value, assume_length=None, max_length=512):
+    # TODO: read max_length from config
     if not value:
-        return value, None
-    if len(value) > length:
-        meta = {
-            "": {
-                "len": len(value),
-                "rem": [["!len", "x", length, len(value)]]
+        return value
+    if assume_length is None:
+        assume_length = len(value)
+
+    if assume_length > max_length:
+        return AnnotatedValue(
+            value=value[:max_length - 3] + u'...',
+            metadata={
+                "len": assume_length,
+                "rem": [["!len", "x", max_length - 3, max_length]]
             }
-        }
-        return value[:length - 3] + '...', meta
-    return value[:length], None
+        )
+    return value[:max_length]
diff --git a/tests/test_stripping.py b/tests/test_stripping.py
@@ -0,0 +1,14 @@
+from sentry_sdk.stripping import AnnotatedValue, flatten_metadata, strip_databag
+
+def test_flatten_metadata():
+    assert flatten_metadata({'foo': u'bar'}) == {'foo': u'bar'}
+    assert flatten_metadata({'foo': ['bar']}) == {'foo': [u'bar']}
+    assert flatten_metadata({'foo': [AnnotatedValue('bar', u'meta')]}) == {
+        'foo': [u'bar'],
+        '': {'foo': {0: {'': u'meta'}}}
+    }
+
+
+def test_strip_databag():
+    d = strip_databag({'foo': u'a' * 2000})
+    assert len(d['foo'].value) == 512

Original file line number	Diff line number	Diff line change
`@@ -138,7 +138,7 @@ def capture_exception(self, error=None):`
`138`	`138`	`}`
`139`	`139`	`return self.capture_event(event)`
`140`	`140`	`except Exception:`
`141`		`- capture_internal_exception()`
	`141`	`+ self.capture_internal_exception()`
`142`	`142`
`143`	`143`	`def capture_internal_exception(self, error=None):`
`144`	`144`	`"""Capture an exception that is likely caused by a bug in the SDK`