Merge branch '36412-Sentry-error-page-stuck-loading' into 'master'

Handle empty stacktrace and entries with no code

See merge request gitlab-org/gitlab!20458

app/assets/javascripts/error_tracking/components/stacktrace.vue

@@ -27,6 +27,8 @@ export default {
       :lines="entry.context"
       :file-path="entry.filename"
       :error-line="entry.lineNo"
+      :error-fn="entry.function"
+      :error-column="entry.colNo"
       :expanded="isFirstEntry(index)"
     />
   </div>

app/assets/javascripts/error_tracking/components/stacktrace_entry.vue

 <script>
+import { __, sprintf } from '~/locale';
 import { GlTooltip } from '@gitlab/ui';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import FileIcon from '~/vue_shared/components/file_icon.vue';
@@ -22,9 +23,20 @@ export default {
       type: String,
       required: true,
     },
+    errorFn: {
+      type: String,
+      required: false,
+      default: '',
+    },
     errorLine: {
       type: Number,
-      required: true,
+      required: false,
+      default: 0,
+    },
+    errorColumn: {
+      type: Number,
+      required: false,
+      default: 0,
     },
     expanded: {
       type: Boolean,
@@ -38,12 +50,23 @@ export default {
     };
   },
   computed: {
-    linesLength() {
-      return this.lines.length;
+    hasCode() {
+      return Boolean(this.lines.length);
     },
     collapseIcon() {
       return this.isExpanded ? 'chevron-down' : 'chevron-right';
     },
+    noCodeFn() {
+      return this.errorFn ? sprintf(__('in %{errorFn} '), { errorFn: this.errorFn }) : '';
+    },
+    noCodeLine() {
+      return this.errorLine
+        ? sprintf(__('at line %{errorLine}%{errorColumn}'), {
+            errorLine: this.errorLine,
+            errorColumn: this.errorColumn ? `:${this.errorColumn}` : '',
+          })
+        : '';
+    },
   },
   methods: {
     isHighlighted(lineNum) {
@@ -66,27 +89,31 @@ export default {
 <template>
   <div class="file-holder">
     <div ref="header" class="file-title file-title-flex-parent">
-      <div class="file-header-content ">
-        <div class="d-inline-block cursor-pointer" @click="toggle()">
+      <div class="file-header-content d-flex align-content-center">
+        <div v-if="hasCode" class="d-inline-block cursor-pointer" @click="toggle()">
           <icon :name="collapseIcon" :size="16" aria-hidden="true" class="append-right-5" />
         </div>
-        <div class="d-inline-block append-right-4">
-          <file-icon
-            :file-name="filePath"
-            :size="18"
-            aria-hidden="true"
-            css-classes="append-right-5"
-          />
-          <strong v-gl-tooltip :title="filePath" class="file-title-name" data-container="body">
-            {{ filePath }}
-          </strong>
-        </div>
-
+        <file-icon
+          :file-name="filePath"
+          :size="18"
+          aria-hidden="true"
+          css-classes="append-right-5"
+        />
+        <strong
+          v-gl-tooltip
+          :title="filePath"
+          class="file-title-name d-inline-block overflow-hidden text-truncate"
+          :class="{ 'limited-width': !hasCode }"
+          data-container="body"
+        >
+          {{ filePath }}
+        </strong>
         <clipboard-button
           :title="__('Copy file path')"
           :text="filePath"
-          css-class="btn-default btn-transparent btn-clipboard"
+          css-class="btn-default btn-transparent btn-clipboard position-static"
         />
+        <span v-if="!hasCode" class="text-tertiary">{{ noCodeFn }}{{ noCodeLine }}</span>
       </div>
     </div>
 

app/assets/javascripts/error_tracking/store/details/getters.js

-export const stacktrace = state => state.stacktraceData.stack_trace_entries.reverse();
+export const stacktrace = state =>
+  state.stacktraceData.stack_trace_entries
+    ? state.stacktraceData.stack_trace_entries.reverse()
+    : [];
 
 export default () => {};

app/assets/stylesheets/pages/error_details.scss

@@ -12,6 +12,12 @@
     }
   }
 
+  .file-title-name {
+    &.limited-width {
+      max-width: 80%;
+    }
+  }
+
   .line_content.old::before {
     content: none !important;
   }

changelogs/unreleased/36412-Sentry-error-page-stuck-loading.yml

+---
+title: Handle empty stacktrace and entries with no code
+merge_request: 20458
+author:
+type: fixed

locale/gitlab.pot

@@ -20386,6 +20386,9 @@ msgstr ""
 msgid "assign yourself"
 msgstr ""
 
+msgid "at line %{errorLine}%{errorColumn}"
+msgstr ""
+
 msgid "attach a new file"
 msgstr ""
 
@@ -20866,6 +20869,9 @@ msgstr ""
 msgid "importing"
 msgstr ""
 
+msgid "in %{errorFn} "
+msgstr ""
+
 msgid "in group %{link_to_group}"
 msgstr ""
 

spec/frontend/error_tracking/components/stacktrace_entry_spec.js

@@ -7,26 +7,23 @@ import Icon from '~/vue_shared/components/icon.vue';
 describe('Stacktrace Entry', () => {
   let wrapper;
 
+  const lines = [
+    [22, '    def safe_thread(name, \u0026block)\n'],
+    [23, '      Thread.new do\n'],
+    [24, "        Thread.current['sidekiq_label'] = name\n"],
+    [25, '        watchdog(name, \u0026block)\n'],
+  ];
+
   function mountComponent(props) {
     wrapper = shallowMount(StackTraceEntry, {
       propsData: {
         filePath: 'sidekiq/util.rb',
-        lines: [
-          [22, '    def safe_thread(name, \u0026block)\n'],
-          [23, '      Thread.new do\n'],
-          [24, "        Thread.current['sidekiq_label'] = name\n"],
-          [25, '        watchdog(name, \u0026block)\n'],
-        ],
         errorLine: 24,
         ...props,
       },
     });
   }
 
-  beforeEach(() => {
-    mountComponent();
-  });
-
   afterEach(() => {
     if (wrapper) {
       wrapper.destroy();
@@ -34,16 +31,47 @@ describe('Stacktrace Entry', () => {
   });
 
   it('should render stacktrace entry collapsed', () => {
+    mountComponent({ lines });
     expect(wrapper.find(StackTraceEntry).exists()).toBe(true);
     expect(wrapper.find(ClipboardButton).exists()).toBe(true);
     expect(wrapper.find(Icon).exists()).toBe(true);
     expect(wrapper.find(FileIcon).exists()).toBe(true);
-    expect(wrapper.element.querySelectorAll('table').length).toBe(0);
+    expect(wrapper.find('table').exists()).toBe(false);
   });
 
   it('should render stacktrace entry table expanded', () => {
-    mountComponent({ expanded: true });
-    expect(wrapper.element.querySelectorAll('tr.line_holder').length).toBe(4);
-    expect(wrapper.element.querySelectorAll('.line_content.old').length).toBe(1);
+    mountComponent({ expanded: true, lines });
+    expect(wrapper.find('table').exists()).toBe(true);
+    expect(wrapper.findAll('tr.line_holder').length).toBe(4);
+    expect(wrapper.findAll('.line_content.old').length).toBe(1);
+  });
+
+  describe('no code block', () => {
+    const findFileHeaderContent = () => wrapper.find('.file-header-content').html();
+
+    it('should hide collapse icon and render error fn name and error line when there is no code block', () => {
+      const extraInfo = { errorLine: 34, errorFn: 'errorFn', errorColumn: 77 };
+      mountComponent({ expanded: false, lines: [], ...extraInfo });
+      expect(wrapper.find(Icon).exists()).toBe(false);
+      expect(findFileHeaderContent()).toContain(
+        `in ${extraInfo.errorFn} at line ${extraInfo.errorLine}:${extraInfo.errorColumn}`,
+      );
+    });
+
+    it('should render only lineNo:columnNO when there is no errorFn ', () => {
+      const extraInfo = { errorLine: 34, errorFn: null, errorColumn: 77 };
+      mountComponent({ expanded: false, lines: [], ...extraInfo });
+      expect(findFileHeaderContent()).not.toContain(`in ${extraInfo.errorFn}`);
+      expect(findFileHeaderContent()).toContain(`${extraInfo.errorLine}:${extraInfo.errorColumn}`);
+    });
+
+    it('should render only lineNo when there is no errorColumn ', () => {
+      const extraInfo = { errorLine: 34, errorFn: 'errorFn', errorColumn: null };
+      mountComponent({ expanded: false, lines: [], ...extraInfo });
+      expect(findFileHeaderContent()).toContain(
+        `in ${extraInfo.errorFn} at line ${extraInfo.errorLine}`,
+      );
+      expect(findFileHeaderContent()).not.toContain(`:${extraInfo.errorColumn}`);
+    });
   });
 });

spec/frontend/error_tracking/store/details/getters_spec.js

 import * as getters from '~/error_tracking/store/details/getters';
 
 describe('Sentry error details store getters', () => {
-  const state = {
-    stacktraceData: { stack_trace_entries: [1, 2] },
-  };
-
   describe('stacktrace', () => {
+    it('should return empty stacktrace when there are no entries', () => {
+      const state = {
+        stacktraceData: { stack_trace_entries: null },
+      };
+      expect(getters.stacktrace(state)).toEqual([]);
+    });
+
     it('should get stacktrace', () => {
+      const state = {
+        stacktraceData: { stack_trace_entries: [1, 2] },
+      };
       expect(getters.stacktrace(state)).toEqual([2, 1]);
     });
   });