From d17f244d59a40fb6c14e363966043e1b1706a287 Mon Sep 17 00:00:00 2001 From: Jon de Andres Date: Fri, 9 Sep 2016 16:11:55 +0200 Subject: [PATCH] Send code and context frame data Users can decide to send the `code` and `context` data for the sent exception frames, so they can easier see in the Rollbar site what happened and where. This feature is configurable using `config.send_extra_frame_data` which can have three values: `:none` (defualt), `:app` and `:all`. Using `:app`, only the `code` and `context` for the project code frames will be sent, and all of them if used `all`. Example: ```ruby Rollbar.configure do |config| # ... config.send_extra_frame_data = :app end ``` --- README.md | 13 ++ lib/rollbar/configuration.rb | 14 ++ lib/rollbar/item.rb | 4 +- lib/rollbar/item/backtrace.rb | 43 +++-- lib/rollbar/item/frame.rb | 112 ++++++++++++ spec/rollbar/item/backtrace_spec.rb | 26 +++ spec/rollbar/item/frame_spec.rb | 267 ++++++++++++++++++++++++++++ 7 files changed, 460 insertions(+), 19 deletions(-) create mode 100644 lib/rollbar/item/frame.rb create mode 100644 spec/rollbar/item/backtrace_spec.rb create mode 100644 spec/rollbar/item/frame_spec.rb diff --git a/README.md b/README.md index cc8bc37e..57e27412 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ This is the Ruby library for Rollbar. It will instrument many kinds of Ruby appl - [Before process hook](#before-process-hook) - [Transform hook](#transform-hook) - [The Scope](#the-scope) +- [Code and context](#code-and-context) - [Silencing exceptions at runtime](#silencing-exceptions-at-runtime) - [Sending backtrace without rescued exceptions](#sending-backtrace-without-rescued-exceptions) - [ActiveJob integration](#activejob-integration) @@ -607,6 +608,18 @@ your_handler = proc do |options| end ``` +## Code and context + +By default we send the next values for each backtrace frame: `filename`, `lineno` and `method`. You can configure to send the `code` and `context` data, which is extra information for the frames. The `code` is the text for the line of code where the error happend. The `context` is the text for the 4 lines before the problematic line and the 4 lines after it. + +Since the backtrace can be really long sometimes and this extra information may be only useful for your own project files, you can configure to send this data for all the frames or only your project files related frames. There are three levels: `:none` (default)`, `:app` (only your project files) and `all`. Example: + +```ruby +Rollbar.configure do |config| + config.send_extra_frame_data = :app +end +``` + ## Silencing exceptions at runtime If you just want to disable exception reporting for a single block, use ```Rollbar.silenced```: diff --git a/lib/rollbar/configuration.rb b/lib/rollbar/configuration.rb index cc20aed8..073add6e 100644 --- a/lib/rollbar/configuration.rb +++ b/lib/rollbar/configuration.rb @@ -2,6 +2,7 @@ module Rollbar class Configuration + SEND_EXTRA_FRAME_DATA_OPTIONS = [:none, :app, :all].freeze attr_accessor :access_token attr_accessor :async_handler @@ -52,6 +53,7 @@ class Configuration attr_accessor :use_eventmachine attr_accessor :web_base attr_accessor :write_to_file + attr_reader :send_extra_frame_data attr_reader :project_gem_paths @@ -111,6 +113,8 @@ def initialize @verify_ssl_peer = true @web_base = DEFAULT_WEB_BASE @write_to_file = false + @send_extra_frame_data = :none + @project_gem_paths = [] end def initialize_copy(orig) @@ -192,6 +196,16 @@ def transform=(*handler) @transform = Array(handler) end + def send_extra_frame_data=(value) + unless SEND_EXTRA_FRAME_DATA_OPTIONS.include?(value) + logger.warning("Wrong 'send_extra_frame_data' value, :none, :app or :full is expected") + + return + end + + @send_extra_frame_data = value + end + # allow params to be read like a hash def [](option) send(option) diff --git a/lib/rollbar/item.rb b/lib/rollbar/item.rb index 21d026a9..4b41cc53 100644 --- a/lib/rollbar/item.rb +++ b/lib/rollbar/item.rb @@ -82,7 +82,7 @@ def build_data }, :body => build_body } - data[:project_package_paths] = configuration.project_gem_paths if configuration.project_gem_paths + data[:project_package_paths] = configuration.project_gem_paths if configuration.project_gem_paths.any? data[:code_version] = configuration.code_version if configuration.code_version data[:uuid] = SecureRandom.uuid if defined?(SecureRandom) && SecureRandom.respond_to?(:uuid) @@ -148,7 +148,7 @@ def build_backtrace_body :configuration => configuration ) - backtrace.build + backtrace.to_h end def build_extra diff --git a/lib/rollbar/item/backtrace.rb b/lib/rollbar/item/backtrace.rb index f5833a05..ac9acdd1 100644 --- a/lib/rollbar/item/backtrace.rb +++ b/lib/rollbar/item/backtrace.rb @@ -1,3 +1,5 @@ +require 'rollbar/item/frame' + module Rollbar class Item class Backtrace @@ -5,15 +7,19 @@ class Backtrace attr_reader :message attr_reader :extra attr_reader :configuration + attr_reader :files + + private :files def initialize(exception, options = {}) @exception = exception @message = options[:message] @extra = options[:extra] @configuration = options[:configuration] + @files = {} end - def build + def to_h traces = trace_chain traces[0][:exception][:description] = message if message @@ -26,10 +32,23 @@ def build end end + alias_method :build, :to_h + + def get_file_lines(filename) + files[filename] ||= read_file(filename) + end + private + def read_file(filename) + return unless File.exist?(filename) + + File.read(filename).split("\n") + rescue + nil + end + def trace_chain - exception traces = [trace_data(exception)] visited = [exception] @@ -45,12 +64,8 @@ def trace_chain end def trace_data(current_exception) - frames = reduce_frames(current_exception) - # reverse so that the order is as rollbar expects - frames.reverse! - { - :frames => frames, + :frames => map_frames(current_exception), :exception => { :class => current_exception.class.name, :message => current_exception.message @@ -58,16 +73,10 @@ def trace_data(current_exception) } end - def reduce_frames(current_exception) - exception_backtrace(current_exception).map do |frame| - # parse the line - match = frame.match(/(.*):(\d+)(?::in `([^']+)')?/) - - if match - { :filename => match[1], :lineno => match[2].to_i, :method => match[3] } - else - { :filename => '', :lineno => 0, :method => frame } - end + def map_frames(current_exception) + exception_backtrace(current_exception).reverse.map do |frame| + Rollbar::Item::Frame.new(self, frame, + :configuration => configuration).to_h end end diff --git a/lib/rollbar/item/frame.rb b/lib/rollbar/item/frame.rb new file mode 100644 index 00000000..d5ff95c2 --- /dev/null +++ b/lib/rollbar/item/frame.rb @@ -0,0 +1,112 @@ +# We want to use Gem.path +require 'rubygems' + +module Rollbar + class Item + # Representation of the trace data per frame in the payload + class Frame + attr_reader :backtrace + attr_reader :frame + attr_reader :configuration + + MAX_CONTEXT_LENGTH = 4 + + def initialize(backtrace, frame, options = {}) + @backtrace = backtrace + @frame = frame + @configuration = options[:configuration] + end + + def to_h + # parse the line + match = frame.match(/(.*):(\d+)(?::in `([^']+)')?/) + + return unknown_frame unless match + + filename = match[1] + lineno = match[2].to_i + frame_data = { + :filename => filename, + :lineno => lineno, + :method => match[3] + } + + frame_data.merge(extra_frame_data(filename, lineno)) + end + + private + + def unknown_frame + { :filename => '', :lineno => 0, :method => frame } + end + + def extra_frame_data(filename, lineno) + file_lines = backtrace.get_file_lines(filename) + + return {} if skip_extra_frame_data?(filename, file_lines) + + { + :code => code_data(file_lines, lineno), + :context => context_data(file_lines, lineno) + } + end + + def skip_extra_frame_data?(filename, file_lines) + config = configuration.send_extra_frame_data + missing_file_lines = !file_lines || file_lines.empty? + + return false if !missing_file_lines && config == :all + + missing_file_lines || + config == :none || + config == :app && outside_project?(filename) + end + + def outside_project?(filename) + project_gem_paths = configuration.project_gem_paths + inside_project_gem_paths = project_gem_paths.any? do |path| + filename.start_with?(path) + end + + # The file is inside the configuration.project_gem_paths, + return false if inside_project_gem_paths + + root = configuration.root + inside_root = root && filename.start_with?(root.to_s) + + # The file is outside the configuration.root + return true unless inside_root + + # At this point, the file is inside the configuration.root. + # Since it's common to have gems installed in {root}/vendor/bundle, + # let's check it's in any of the Gem.path paths + Gem.path.any? { |path| filename.start_with?(path) } + end + + def code_data(file_lines, lineno) + file_lines[lineno - 1] + end + + def context_data(file_lines, lineno) + { + :pre => pre_data(file_lines, lineno), + :post => post_data(file_lines, lineno) + } + end + + def post_data(file_lines, lineno) + from_line = lineno + number_of_lines = [from_line + MAX_CONTEXT_LENGTH, file_lines.size].min - from_line + + file_lines[from_line, number_of_lines] + end + + def pre_data(file_lines, lineno) + to_line = lineno - 2 + from_line = [to_line - MAX_CONTEXT_LENGTH + 1, 0].max + + file_lines[from_line, (to_line - from_line + 1)].select(&:present?) + end + end + end +end diff --git a/spec/rollbar/item/backtrace_spec.rb b/spec/rollbar/item/backtrace_spec.rb new file mode 100644 index 00000000..5c167142 --- /dev/null +++ b/spec/rollbar/item/backtrace_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' +require 'tempfile' +require 'rollbar/item/backtrace' + +describe Rollbar::Item::Backtrace do + describe '#get_file_lines' do + subject { described_class.new(exception) } + + let(:exception) { Exception.new } + let(:file) { Tempfile.new('foo') } + + before do + File.open(file.path, 'w') do |f| + f << "foo\nbar" + end + end + + it 'returns the lines of the file' do + lines = subject.get_file_lines(file.path) + + expect(lines.size).to be_eql(2) + expect(lines[0]).to be_eql('foo') + expect(lines[1]).to be_eql('bar') + end + end +end diff --git a/spec/rollbar/item/frame_spec.rb b/spec/rollbar/item/frame_spec.rb new file mode 100644 index 00000000..47b0c6cb --- /dev/null +++ b/spec/rollbar/item/frame_spec.rb @@ -0,0 +1,267 @@ +require 'spec_helper' +require 'tempfile' +require 'rollbar/item/backtrace' +require 'rollbar/item/frame' + +describe Rollbar::Item::Frame do + subject { described_class.new(backtrace, frame, options) } + + let(:backtrace) { double('backtrace') } + let(:options) { {} } + + describe '#to_h' do + context 'with a frame that is not a valid frame' do + let(:frame) { 'this frame is not valid' } + + it 'return an unknown frame value' do + expected_result = { + :filename => '', + :lineno => 0, + :method => frame + } + + result = subject.to_h + expect(result).to be_eql(expected_result) + end + end + + context 'with valid frame' do + let(:file) do + <<-END +foo1 +foo2 +foo3 +foo4 +foo5 +foo6 +foo7 +foo8 +foo9 +foo10 +foo11 +foo12 +foo13 + END + end + let(:filepath) do + '/var/www/rollbar/playground/rails4.2/vendor/bundle/gems/actionpack-4.2.0/lib/action_controller/metal/implicit_render.rb' + end + let(:frame) do + "#{filepath}:7:in `send_action'" + end + let(:options) do + { :configuration => configuration } + end + + before do + allow(backtrace).to receive(:get_file_lines).with(filepath).and_return(file.split("\n")) + end + + context 'with send_extra_frame_data = :none' do + let(:configuration) do + double('configuration', + :send_extra_frame_data => :none, + :root => '/var/www') + end + + it 'just return the filename, lineno and method' do + expected_result = { + :filename => filepath, + :lineno => 7, + :method => 'send_action' + } + + expect(subject.to_h).to be_eql(expected_result) + end + end + + context 'with send_extra_frame_data = :all' do + let(:configuration) do + double('configuration', + :send_extra_frame_data => :all, + :root => '/var/www') + end + + it 'returns also code and context' do + expected_result = { + :filename => filepath, + :lineno => 7, + :method => 'send_action', + :code => 'foo7', + :context => { + :pre => %w(foo3 foo4 foo5 foo6), + :post => %w(foo8 foo9 foo10 foo11) + } + } + + expect(subject.to_h).to be_eql(expected_result) + end + + context 'if there is not lines in the file' do + let(:file) do + '' + end + it 'just returns the basic data' do + expected_result = { + :filename => filepath, + :lineno => 7, + :method => 'send_action' + } + + expect(subject.to_h).to be_eql(expected_result) + end + end + + context 'if the file couldnt be read' do + before do + allow(backtrace).to receive(:get_file_lines).with(filepath).and_return(nil) + end + + it 'just returns the basic data' do + expected_result = { + :filename => filepath, + :lineno => 7, + :method => 'send_action' + } + + expect(subject.to_h).to be_eql(expected_result) + end + end + end + + context 'with send_extra_frame_data = :app' do + context 'with frame outside the root' do + let(:configuration) do + double('configuration', + :send_extra_frame_data => :app, + :root => '/outside/project', + :project_gem_paths => []) + end + + it 'just returns the basic frame data' do + expected_result = { + :filename => filepath, + :lineno => 7, + :method => 'send_action' + } + + expect(subject.to_h).to be_eql(expected_result) + end + end + + context 'with frame inside project_gem_paths' do + let(:configuration) do + double('configuration', + :send_extra_frame_data => :app, + :root => '/var/outside/', + :project_gem_paths => ['/var/www/']) + end + + it 'returns also context and code data' do + expected_result = { + :filename => filepath, + :lineno => 7, + :method => 'send_action', + :code => 'foo7', + :context => { + :pre => %w(foo3 foo4 foo5 foo6), + :post => %w(foo8 foo9 foo10 foo11) + } + } + + expect(subject.to_h).to be_eql(expected_result) + end + end + + context 'and frame inside app root' do + let(:configuration) do + double('configuration', + :send_extra_frame_data => :app, + :root => '/var/www', + :project_gem_paths => []) + end + + it 'returns also the context and code data' do + expected_result = { + :filename => filepath, + :lineno => 7, + :method => 'send_action', + :code => 'foo7', + :context => { + :pre => %w(foo3 foo4 foo5 foo6), + :post => %w(foo8 foo9 foo10 foo11) + } + } + + expect(subject.to_h).to be_eql(expected_result) + end + + context 'but inside Gem.path' do + let(:configuration) do + double('configuration', + :send_extra_frame_data => :app, + :root => '/var/www/', + :project_gem_paths => []) + end + + before do + allow(Gem).to receive(:path).and_return(['/var/www/rollbar']) + end + + it 'just returns also the basic data' do + expected_result = { + :filename => filepath, + :lineno => 7, + :method => 'send_action' + } + + expect(subject.to_h).to be_eql(expected_result) + end + end + + context 'having less pre lines than maximum' do + let(:frame) do + "#{filepath}:3:in `send_action'" + end + + it 'returns up to 2 pre lines' do + expected_result = { + :filename => filepath, + :lineno => 3, + :method => 'send_action', + :code => 'foo3', + :context => { + :pre => %w(foo1 foo2), + :post => %w(foo4 foo5 foo6 foo7) + } + } + + expect(subject.to_h).to be_eql(expected_result) + end + end + + context 'having less post lines than maximum' do + let(:frame) do + "#{filepath}:11:in `send_action'" + end + + it 'returns up to 2 post lines' do + expected_result = { + :filename => filepath, + :lineno => 11, + :method => 'send_action', + :code => 'foo11', + :context => { + :pre => %w(foo7 foo8 foo9 foo10), + :post => %w(foo12 foo13) + } + } + + expect(subject.to_h).to be_eql(expected_result) + end + end + end + end + end + end +end