Extract stacktrace builder into own utility class

This utility can be used in different places and tested
individually.

app/models/error_tracking/error_event.rb

@@ -15,7 +15,7 @@ class ErrorTracking::ErrorEvent < ApplicationRecord
   validates :occurred_at, presence: true
 
   def stacktrace
-    @stacktrace ||= build_stacktrace
+    @stacktrace ||= ErrorTracking::StacktraceBuilder.new(payload).stacktrace
   end
 
   # For compatibility with sentry integration
@@ -30,56 +30,4 @@ def to_sentry_error_event
   def release
     payload.dig('release')
   end
-
-  private
-
-  def build_stacktrace
-    raw_stacktrace = find_stacktrace_from_payload
-
-    return [] unless raw_stacktrace
-
-    raw_stacktrace.map do |entry|
-      {
-        'lineNo' => entry['lineno'],
-        'context' => build_stacktrace_context(entry),
-        'filename' => entry['filename'],
-        'function' => entry['function'],
-        'colNo' => 0 # we don't support colNo yet.
-      }
-    end
-  end
-
-  def find_stacktrace_from_payload
-    exception_entry = payload.dig('exception')
-
-    if exception_entry
-      exception_values = exception_entry.dig('values')
-      stack_trace_entry = exception_values&.detect { |h| h['stacktrace'].present? }
-      stack_trace_entry&.dig('stacktrace', 'frames')
-    end
-  end
-
-  def build_stacktrace_context(entry)
-    context = []
-    error_line = entry['context_line']
-    error_line_no = entry['lineno']
-    pre_context = entry['pre_context']
-    post_context = entry['post_context']
-
-    context += lines_with_position(pre_context, error_line_no - pre_context.size) if pre_context
-    context += lines_with_position([error_line], error_line_no)
-    context += lines_with_position(post_context, error_line_no + 1) if post_context
-
-    context.reject(&:blank?)
-  end
-
-  def lines_with_position(lines, position)
-    return [] if lines.blank?
-
-    lines.map.with_index do |line, index|
-      next unless line
-
-      [position + index, line]
-    end
-  end
 end

lib/error_tracking/stacktrace_builder.rb

+# frozen_string_literal: true
+
+module ErrorTracking
+  class StacktraceBuilder
+    attr_reader :stacktrace
+
+    def initialize(payload)
+      @stacktrace = build_stacktrace(payload)
+    end
+
+    private
+
+    def build_stacktrace(payload)
+      raw_stacktrace = raw_stacktrace_from_payload(payload)
+      return [] unless raw_stacktrace
+
+      raw_stacktrace.map do |entry|
+        {
+          'lineNo' => entry['lineno'],
+          'context' => build_stacktrace_context(entry),
+          'filename' => entry['filename'],
+          'function' => entry['function'],
+          'colNo' => 0 # we don't support colNo yet.
+        }
+      end
+    end
+
+    def raw_stacktrace_from_payload(payload)
+      exception_entry = payload['exception']
+      return unless exception_entry
+
+      exception_values = exception_entry['values']
+      stack_trace_entry = exception_values&.detect { |h| h['stacktrace'].present? }
+      stack_trace_entry&.dig('stacktrace', 'frames')
+    end
+
+    def build_stacktrace_context(entry)
+      error_line = entry['context_line']
+      error_line_no = entry['lineno']
+      pre_context = entry['pre_context']
+      post_context = entry['post_context']
+
+      context = []
+      context.concat lines_with_position(pre_context, error_line_no - pre_context.size) if pre_context
+      context.concat lines_with_position([error_line], error_line_no)
+      context.concat lines_with_position(post_context, error_line_no + 1) if post_context
+
+      context.reject(&:blank?)
+    end
+
+    def lines_with_position(lines, position)
+      return [] if lines.blank?
+
+      lines.map.with_index do |line, index|
+        next unless line
+
+        [position + index, line]
+      end
+    end
+  end
+end

spec/lib/error_tracking/stacktrace_builder_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'support/helpers/fast_rails_root'
+require 'oj'
+
+RSpec.describe ErrorTracking::StacktraceBuilder do
+  include FastRailsRoot
+
+  describe '#stacktrace' do
+    let(:original_payload) { Gitlab::Json.parse(File.read(rails_root_join('spec/fixtures', payload_file))) }
+    let(:payload) { original_payload }
+    let(:payload_file) { 'error_tracking/parsed_event.json' }
+
+    subject(:stacktrace) { described_class.new(payload).stacktrace }
+
+    context 'with full error context' do
+      it 'generates a correct stacktrace in expected format' do
+        expected_context = [
+          [132, "          end\n"],
+          [133, "\n"],
+          [134, "          begin\n"],
+          [135, "            block.call(work, *extra)\n"],
+          [136, "          rescue Exception => e\n"],
+          [137, "            STDERR.puts \"Error reached top of thread-pool: #\{e.message\} (#\{e.class\})\"\n"],
+          [138, "          end\n"]
+        ]
+
+        expected_entry = {
+          'lineNo' => 135,
+          'context' => expected_context,
+          'filename' => 'puma/thread_pool.rb',
+          'function' => 'block in spawn_thread',
+          'colNo' => 0
+        }
+
+        expect(stacktrace).to be_kind_of(Array)
+        expect(stacktrace.first).to eq(expected_entry)
+      end
+    end
+
+    context 'when error context is missing' do
+      let(:payload_file) { 'error_tracking/browser_event.json' }
+
+      it 'generates a stacktrace without context' do
+        expected_entry = {
+          'lineNo' => 6395,
+          'context' => [],
+          'filename' => 'webpack-internal:///./node_modules/vue/dist/vue.runtime.esm.js',
+          'function' => 'hydrate',
+          'colNo' => 0
+        }
+
+        expect(stacktrace).to be_kind_of(Array)
+        expect(stacktrace.first).to eq(expected_entry)
+      end
+    end
+
+    context 'with empty payload' do
+      let(:payload) { {} }
+
+      it { is_expected.to eq([]) }
+    end
+
+    context 'without exception field' do
+      let(:payload) { original_payload.except('exception') }
+
+      it { is_expected.to eq([]) }
+    end
+
+    context 'without exception.values field' do
+      before do
+        original_payload['exception'].delete('values')
+      end
+
+      it { is_expected.to eq([]) }
+    end
+
+    context 'without any exception.values[].stacktrace fields' do
+      before do
+        original_payload.dig('exception', 'values').each { |value| value['stacktrace'] = '' }
+      end
+
+      it { is_expected.to eq([]) }
+    end
+
+    context 'without any exception.values[].stacktrace.frame fields' do
+      before do
+        original_payload.dig('exception', 'values').each { |value| value['stacktrace'].delete('frames') }
+      end
+
+      it { is_expected.to eq([]) }
+    end
+  end
+end

spec/models/error_tracking/error_event_spec.rb

@@ -2,7 +2,9 @@
 
 require 'spec_helper'
 
-RSpec.describe ErrorTracking::ErrorEvent, type: :model do
+RSpec.describe ErrorTracking::ErrorEvent do
+  include AfterNextHelpers
+
   let_it_be(:event) { create(:error_tracking_error_event) }
 
   describe 'relationships' do
@@ -18,44 +20,12 @@
   end
 
   describe '#stacktrace' do
-    it 'generates a correct stacktrace in expected format' do
-      expected_context = [
-        [132, "          end\n"],
-        [133, "\n"],
-        [134, "          begin\n"],
-        [135, "            block.call(work, *extra)\n"],
-        [136, "          rescue Exception => e\n"],
-        [137, "            STDERR.puts \"Error reached top of thread-pool: #\{e.message\} (#\{e.class\})\"\n"],
-        [138, "          end\n"]
-      ]
-
-      expected_entry = {
-        'lineNo' => 135,
-        'context' => expected_context,
-        'filename' => 'puma/thread_pool.rb',
-        'function' => 'block in spawn_thread',
-        'colNo' => 0
-      }
+    it 'builds a stacktrace' do
+      expect_next(ErrorTracking::StacktraceBuilder, event.payload)
+        .to receive(:stacktrace).and_call_original
 
       expect(event.stacktrace).to be_kind_of(Array)
-      expect(event.stacktrace.first).to eq(expected_entry)
-    end
-
-    context 'error context is missing' do
-      let(:event) { create(:error_tracking_error_event, :browser) }
-
-      it 'generates a stacktrace without context' do
-        expected_entry = {
-          'lineNo' => 6395,
-          'context' => [],
-          'filename' => 'webpack-internal:///./node_modules/vue/dist/vue.runtime.esm.js',
-          'function' => 'hydrate',
-          'colNo' => 0
-        }
-
-        expect(event.stacktrace).to be_kind_of(Array)
-        expect(event.stacktrace.first).to eq(expected_entry)
-      end
+      expect(event.stacktrace).not_to be_empty
     end
   end