Browse Source

fix(stacktrace): Improve platform icon logic for mixed-platform stack traces (#61551)

Fixes https://github.com/getsentry/sentry/issues/53132

Previously, we looked to see if all file extensions in the stack trace
were the same. If so, we returned the icon for that file extension.
Otherwise, we returned the event platform. For platforms like react
native, this did not work very well.

This modifies the logic to look at the most recent in-app frames and
return the first match we find. It also takes into account
`frame.platform` which was not used before.
Malachi Willey 1 year ago
parent
commit
dd5bd59fa1

+ 75 - 0
fixtures/js-stubs/eventStacktraceFrame.ts

@@ -0,0 +1,75 @@
+import {Frame} from 'sentry/types';
+
+export function EventStacktraceFrame(params = {}): Frame {
+  return {
+    filename: 'raven/base.py',
+    absPath: '/home/ubuntu/.virtualenvs/getsentry/src/raven/raven/base.py',
+    module: 'raven.base',
+    package: null,
+    platform: null,
+    instructionAddr: null,
+    symbolAddr: null,
+    function: 'build_msg',
+    rawFunction: null,
+    symbol: null,
+    context: [
+      [298, '                frames = stack'],
+      [299, ''],
+      [300, '            data.update({'],
+      [301, "                'sentry.interfaces.Stacktrace': {"],
+      [302, "                    'frames': get_stack_info(frames,"],
+      [303, '                        transformer=self.transform)'],
+      [304, '                },'],
+      [305, '            })'],
+      [306, ''],
+      [307, "        if 'sentry.interfaces.Stacktrace' in data:"],
+      [308, '            if self.include_paths:'],
+    ],
+    lineNo: 303,
+    colNo: null,
+    inApp: true,
+    trust: null,
+    vars: {
+      "'culprit'": null,
+      "'data'": {
+        "'message'": "u'This is a test message generated using ``raven test``'",
+        "'sentry.interfaces.Message'": {
+          "'message'": "u'This is a test message generated using ``raven test``'",
+          "'params'": [],
+        },
+      },
+      "'date'": 'datetime.datetime(2013, 8, 13, 3, 8, 24, 880386)',
+      "'event_id'": "'54a322436e1b47b88e239b78998ae742'",
+      "'event_type'": "'raven.events.Message'",
+      "'extra'": {
+        "'go_deeper'": [['{"\'bar\'":["\'baz\'"],"\'foo\'":"\'bar\'"}']],
+        "'loadavg'": [0.37255859375, 0.5341796875, 0.62939453125],
+        "'user'": "'dcramer'",
+      },
+      "'frames'": '<generator object iter_stack_frames at 0x107bcc3c0>',
+      "'handler'": '<raven.events.Message object at 0x107bd0890>',
+      "'k'": "'sentry.interfaces.Message'",
+      "'kwargs'": {
+        "'level'": 20,
+        "'message'": "'This is a test message generated using ``raven test``'",
+      },
+      "'public_key'": null,
+      "'result'": {
+        "'message'": "u'This is a test message generated using ``raven test``'",
+        "'sentry.interfaces.Message'": {
+          "'message'": "u'This is a test message generated using ``raven test``'",
+          "'params'": [],
+        },
+      },
+      "'self'": '<raven.base.Client object at 0x107bb8210>',
+      "'stack'": true,
+      "'tags'": null,
+      "'time_spent'": null,
+      "'v'": {
+        "'message'": "u'This is a test message generated using ``raven test``'",
+        "'params'": [],
+      },
+    },
+    ...params,
+  };
+}

+ 79 - 0
static/app/components/events/interfaces/crashContent/stackTrace/content.spec.tsx

@@ -1,5 +1,6 @@
 import {Event as EventFixture} from 'sentry-fixture/event';
 import {EventEntryStacktrace} from 'sentry-fixture/eventEntryStacktrace';
+import {EventStacktraceFrame} from 'sentry-fixture/eventStacktraceFrame';
 
 import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary';
 
@@ -417,4 +418,82 @@ describe('StackTrace', function () {
       expect(frameTitles[1]).toHaveTextContent('raven/base.py in build_msg at line 303');
     });
   });
+
+  describe('platform icons', function () {
+    it('uses the top in-app frame file extension for mixed stack trace platforms', function () {
+      renderedComponent({
+        data: {
+          ...data,
+          frames: [
+            EventStacktraceFrame({
+              inApp: true,
+              filename: 'foo.cs',
+            }),
+            EventStacktraceFrame({
+              inApp: true,
+              filename: 'foo.py',
+            }),
+            EventStacktraceFrame({
+              inApp: true,
+              filename: 'foo',
+            }),
+            EventStacktraceFrame({
+              inApp: false,
+              filename: 'foo.rb',
+            }),
+          ],
+        },
+      });
+
+      // foo.py is the most recent in-app frame with a valid file extension
+      expect(screen.getByTestId('platform-icon-python')).toBeInTheDocument();
+    });
+
+    it('uses frame.platform if file extension does not work', function () {
+      renderedComponent({
+        data: {
+          ...data,
+          frames: [
+            EventStacktraceFrame({
+              inApp: true,
+              filename: 'foo.cs',
+            }),
+            EventStacktraceFrame({
+              inApp: true,
+              filename: 'foo',
+              platform: 'node',
+            }),
+            EventStacktraceFrame({
+              inApp: true,
+              filename: 'foo',
+            }),
+            EventStacktraceFrame({
+              inApp: false,
+              filename: 'foo.rb',
+            }),
+          ],
+        },
+      });
+
+      expect(screen.getByTestId('platform-icon-node')).toBeInTheDocument();
+    });
+
+    it('falls back to the event platform if there is no other information', function () {
+      renderedComponent({
+        data: {
+          ...data,
+          frames: [
+            EventStacktraceFrame({
+              inApp: true,
+              filename: 'foo',
+              platform: null,
+            }),
+          ],
+        },
+        platform: 'python',
+      });
+
+      expect(screen.getByTestId('platform-icon-python')).toBeInTheDocument();
+    });
+  });
 });

+ 38 - 9
static/app/components/events/interfaces/utils.tsx

@@ -1,7 +1,6 @@
 import * as Sentry from '@sentry/react';
-import compact from 'lodash/compact';
 import isString from 'lodash/isString';
-import uniq from 'lodash/uniq';
+import partition from 'lodash/partition';
 import * as qs from 'query-string';
 
 import getThreadException from 'sentry/components/events/interfaces/threads/threadSelector/getThreadException';
@@ -319,18 +318,48 @@ export function parseAssembly(assembly: string | null) {
   return {name, version, culture, publicKeyToken};
 }
 
-export function stackTracePlatformIcon(platform: PlatformKey, frames: Frame[]) {
-  const fileExtensions = uniq(
-    compact(frames.map(frame => getFileExtension(frame.filename ?? '')))
+function getFramePlatform(frame: Frame) {
+  const fileExtension = getFileExtension(frame.filename ?? '');
+  const fileExtensionPlatform = fileExtension
+    ? fileExtensionToPlatform(fileExtension)
+    : null;
+
+  if (fileExtensionPlatform) {
+    return fileExtensionPlatform;
+  }
+
+  if (frame.platform) {
+    return frame.platform;
+  }
+
+  return null;
+}
+
+/**
+ * Returns the representative platform for the given stack trace frames.
+ * Prioritizes recent in-app frames, checking first for a matching file extension
+ * and then for a frame.platform attribute [1].
+ *
+ * If none of the frames have a platform, falls back to the event platform.
+ *
+ * [1] https://develop.sentry.dev/sdk/event-payloads/stacktrace/#frame-attributes
+ */
+export function stackTracePlatformIcon(eventPlatform: PlatformKey, frames: Frame[]) {
+  const [inAppFrames, systemFrames] = partition(
+    // Reverse frames to get newest-first ordering
+    [...frames].reverse(),
+    frame => frame.inApp
   );
 
-  if (fileExtensions.length === 1) {
-    const newPlatform = fileExtensionToPlatform(fileExtensions[0]);
+  for (const frame of [...inAppFrames, ...systemFrames]) {
+    const framePlatform = getFramePlatform(frame);
 
-    return newPlatform ?? platform;
+    if (framePlatform) {
+      return framePlatform;
+    }
   }
 
-  return platform;
+  return eventPlatform;
 }
 
 export function isStacktraceNewestFirst() {