fix(api): Resolve internal errors in the Minidump endpoint (#14902)

src/sentry/lang/native/minidump.py

@@ -58,12 +58,13 @@ def merge_attached_event(mpack_event, data):
     """
     Merges an event payload attached in the ``__sentry-event`` attachment.
     """
-    if mpack_event.size > MAX_MSGPACK_EVENT_SIZE_BYTES:
+    size = mpack_event.size
+    if size == 0 or size > MAX_MSGPACK_EVENT_SIZE_BYTES:
         return
 
     try:
         event = unpack(mpack_event)
-    except (UnpackException, ExtraData) as e:
+    except (ValueError, UnpackException, ExtraData) as e:
         minidumps_logger.exception(e)
         return
 
@@ -77,13 +78,14 @@ def merge_attached_breadcrumbs(mpack_breadcrumbs, data):
     """
     Merges breadcrumbs attached in the ``__sentry-breadcrumbs`` attachment(s).
     """
-    if mpack_breadcrumbs.size > MAX_MSGPACK_BREADCRUMB_SIZE_BYTES:
+    size = mpack_breadcrumbs.size
+    if size == 0 or size > MAX_MSGPACK_BREADCRUMB_SIZE_BYTES:
         return
 
     try:
         unpacker = Unpacker(mpack_breadcrumbs)
         breadcrumbs = list(unpacker)
-    except (UnpackException, ExtraData) as e:
+    except (ValueError, UnpackException, ExtraData) as e:
         minidumps_logger.exception(e)
         return
 

src/sentry/web/api.py

@@ -753,9 +753,15 @@ class MinidumpView(StoreView):
             else:
                 # Custom clients can submit longer payloads and should JSON
                 # encode event data into the optional `sentry` field.
-                extra = request.POST
+                extra = request.POST.dict()
                 json_data = extra.pop("sentry", None)
-                data = json.loads(json_data[0]) if json_data else {}
+                try:
+                    data = json.loads(json_data) if json_data else {}
+                except ValueError:
+                    data = {}
+
+            if not isinstance(data, dict):
+                data = {}
 
             # Merge additional form fields from the request with `extra` data
             # from the event payload and set defaults for processing. This is

tests/sentry/lang/java/test_plugin.py

@@ -8,6 +8,7 @@ from django.core.files.uploadedfile import SimpleUploadedFile
 
 from sentry import eventstore
 from sentry.testutils import TestCase
+from sentry.utils import json
 
 
 PROGUARD_UUID = "6dc7fdb0-d2fb-4c8e-9d6b-bb1aa98929b1"
@@ -88,7 +89,7 @@ class BasicResolvingIntegrationTest(TestCase):
 
         # We do a preflight post, because there are many queries polluting the array
         # before the actual "processing" happens (like, auth_user)
-        self._postWithHeader(event_data)
+        resp = self._postWithHeader(event_data)
         with self.assertWriteQueries(
             {
                 "nodestore_node": 2,
@@ -102,11 +103,11 @@ class BasicResolvingIntegrationTest(TestCase):
                 "sentry_userreport": 1,
             }
         ):
-            resp = self._postWithHeader(event_data)
+            self._postWithHeader(event_data)
         assert resp.status_code == 200
+        event_id = json.loads(resp.content)["id"]
 
-        event = eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
-
+        event = eventstore.get_event_by_id(self.project.id, event_id)
         bt = event.interfaces["exception"].values[0].stacktrace
         frames = bt.frames
 
@@ -183,8 +184,9 @@ class BasicResolvingIntegrationTest(TestCase):
 
         resp = self._postWithHeader(event_data)
         assert resp.status_code == 200
+        event_id = json.loads(resp.content)["id"]
 
-        event = eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
+        event = eventstore.get_event_by_id(self.project.id, event_id)
 
         assert len(event.data["errors"]) == 1
         assert event.data["errors"][0] == {

tests/sentry/lang/javascript/test_example.py

@@ -63,8 +63,9 @@ class ExampleTestCase(TestCase):
 
         resp = self._postWithHeader(data)
         assert resp.status_code == 200
+        event_id = json.loads(resp.content)["id"]
 
-        event = eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
+        event = eventstore.get_event_by_id(self.project.id, event_id)
 
         exception = event.interfaces["exception"]
         frame_list = exception.values[0].stacktrace.frames

tests/sentry/lang/javascript/test_plugin.py

@@ -10,6 +10,7 @@ from sentry import eventstore
 from sentry.models import File, Release, ReleaseFile
 from sentry.testutils import TestCase, SnubaTestCase
 from sentry.testutils.helpers.datetime import iso_format, before_now
+from sentry.utils import json
 
 
 BASE64_SOURCEMAP = "data:application/json;base64," + (
@@ -35,8 +36,8 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         super(JavascriptIntegrationTest, self).setUp()
         self.min_ago = iso_format(before_now(minutes=1))
 
-    def get_event(self):
-        return eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
+    def get_event(self, event_id):
+        return eventstore.get_event_by_id(self.project.id, event_id)
 
     def test_adds_contexts_without_device(self):
         data = {
@@ -73,7 +74,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
             resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
 
         contexts = event.interfaces["contexts"].to_json()
         assert contexts.get("os") == {"name": "Windows 8", "type": "os"}
@@ -98,7 +99,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         contexts = event.interfaces["contexts"].to_json()
         assert contexts.get("os") == {"name": "Android", "type": "os", "version": "4.3"}
         assert contexts.get("browser") == {"name": "Android", "type": "browser", "version": "4.3"}
@@ -128,7 +129,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         contexts = event.interfaces["contexts"].to_json()
         assert contexts.get("os") is None
         assert contexts.get("browser") is None
@@ -184,7 +185,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
             allow_scraping=True,
         )
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         exception = event.interfaces["exception"]
         frame_list = exception.values[0].stacktrace.frames
 
@@ -244,7 +245,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
             allow_scraping=True,
         )
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         exception = event.interfaces["exception"]
         frame_list = exception.values[0].stacktrace.frames
 
@@ -277,7 +278,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
 
         message = event.interfaces["logentry"]
         assert (
@@ -355,7 +356,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["errors"] == [
             {"type": "js_no_source", "url": "http//example.com/index.html"}
         ]
@@ -434,7 +435,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["errors"] == [
             {"type": "js_no_source", "url": "http//example.com/index.html"}
         ]
@@ -496,7 +497,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert "errors" not in event.data
 
         exception = event.interfaces["exception"]
@@ -573,7 +574,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert "errors" not in event.data
 
         exception = event.interfaces["exception"]
@@ -734,7 +735,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert "errors" not in event.data
 
         exception = event.interfaces["exception"]
@@ -880,7 +881,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert "errors" not in event.data
 
         exception = event.interfaces["exception"]
@@ -956,7 +957,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["errors"] == [
             {"url": u"http://example.com/file1.js", "type": "fetch_invalid_http_code", "value": 404}
         ]
@@ -1021,7 +1022,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["errors"] == [
             {"url": u"http://example.com/unsupported.sourcemap.js", "type": "js_invalid_source"}
         ]
@@ -1053,7 +1054,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["errors"] == [{"url": u"<data url>", "type": "js_no_source"}]
 
     @responses.activate
@@ -1094,7 +1095,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code == 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert "errors" not in event.data
 
     @responses.activate
@@ -1155,7 +1156,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["errors"] == [
             {"url": u"http://example.com/file1.js", "type": "js_invalid_content"},
             {"url": u"http://example.com/file2.js", "type": "js_invalid_content"},
@@ -1251,7 +1252,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
 
         exception = event.interfaces["exception"]
         frame_list = exception.values[0].stacktrace.frames
@@ -1362,7 +1363,7 @@ class JavascriptIntegrationTest(TestCase, SnubaTestCase):
         resp = self._postWithHeader(data)
         assert resp.status_code, 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         exception = event.interfaces["exception"]
         frame_list = exception.values[0].stacktrace.frames
 

tests/sentry/lang/native/test_minidump.py

@@ -38,6 +38,18 @@ def test_merge_attached_event_arbitrary_key():
     assert event["key"] == "value"
 
 
+def test_merge_attached_event_empty_file():
+    event = {}
+    merge_attached_event(MockFile(b""), event)
+    assert not event
+
+
+def test_merge_attached_event_invalid_file():
+    event = {}
+    merge_attached_event(MockFile(b"\xde"), event)
+    assert not event
+
+
 def test_merge_attached_breadcrumbs_empty_creates_crumb():
     mpack_crumb = msgpack.packb({})
     event = {}
@@ -52,6 +64,18 @@ def test_merge_attached_breadcrumb_too_large_empty():
     assert not event.get("breadcrumbs")
 
 
+def test_merge_attached_breadcrumbs_empty_file():
+    event = {}
+    merge_attached_breadcrumbs(MockFile(b""), event)
+    assert not event.get("breadcrumbs")
+
+
+def test_merge_attached_breadcrumbs_invalid_file():
+    event = {}
+    merge_attached_breadcrumbs(MockFile(b"\xde"), event)
+    assert not event.get("breadcrumbs")
+
+
 # See:
 # https://github.com/getsentry/sentrypad/blob/e1d4feb65c3e9db829cc4ca9d4003ff3c818d95a/src/sentry.cpp#L337-L366
 def test_merge_attached_breadcrumbs_single_crumb():

tests/symbolicator/test_minidump_full.py

@@ -72,9 +72,12 @@ class SymbolicatorMinidumpIntegrationTest(TransactionTestCase):
                     f, {"sentry[logger]": "test-logger", "some_file": attachment}
                 )
                 assert resp.status_code == 200
+                event_id = resp.content
 
-        event = eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
+        event = eventstore.get_event_by_id(self.project.id, event_id)
         insta_snapshot_stacktrace_data(self, event.data)
+        assert event.data.get("logger") == "test-logger"
+        # assert event.data.get("extra") == {"foo": "bar"}
 
         attachments = sorted(
             EventAttachment.objects.filter(event_id=event.event_id), key=lambda x: x.name
@@ -89,6 +92,40 @@ class SymbolicatorMinidumpIntegrationTest(TransactionTestCase):
         assert minidump.file.type == "event.minidump"
         assert minidump.file.checksum == "74bb01c850e8d65d3ffbc5bad5cabc4668fce247"
 
+    def test_full_minidump_json_extra(self):
+        self.project.update_option("sentry:store_crash_reports", True)
+        self.upload_symbols()
+
+        with self.feature("organizations:event-attachments"):
+            with open(get_fixture_path("windows.dmp"), "rb") as f:
+                resp = self._postMinidumpWithHeader(
+                    f, {"sentry": '{"logger":"test-logger"}', "foo": "bar"}
+                )
+                assert resp.status_code == 200
+                event_id = resp.content
+
+        event = eventstore.get_event_by_id(self.project.id, event_id)
+        assert event.data.get("logger") == "test-logger"
+        assert event.data.get("extra") == {"foo": "bar"}
+        # Other assertions are performed by `test_full_minidump`
+
+    def test_full_minidump_invalid_extra(self):
+        self.project.update_option("sentry:store_crash_reports", True)
+        self.upload_symbols()
+
+        with self.feature("organizations:event-attachments"):
+            with open(get_fixture_path("windows.dmp"), "rb") as f:
+                resp = self._postMinidumpWithHeader(
+                    f, {"sentry": "{{{{", "foo": "bar"}  # invalid sentry JSON
+                )
+                assert resp.status_code == 200
+                event_id = resp.content
+
+        event = eventstore.get_event_by_id(self.project.id, event_id)
+        assert not event.data.get("logger")
+        assert event.data.get("extra") == {"foo": "bar"}
+        # Other assertions are performed by `test_full_minidump`
+
     def test_raw_minidump(self):
         self.project.update_option("sentry:store_crash_reports", True)
         self.upload_symbols()
@@ -98,8 +135,9 @@ class SymbolicatorMinidumpIntegrationTest(TransactionTestCase):
                 # Send as raw request body instead of multipart/form-data
                 resp = self._postMinidumpWithHeader(f, raw=True)
                 assert resp.status_code == 200
+                event_id = resp.content
 
-        event = eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
+        event = eventstore.get_event_by_id(self.project.id, event_id)
         insta_snapshot_stacktrace_data(self, event.data)
 
     def test_missing_dsym(self):
@@ -107,7 +145,8 @@ class SymbolicatorMinidumpIntegrationTest(TransactionTestCase):
             with open(get_fixture_path("windows.dmp"), "rb") as f:
                 resp = self._postMinidumpWithHeader(f, {"sentry[logger]": "test-logger"})
                 assert resp.status_code == 200
+                event_id = resp.content
 
-        event = eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
+        event = eventstore.get_event_by_id(self.project.id, event_id)
         insta_snapshot_stacktrace_data(self, event.data)
         assert not EventAttachment.objects.filter(event_id=event.event_id)

tests/symbolicator/test_payload_full.py

@@ -9,10 +9,11 @@ from six import BytesIO
 from django.core.urlresolvers import reverse
 from django.core.files.uploadedfile import SimpleUploadedFile
 
+from sentry import eventstore
 from sentry.testutils import TransactionTestCase
 from sentry.models import File, ProjectDebugFile
-from sentry import eventstore
 from sentry.testutils.helpers.datetime import iso_format, before_now
+from sentry.utils import json
 
 from tests.symbolicator import get_fixture_path, insta_snapshot_stacktrace_data
 
@@ -57,8 +58,8 @@ REAL_RESOLVING_EVENT_DATA = {
 
 
 class ResolvingIntegrationTestBase(object):
-    def get_event(self):
-        return eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
+    def get_event(self, event_id):
+        return eventstore.get_event_by_id(self.project.id, event_id)
 
     def test_real_resolving(self):
         url = reverse(
@@ -91,7 +92,7 @@ class ResolvingIntegrationTestBase(object):
         resp = self._postWithHeader(dict(project=self.project.id, **REAL_RESOLVING_EVENT_DATA))
         assert resp.status_code == 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
 
         assert event.data["culprit"] == "main"
         insta_snapshot_stacktrace_data(self, event.data)
@@ -153,7 +154,7 @@ class ResolvingIntegrationTestBase(object):
         resp = self._postWithHeader(event_data)
         assert resp.status_code == 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["culprit"] == "main"
         insta_snapshot_stacktrace_data(self, event.data)
 
@@ -163,7 +164,7 @@ class ResolvingIntegrationTestBase(object):
         resp = self._postWithHeader(dict(project=self.project.id, **REAL_RESOLVING_EVENT_DATA))
         assert resp.status_code == 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["culprit"] == "unknown"
         insta_snapshot_stacktrace_data(self, event.data)
 
@@ -176,7 +177,7 @@ class ResolvingIntegrationTestBase(object):
         resp = self._postWithHeader(payload)
         assert resp.status_code == 200
 
-        event = self.get_event()
+        event = self.get_event(json.loads(resp.content)["id"])
         assert event.data["culprit"] == "unknown"
         insta_snapshot_stacktrace_data(self, event.data)
 

tests/symbolicator/test_unreal_full.py

@@ -77,8 +77,9 @@ class SymbolicatorUnrealIntegrationTest(TransactionTestCase):
             with open(filename, "rb") as f:
                 resp = self._postUnrealWithHeader(f.read())
                 assert resp.status_code == 200
+                event_id = resp.content
 
-        event = eventstore.get_events(filter_keys={"project_id": [self.project.id]})[0]
+        event = eventstore.get_event_by_id(self.project.id, event_id)
 
         self.insta_snapshot(
             {