Commit 88b72813f078b78ff467ea13d0a7f0b94be04b2c
Exists in
master
and in
1 other branch
Merge pull request #271 from martinciu/backtrace-extraction
Backtrace extraction
Showing
26 changed files
with
297 additions
and
148 deletions
Show diff stats
| @@ -0,0 +1,16 @@ | @@ -0,0 +1,16 @@ | ||
| 1 | +module BacktraceHelper | ||
| 2 | + # Group lines into sections of in-app files and external files | ||
| 3 | + # (An implementation of Enumerable#chunk so we don't break 1.8.7 support.) | ||
| 4 | + def grouped_lines(lines) | ||
| 5 | + line_groups = [] | ||
| 6 | + lines.each do |line| | ||
| 7 | + in_app = line.in_app? | ||
| 8 | + if line_groups.last && line_groups.last[0] == in_app | ||
| 9 | + line_groups.last[1] << line | ||
| 10 | + else | ||
| 11 | + line_groups << [in_app, [line]] | ||
| 12 | + end | ||
| 13 | + end | ||
| 14 | + line_groups | ||
| 15 | + end | ||
| 16 | +end |
| @@ -0,0 +1,38 @@ | @@ -0,0 +1,38 @@ | ||
| 1 | +module BacktraceLineHelper | ||
| 2 | + def link_to_source_file(line, &block) | ||
| 3 | + text = capture_haml(&block) | ||
| 4 | + line.in_app? ? link_to_in_app_source_file(line, text) : link_to_external_source_file(text) | ||
| 5 | + end | ||
| 6 | + | ||
| 7 | + private | ||
| 8 | + def link_to_in_app_source_file(line, text) | ||
| 9 | + link_to_repo_source_file(line, text) || link_to_issue_tracker_file(line, text) | ||
| 10 | + end | ||
| 11 | + | ||
| 12 | + def link_to_repo_source_file(line, text) | ||
| 13 | + link_to_github(line, text) || link_to_bitbucket(line, text) | ||
| 14 | + end | ||
| 15 | + | ||
| 16 | + def link_to_external_source_file(text) | ||
| 17 | + text | ||
| 18 | + end | ||
| 19 | + | ||
| 20 | + def link_to_github(line, text = nil) | ||
| 21 | + return unless line.app.github_repo? | ||
| 22 | + href = "%s#L%s" % [line.app.github_url_to_file(line.file), line.number] | ||
| 23 | + link_to(text || line.file_name, href, :target => '_blank') | ||
| 24 | + end | ||
| 25 | + | ||
| 26 | + def link_to_bitbucket(line, text = nil) | ||
| 27 | + return unless line.app.bitbucket_repo? | ||
| 28 | + href = "%s#cl-%s" % [line.app.bitbucket_url_to_file(line.file), line.number] | ||
| 29 | + link_to(text || line.file_name, href, :target => '_blank') | ||
| 30 | + end | ||
| 31 | + | ||
| 32 | + def link_to_issue_tracker_file(line, text = nil) | ||
| 33 | + return unless line.app.issue_tracker && line.app.issue_tracker.respond_to?(:url_to_file) | ||
| 34 | + href = line.app.issue_tracker.url_to_file(line.file, line.number) | ||
| 35 | + link_to(text || line.file_name, href, :target => '_blank') | ||
| 36 | + end | ||
| 37 | + | ||
| 38 | +end |
app/helpers/notices_helper.rb
| 1 | # encoding: utf-8 | 1 | # encoding: utf-8 |
| 2 | module NoticesHelper | 2 | module NoticesHelper |
| 3 | - def in_app_backtrace_line?(line) | ||
| 4 | - !!(line['file'] =~ %r{^\[PROJECT_ROOT\]/(?!(vendor))}) | ||
| 5 | - end | ||
| 6 | - | ||
| 7 | def notice_atom_summary(notice) | 3 | def notice_atom_summary(notice) |
| 8 | render "notices/atom_entry.html.haml", :notice => notice | 4 | render "notices/atom_entry.html.haml", :notice => notice |
| 9 | end | 5 | end |
| 10 | - | ||
| 11 | - def link_to_source_file(app, line, &block) | ||
| 12 | - text = capture_haml(&block) | ||
| 13 | - if in_app_backtrace_line?(line) | ||
| 14 | - return link_to_github(app, line, text) if app.github_repo? | ||
| 15 | - return link_to_bitbucket(app, line, text) if app.bitbucket_repo? | ||
| 16 | - if app.issue_tracker && app.issue_tracker.respond_to?(:url_to_file) | ||
| 17 | - # Return link to file on tracker if issue tracker supports this | ||
| 18 | - return link_to_issue_tracker_file(app, line, text) | ||
| 19 | - end | ||
| 20 | - end | ||
| 21 | - text | ||
| 22 | - end | ||
| 23 | - | ||
| 24 | - def filepath_parts(file) | ||
| 25 | - [file.split('/').last, file.gsub('[PROJECT_ROOT]', '')] | ||
| 26 | - end | ||
| 27 | - | ||
| 28 | - def link_to_github(app, line, text = nil) | ||
| 29 | - file_name, file_path = filepath_parts(line['file']) | ||
| 30 | - href = "%s#L%s" % [app.github_url_to_file(file_path), line['number']] | ||
| 31 | - link_to(text || file_name, href, :target => '_blank') | ||
| 32 | - end | ||
| 33 | - | ||
| 34 | - def link_to_bitbucket(app, line, text = nil) | ||
| 35 | - file_name, file_path = filepath_parts(line['file']) | ||
| 36 | - href = "%s#cl-%s" % [app.bitbucket_url_to_file(file_path), line['number']] | ||
| 37 | - link_to(text || file_name, href, :target => '_blank') | ||
| 38 | - end | ||
| 39 | - | ||
| 40 | - def link_to_issue_tracker_file(app, line, text = nil) | ||
| 41 | - file_name, file_path = filepath_parts(line['file']) | ||
| 42 | - href = app.issue_tracker.url_to_file(file_path, line['number']) | ||
| 43 | - link_to(text || file_name, href, :target => '_blank') | ||
| 44 | - end | ||
| 45 | - | ||
| 46 | - # Group lines into sections of in-app files and external files | ||
| 47 | - # (An implementation of Enumerable#chunk so we don't break 1.8.7 support.) | ||
| 48 | - def grouped_lines(lines) | ||
| 49 | - line_groups = [] | ||
| 50 | - lines.each do |line| | ||
| 51 | - in_app = in_app_backtrace_line?(line) | ||
| 52 | - if line_groups.last && line_groups.last[0] == in_app | ||
| 53 | - line_groups.last[1] << line | ||
| 54 | - else | ||
| 55 | - line_groups << [in_app, [line]] | ||
| 56 | - end | ||
| 57 | - end | ||
| 58 | - line_groups | ||
| 59 | - end | ||
| 60 | - | ||
| 61 | - def path_for_backtrace_line(line) | ||
| 62 | - path = File.dirname(line['file']) | ||
| 63 | - return '' if path == '.' | ||
| 64 | - # Remove [PROJECT_ROOT] | ||
| 65 | - path.gsub!('[PROJECT_ROOT]/', '') | ||
| 66 | - # Make gem name bold if starts with [GEM_ROOT]/gems | ||
| 67 | - path.gsub!(/\[GEM_ROOT\]\/gems\/([^\/]+)/, "<strong>\\1</strong>") | ||
| 68 | - (path << '/').html_safe | ||
| 69 | - end | ||
| 70 | - | ||
| 71 | - def file_for_backtrace_line(line) | ||
| 72 | - file = File.basename(line['file']) | ||
| 73 | - end | ||
| 74 | end | 6 | end |
| 75 | 7 |
| @@ -0,0 +1,36 @@ | @@ -0,0 +1,36 @@ | ||
| 1 | +class Backtrace | ||
| 2 | + include Mongoid::Document | ||
| 3 | + include Mongoid::Timestamps | ||
| 4 | + | ||
| 5 | + field :fingerprint | ||
| 6 | + index :fingerprint | ||
| 7 | + | ||
| 8 | + has_many :notices | ||
| 9 | + has_one :notice | ||
| 10 | + | ||
| 11 | + embeds_many :lines, :class_name => "BacktraceLine" | ||
| 12 | + | ||
| 13 | + after_initialize :generate_fingerprint | ||
| 14 | + | ||
| 15 | + delegate :app, :to => :notice | ||
| 16 | + | ||
| 17 | + def self.find_or_create(attributes = {}) | ||
| 18 | + new(attributes).similar || create(attributes) | ||
| 19 | + end | ||
| 20 | + | ||
| 21 | + def similar | ||
| 22 | + Backtrace.first(:conditions => { :fingerprint => fingerprint } ) | ||
| 23 | + end | ||
| 24 | + | ||
| 25 | + def raw=(raw) | ||
| 26 | + raw.each do |raw_line| | ||
| 27 | + lines << BacktraceLine.new(BacktraceLineNormalizer.new(raw_line).call) | ||
| 28 | + end | ||
| 29 | + end | ||
| 30 | + | ||
| 31 | + private | ||
| 32 | + def generate_fingerprint | ||
| 33 | + self.fingerprint = Digest::SHA1.hexdigest(lines.map(&:to_s).join) | ||
| 34 | + end | ||
| 35 | + | ||
| 36 | +end |
| @@ -0,0 +1,42 @@ | @@ -0,0 +1,42 @@ | ||
| 1 | +class BacktraceLine | ||
| 2 | + include Mongoid::Document | ||
| 3 | + IN_APP_PATH = %r{^\[PROJECT_ROOT\]\/(?!(vendor))} | ||
| 4 | + GEMS_PATH = %r{\[GEM_ROOT\]\/gems\/([^\/]+)} | ||
| 5 | + | ||
| 6 | + field :number, :type => Integer | ||
| 7 | + field :file | ||
| 8 | + field :method | ||
| 9 | + | ||
| 10 | + embedded_in :backtrace | ||
| 11 | + | ||
| 12 | + scope :in_app, where(:file => IN_APP_PATH) | ||
| 13 | + | ||
| 14 | + delegate :app, :to => :backtrace | ||
| 15 | + | ||
| 16 | + def to_s | ||
| 17 | + "#{file}:#{number}" | ||
| 18 | + end | ||
| 19 | + | ||
| 20 | + def in_app? | ||
| 21 | + !!(file =~ IN_APP_PATH) | ||
| 22 | + end | ||
| 23 | + | ||
| 24 | + def path | ||
| 25 | + File.dirname(file).gsub(/^\.$/, '') + "/" | ||
| 26 | + end | ||
| 27 | + | ||
| 28 | + def file_relative | ||
| 29 | + file.to_s.sub(IN_APP_PATH, '') | ||
| 30 | + end | ||
| 31 | + | ||
| 32 | + def file_name | ||
| 33 | + File.basename file | ||
| 34 | + end | ||
| 35 | + | ||
| 36 | + def decorated_path | ||
| 37 | + path.sub(BacktraceLine::IN_APP_PATH, ''). | ||
| 38 | + sub(BacktraceLine::GEMS_PATH, "<strong>\\1</strong>") | ||
| 39 | + end | ||
| 40 | + | ||
| 41 | +end | ||
| 42 | + |
| @@ -0,0 +1,19 @@ | @@ -0,0 +1,19 @@ | ||
| 1 | +class BacktraceLineNormalizer | ||
| 2 | + def initialize(raw_line) | ||
| 3 | + @raw_line = raw_line | ||
| 4 | + end | ||
| 5 | + | ||
| 6 | + def call | ||
| 7 | + @raw_line.merge 'file' => normalized_file, 'method' => normalized_method | ||
| 8 | + end | ||
| 9 | + | ||
| 10 | + private | ||
| 11 | + def normalized_file | ||
| 12 | + @raw_line['file'].blank? ? "[unknown source]" : @raw_line['file'].to_s.gsub(/\[PROJECT_ROOT\]\/.*\/ruby\/[0-9.]+\/gems/, '[GEM_ROOT]/gems') | ||
| 13 | + end | ||
| 14 | + | ||
| 15 | + def normalized_method | ||
| 16 | + @raw_line['method'].gsub(/[0-9_]{10,}+/, "__FRAGMENT__") | ||
| 17 | + end | ||
| 18 | + | ||
| 19 | +end |
app/models/error_report.rb
| @@ -2,7 +2,7 @@ require 'digest/sha1' | @@ -2,7 +2,7 @@ require 'digest/sha1' | ||
| 2 | require 'hoptoad_notifier' | 2 | require 'hoptoad_notifier' |
| 3 | 3 | ||
| 4 | class ErrorReport | 4 | class ErrorReport |
| 5 | - attr_reader :error_class, :message, :backtrace, :request, :server_environment, :api_key, :notifier, :user_attributes, :current_user | 5 | + attr_reader :error_class, :message, :request, :server_environment, :api_key, :notifier, :user_attributes, :current_user |
| 6 | 6 | ||
| 7 | def initialize(xml_or_attributes) | 7 | def initialize(xml_or_attributes) |
| 8 | @attributes = (xml_or_attributes.is_a?(String) ? Hoptoad.parse_xml!(xml_or_attributes) : xml_or_attributes).with_indifferent_access | 8 | @attributes = (xml_or_attributes.is_a?(String) ? Hoptoad.parse_xml!(xml_or_attributes) : xml_or_attributes).with_indifferent_access |
| @@ -29,11 +29,15 @@ class ErrorReport | @@ -29,11 +29,15 @@ class ErrorReport | ||
| 29 | @app ||= App.find_by_api_key!(api_key) | 29 | @app ||= App.find_by_api_key!(api_key) |
| 30 | end | 30 | end |
| 31 | 31 | ||
| 32 | + def backtrace | ||
| 33 | + @normalized_backtrace ||= Backtrace.find_or_create(:raw => @backtrace) | ||
| 34 | + end | ||
| 35 | + | ||
| 32 | def generate_notice! | 36 | def generate_notice! |
| 33 | notice = Notice.new( | 37 | notice = Notice.new( |
| 34 | :message => message, | 38 | :message => message, |
| 35 | :error_class => error_class, | 39 | :error_class => error_class, |
| 36 | - :backtrace => backtrace, | 40 | + :backtrace_id => backtrace.id, |
| 37 | :request => request, | 41 | :request => request, |
| 38 | :server_environment => server_environment, | 42 | :server_environment => server_environment, |
| 39 | :notifier => notifier, | 43 | :notifier => notifier, |
| @@ -55,7 +59,7 @@ class ErrorReport | @@ -55,7 +59,7 @@ class ErrorReport | ||
| 55 | private | 59 | private |
| 56 | def fingerprint_source | 60 | def fingerprint_source |
| 57 | { | 61 | { |
| 58 | - :backtrace => normalized_backtrace.to_s, | 62 | + :backtrace => backtrace.id, |
| 59 | :error_class => error_class, | 63 | :error_class => error_class, |
| 60 | :component => component, | 64 | :component => component, |
| 61 | :action => action, | 65 | :action => action, |
| @@ -64,11 +68,5 @@ class ErrorReport | @@ -64,11 +68,5 @@ class ErrorReport | ||
| 64 | } | 68 | } |
| 65 | end | 69 | end |
| 66 | 70 | ||
| 67 | - def normalized_backtrace | ||
| 68 | - backtrace[0...3].map do |trace| | ||
| 69 | - trace.merge 'method' => trace['method'].gsub(/[0-9_]{10,}+/, "__FRAGMENT__") | ||
| 70 | - end | ||
| 71 | - end | ||
| 72 | - | ||
| 73 | end | 71 | end |
| 74 | 72 |
app/models/notice.rb
| @@ -6,15 +6,17 @@ class Notice | @@ -6,15 +6,17 @@ class Notice | ||
| 6 | include Mongoid::Timestamps | 6 | include Mongoid::Timestamps |
| 7 | 7 | ||
| 8 | field :message | 8 | field :message |
| 9 | - field :backtrace, :type => Array | ||
| 10 | field :server_environment, :type => Hash | 9 | field :server_environment, :type => Hash |
| 11 | field :request, :type => Hash | 10 | field :request, :type => Hash |
| 12 | field :notifier, :type => Hash | 11 | field :notifier, :type => Hash |
| 13 | field :user_attributes, :type => Hash | 12 | field :user_attributes, :type => Hash |
| 14 | field :current_user, :type => Hash | 13 | field :current_user, :type => Hash |
| 15 | field :error_class | 14 | field :error_class |
| 15 | + delegate :lines, :to => :backtrace, :prefix => true | ||
| 16 | + delegate :app, :to => :err | ||
| 16 | 17 | ||
| 17 | belongs_to :err | 18 | belongs_to :err |
| 19 | + belongs_to :backtrace, :index => true | ||
| 18 | index :created_at | 20 | index :created_at |
| 19 | index( | 21 | index( |
| 20 | [ | 22 | [ |
| @@ -90,17 +92,8 @@ class Notice | @@ -90,17 +92,8 @@ class Notice | ||
| 90 | request['session'] || {} | 92 | request['session'] || {} |
| 91 | end | 93 | end |
| 92 | 94 | ||
| 93 | - # Backtrace containing only files from the app itself (ignore gems) | ||
| 94 | - def app_backtrace | ||
| 95 | - backtrace.select { |l| l && l['file'] && l['file'].include?("[PROJECT_ROOT]") } | ||
| 96 | - end | ||
| 97 | - | ||
| 98 | - def backtrace | ||
| 99 | - # If gems are vendored into project, treat vendored gem dir as [GEM_ROOT] | ||
| 100 | - (read_attribute(:backtrace) || []).map do |line| | ||
| 101 | - # Changes "[PROJECT_ROOT]/rubygems/ruby/1.9.1/gems" to "[GEM_ROOT]/gems" | ||
| 102 | - line.merge 'file' => line['file'].to_s.gsub(/\[PROJECT_ROOT\]\/.*\/ruby\/[0-9.]+\/gems/, '[GEM_ROOT]/gems') | ||
| 103 | - end | 95 | + def in_app_backtrace_lines |
| 96 | + backtrace_lines.in_app | ||
| 104 | end | 97 | end |
| 105 | 98 | ||
| 106 | protected | 99 | protected |
| @@ -129,8 +122,6 @@ class Notice | @@ -129,8 +122,6 @@ class Notice | ||
| 129 | [:server_environment, :request, :notifier].each do |h| | 122 | [:server_environment, :request, :notifier].each do |h| |
| 130 | send("#{h}=",sanitize_hash(send(h))) | 123 | send("#{h}=",sanitize_hash(send(h))) |
| 131 | end | 124 | end |
| 132 | - # Set unknown backtrace files | ||
| 133 | - read_attribute(:backtrace).each{|line| line['file'] = "[unknown source]" if line['file'].blank? } | ||
| 134 | end | 125 | end |
| 135 | 126 | ||
| 136 | def sanitize_hash(h) | 127 | def sanitize_hash(h) |
app/views/issue_trackers/fogbugz_body.txt.erb
| @@ -19,8 +19,8 @@ | @@ -19,8 +19,8 @@ | ||
| 19 | <%= pretty_hash(notice.session) %> | 19 | <%= pretty_hash(notice.session) %> |
| 20 | 20 | ||
| 21 | Backtrace | 21 | Backtrace |
| 22 | - <% for line in notice.backtrace %> | ||
| 23 | - <%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> | 22 | + <% for line in notice.backtrace_lines %> |
| 23 | + <%= line.number %>: <%= line.file_relative %> | ||
| 24 | <% end %> | 24 | <% end %> |
| 25 | 25 | ||
| 26 | Environment | 26 | Environment |
app/views/issue_trackers/github_issues_body.txt.erb
| @@ -27,7 +27,7 @@ | @@ -27,7 +27,7 @@ | ||
| 27 | 27 | ||
| 28 | ## Backtrace ## | 28 | ## Backtrace ## |
| 29 | ``` | 29 | ``` |
| 30 | -<% for line in notice.backtrace %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>** | 30 | +<% for line in notice.backtrace_lines %><%= line.number %>: <%= line.file_relative %> -> **<%= line.method %>** |
| 31 | <% end %> | 31 | <% end %> |
| 32 | ``` | 32 | ``` |
| 33 | 33 |
app/views/issue_trackers/lighthouseapp_body.txt.erb
| @@ -23,7 +23,7 @@ | @@ -23,7 +23,7 @@ | ||
| 23 | 23 | ||
| 24 | ## Backtrace ## | 24 | ## Backtrace ## |
| 25 | <code> | 25 | <code> |
| 26 | - <% for line in notice.backtrace %><%= line['number'] %>: <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> -> **<%= line['method'] %>** | 26 | + <% for line in notice.backtrace_lines %><%= line.number %>: <%= line.file_relative %> -> **<%= line.method %>** |
| 27 | <% end %> | 27 | <% end %> |
| 28 | </code> | 28 | </code> |
| 29 | 29 |
app/views/issue_trackers/pivotal_body.txt.erb
| @@ -12,6 +12,6 @@ See this exception on Errbit: <%= app_problem_url problem.app, problem %> | @@ -12,6 +12,6 @@ See this exception on Errbit: <%= app_problem_url problem.app, problem %> | ||
| 12 | <%= pretty_hash notice.session %> | 12 | <%= pretty_hash notice.session %> |
| 13 | 13 | ||
| 14 | Backtrace: | 14 | Backtrace: |
| 15 | - <%= notice.backtrace[0..4].map { |line| "#{line['number']}: #{line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '')} -> *#{line['method']}*" }.join "\n" %> | 15 | + <%= notice.backtrace_lines[0..4].map { |line| "#{line.number}: #{line.file_relative} -> *#{line.method}*" }.join "\n" %> |
| 16 | <% end %> | 16 | <% end %> |
| 17 | 17 |
app/views/issue_trackers/textile_body.txt.erb
| @@ -32,7 +32,7 @@ h2. Session | @@ -32,7 +32,7 @@ h2. Session | ||
| 32 | h2. Backtrace | 32 | h2. Backtrace |
| 33 | 33 | ||
| 34 | | Line | File | Method | | 34 | | Line | File | Method | |
| 35 | -<% for line in notice.backtrace %>| <%= line['number'] %> | <%= line['file'].to_s.sub(/^\[PROJECT_ROOT\]/, '') %> | *<%= line['method'] %>* | | 35 | +<% for line in notice.backtrace_lines %>| <%= line.number %> | <%= line.file_relative %> | *<%= line.method %>* | |
| 36 | <% end %> | 36 | <% end %> |
| 37 | 37 | ||
| 38 | h2. Environment | 38 | h2. Environment |
app/views/mailer/err_notification.html.haml
| @@ -27,16 +27,15 @@ | @@ -27,16 +27,15 @@ | ||
| 27 | %p.heading WHERE: | 27 | %p.heading WHERE: |
| 28 | %p.monospace | 28 | %p.monospace |
| 29 | = @notice.where | 29 | = @notice.where |
| 30 | - - if (app_backtrace = @notice.app_backtrace).any? | ||
| 31 | - - app_backtrace.map {|l| "#{l['file']}:#{l['number']}" }.each do |line| | ||
| 32 | - %p.backtrace= line | 30 | + - @notice.in_app_backtrace_lines.each do |line| |
| 31 | + %p.backtrace= line | ||
| 33 | %br | 32 | %br |
| 34 | %p.heading URL: | 33 | %p.heading URL: |
| 35 | %p.monospace | 34 | %p.monospace |
| 36 | - if @notice.request['url'].present? | 35 | - if @notice.request['url'].present? |
| 37 | = link_to @notice.request['url'], @notice.request['url'] | 36 | = link_to @notice.request['url'], @notice.request['url'] |
| 38 | %p.heading BACKTRACE: | 37 | %p.heading BACKTRACE: |
| 39 | - - @notice.backtrace.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line| | 38 | + - @notice.backtrace_lines.each do |line| |
| 40 | %p.backtrace= line | 39 | %p.backtrace= line |
| 41 | %br | 40 | %br |
| 42 | 41 |
app/views/mailer/err_notification.text.erb
| @@ -14,7 +14,7 @@ WHERE: | @@ -14,7 +14,7 @@ WHERE: | ||
| 14 | 14 | ||
| 15 | <%= @notice.where %> | 15 | <%= @notice.where %> |
| 16 | 16 | ||
| 17 | -<% @notice.app_backtrace.map {|l| "#{l['file']}:#{l['number']}" }.each do |line| %> | 17 | +<% @notice.in_app_backtrace_lines.each do |line| %> |
| 18 | <%= line %> | 18 | <%= line %> |
| 19 | <% end %> | 19 | <% end %> |
| 20 | 20 | ||
| @@ -26,7 +26,7 @@ URL: | @@ -26,7 +26,7 @@ URL: | ||
| 26 | 26 | ||
| 27 | BACKTRACE: | 27 | BACKTRACE: |
| 28 | 28 | ||
| 29 | -<% @notice.backtrace.map {|l| l ? "#{l['file']}:#{l['number']}" : nil }.compact.each do |line| %> | 29 | +<% @notice.backtrace_lines.each do |line| %> |
| 30 | <%= line %> | 30 | <%= line %> |
| 31 | <% end %> | 31 | <% end %> |
| 32 | 32 |
app/views/notices/_atom_entry.html.haml
| @@ -22,13 +22,13 @@ | @@ -22,13 +22,13 @@ | ||
| 22 | 22 | ||
| 23 | %h3 Backtrace | 23 | %h3 Backtrace |
| 24 | %table | 24 | %table |
| 25 | - - for line in notice.backtrace | 25 | + - for line in notice.backtrace_lines |
| 26 | %tr | 26 | %tr |
| 27 | %td | 27 | %td |
| 28 | - = "#{line['number']}:" | 28 | + = "#{line.number}:" |
| 29 | | 29 | |
| 30 | %td | 30 | %td |
| 31 | - = raw "#{h line['file'].sub(/^\[PROJECT_ROOT\]/, '')} -> #{content_tag :strong, h(line['method'])}" | 31 | + = raw "#{h line.file_relative} -> #{content_tag :strong, h(line.method)}" |
| 32 | 32 | ||
| 33 | %h3 Environment | 33 | %h3 Environment |
| 34 | %table | 34 | %table |
app/views/notices/_backtrace_line.html.haml
| 1 | -%tr{:class => defined?(row_class) ? row_class : nil} | ||
| 2 | - %td.line{:class => (in_app_backtrace_line?(line) ? 'in-app' : nil)} | ||
| 3 | - = link_to_source_file(@app, line) do | ||
| 4 | - %span.path>= path_for_backtrace_line(line) | ||
| 5 | - %span.file>= file_for_backtrace_line(line) | ||
| 6 | - - if line['number'].present? | ||
| 7 | - %span.number>= ":#{line['number']}" | 1 | +%tr{:class => defined?(row_class) && row_class} |
| 2 | + %td.line{:class => line.in_app? && 'in-app' } | ||
| 3 | + = link_to_source_file(line) do | ||
| 4 | + %span.path>=raw line.decorated_path | ||
| 5 | + %span.file>= line.file_name | ||
| 6 | + - if line.number.present? | ||
| 7 | + %span.number>= ":#{line.number}" | ||
| 8 | → | 8 | → |
| 9 | - %span.method= line['method'] | 9 | + %span.method= line.method |
app/views/problems/show.html.haml
| @@ -68,7 +68,7 @@ | @@ -68,7 +68,7 @@ | ||
| 68 | 68 | ||
| 69 | #backtrace | 69 | #backtrace |
| 70 | %h3 Backtrace | 70 | %h3 Backtrace |
| 71 | - = render 'notices/backtrace', :lines => @notice.backtrace | 71 | + = render 'notices/backtrace', :lines => @notice.backtrace_lines |
| 72 | 72 | ||
| 73 | - if @notice.user_attributes.present? | 73 | - if @notice.user_attributes.present? |
| 74 | #user_attributes | 74 | #user_attributes |
| @@ -0,0 +1,15 @@ | @@ -0,0 +1,15 @@ | ||
| 1 | +class ExtractBacktraces < Mongoid::Migration | ||
| 2 | + def self.up | ||
| 3 | + say "It could take long time (hours if you have many Notices)" | ||
| 4 | + Notice.unscoped.all.each do |notice| | ||
| 5 | + backtrace = Backtrace.find_or_create(:raw => notice['backtrace']) | ||
| 6 | + notice.backtrace = backtrace | ||
| 7 | + notice['backtrace'] = nil | ||
| 8 | + notice.save! | ||
| 9 | + end | ||
| 10 | + say "run `db.repairDatabase()` (in mongodb console) to recover deleted space" | ||
| 11 | + end | ||
| 12 | + | ||
| 13 | + def self.down | ||
| 14 | + end | ||
| 15 | +end |
db/migrate/20121005142110_regenerate_err_fingerprints.rb
0 → 100644
| @@ -0,0 +1,19 @@ | @@ -0,0 +1,19 @@ | ||
| 1 | +class RegenerateErrFingerprints < Mongoid::Migration | ||
| 2 | + def self.up | ||
| 3 | + Err.all.each do |err| | ||
| 4 | + fingerprint_source = { | ||
| 5 | + :backtrace => err.notices.first.backtrace_id, | ||
| 6 | + :error_class => err.error_class, | ||
| 7 | + :component => err.component, | ||
| 8 | + :action => err.action, | ||
| 9 | + :environment => err.environment, | ||
| 10 | + :api_key => err.app.api_key | ||
| 11 | + } | ||
| 12 | + fingerprint = Digest::SHA1.hexdigest(fingerprint_source.to_s) | ||
| 13 | + err.update_attribute(:fingerprint, fingerprint) | ||
| 14 | + end | ||
| 15 | + end | ||
| 16 | + | ||
| 17 | + def self.down | ||
| 18 | + end | ||
| 19 | +end |
spec/fabricators/err_fabricator.rb
| @@ -9,19 +9,19 @@ end | @@ -9,19 +9,19 @@ end | ||
| 9 | Fabricator :notice do | 9 | Fabricator :notice do |
| 10 | err! | 10 | err! |
| 11 | message 'FooError: Too Much Bar' | 11 | message 'FooError: Too Much Bar' |
| 12 | - backtrace { random_backtrace } | 12 | + backtrace! |
| 13 | server_environment { {'environment-name' => 'production'} } | 13 | server_environment { {'environment-name' => 'production'} } |
| 14 | request {{ 'component' => 'foo', 'action' => 'bar' }} | 14 | request {{ 'component' => 'foo', 'action' => 'bar' }} |
| 15 | notifier {{ 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }} | 15 | notifier {{ 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }} |
| 16 | end | 16 | end |
| 17 | 17 | ||
| 18 | -def random_backtrace | ||
| 19 | - backtrace = [] | ||
| 20 | - 99.times {|t| backtrace << { | ||
| 21 | - 'number' => rand(999), | ||
| 22 | - 'file' => "/path/to/file/#{SecureRandom.hex(4)}.rb", | ||
| 23 | - 'method' => ActiveSupport.methods.shuffle.first | ||
| 24 | - }} | ||
| 25 | - backtrace | 18 | +Fabricator :backtrace do |
| 19 | + fingerprint "fingerprint" | ||
| 20 | + lines(:count => 99) { Fabricate.build(:backtrace_line) } | ||
| 26 | end | 21 | end |
| 27 | 22 | ||
| 23 | +Fabricator :backtrace_line do | ||
| 24 | + number { rand(999) } | ||
| 25 | + file { "/path/to/file/#{SecureRandom.hex(4)}.rb" } | ||
| 26 | + method(:method) { ActiveSupport.methods.shuffle.first } | ||
| 27 | +end |
spec/models/app_spec.rb
| @@ -200,8 +200,8 @@ describe App do | @@ -200,8 +200,8 @@ describe App do | ||
| 200 | 200 | ||
| 201 | it 'captures the backtrace' do | 201 | it 'captures the backtrace' do |
| 202 | @notice = App.report_error!(@xml) | 202 | @notice = App.report_error!(@xml) |
| 203 | - @notice.backtrace.size.should == 73 | ||
| 204 | - @notice.backtrace.last['file'].should == '[GEM_ROOT]/bin/rake' | 203 | + @notice.backtrace_lines.size.should == 73 |
| 204 | + @notice.backtrace_lines.last['file'].should == '[GEM_ROOT]/bin/rake' | ||
| 205 | end | 205 | end |
| 206 | 206 | ||
| 207 | it 'captures the server_environment' do | 207 | it 'captures the server_environment' do |
| @@ -228,7 +228,7 @@ describe App do | @@ -228,7 +228,7 @@ describe App do | ||
| 228 | it "should handle params with only a single line of backtrace" do | 228 | it "should handle params with only a single line of backtrace" do |
| 229 | xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read | 229 | xml = Rails.root.join('spec','fixtures','hoptoad_test_notice_with_one_line_of_backtrace.xml').read |
| 230 | lambda { @notice = App.report_error!(xml) }.should_not raise_error | 230 | lambda { @notice = App.report_error!(xml) }.should_not raise_error |
| 231 | - @notice.backtrace.length.should == 1 | 231 | + @notice.backtrace_lines.length.should == 1 |
| 232 | end | 232 | end |
| 233 | 233 | ||
| 234 | it 'captures the current_user' do | 234 | it 'captures the current_user' do |
| @@ -238,7 +238,7 @@ describe App do | @@ -238,7 +238,7 @@ describe App do | ||
| 238 | @notice.current_user['email'].should == 'mr.bean@example.com' | 238 | @notice.current_user['email'].should == 'mr.bean@example.com' |
| 239 | @notice.current_user['username'].should == 'mrbean' | 239 | @notice.current_user['username'].should == 'mrbean' |
| 240 | end | 240 | end |
| 241 | - | 241 | + |
| 242 | end | 242 | end |
| 243 | 243 | ||
| 244 | 244 |
| @@ -0,0 +1,14 @@ | @@ -0,0 +1,14 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe BacktraceLineNormalizer do | ||
| 4 | + subject { described_class.new(raw_line).call } | ||
| 5 | + | ||
| 6 | + describe "sanitize file" do | ||
| 7 | + let(:raw_line) { { 'number' => rand(999), 'file' => nil, 'method' => ActiveSupport.methods.shuffle.first.to_s } } | ||
| 8 | + | ||
| 9 | + it "should replace nil file with [unknown source]" do | ||
| 10 | + subject['file'].should == "[unknown source]" | ||
| 11 | + end | ||
| 12 | + | ||
| 13 | + end | ||
| 14 | +end |
| @@ -0,0 +1,46 @@ | @@ -0,0 +1,46 @@ | ||
| 1 | +require 'spec_helper' | ||
| 2 | + | ||
| 3 | +describe Backtrace do | ||
| 4 | + subject { described_class.new } | ||
| 5 | + | ||
| 6 | + its(:fingerprint) { should be_present } | ||
| 7 | + | ||
| 8 | + describe "#similar" do | ||
| 9 | + context "no similar backtrace" do | ||
| 10 | + its(:similar) { should be_nil } | ||
| 11 | + end | ||
| 12 | + | ||
| 13 | + context "similar backtrace exist" do | ||
| 14 | + let!(:similar_backtrace) { Fabricate(:backtrace, :fingerprint => fingerprint) } | ||
| 15 | + let(:fingerprint) { "fingerprint" } | ||
| 16 | + | ||
| 17 | + before { subject.stub(:fingerprint => fingerprint) } | ||
| 18 | + | ||
| 19 | + its(:similar) { should == similar_backtrace } | ||
| 20 | + end | ||
| 21 | + end | ||
| 22 | + | ||
| 23 | + describe "find_or_create" do | ||
| 24 | + subject { described_class.find_or_create(attributes) } | ||
| 25 | + let(:attributes) { mock :attributes } | ||
| 26 | + let(:backtrace) { mock :backtrace } | ||
| 27 | + | ||
| 28 | + before { described_class.stub(:new => backtrace) } | ||
| 29 | + | ||
| 30 | + context "no similar backtrace" do | ||
| 31 | + before { backtrace.stub(:similar => nil) } | ||
| 32 | + it "create new backtrace" do | ||
| 33 | + described_class.should_receive(:create).with(attributes) | ||
| 34 | + | ||
| 35 | + described_class.find_or_create(attributes) | ||
| 36 | + end | ||
| 37 | + end | ||
| 38 | + | ||
| 39 | + context "similar backtrace exist" do | ||
| 40 | + let(:similar_backtrace) { mock :similar_backtrace } | ||
| 41 | + before { backtrace.stub(:similar => similar_backtrace) } | ||
| 42 | + | ||
| 43 | + it { should == similar_backtrace } | ||
| 44 | + end | ||
| 45 | + end | ||
| 46 | +end |
spec/models/notice_observer_spec.rb
| @@ -46,6 +46,7 @@ describe NoticeObserver do | @@ -46,6 +46,7 @@ describe NoticeObserver do | ||
| 46 | describe "should send a notification if a notification service is configured" do | 46 | describe "should send a notification if a notification service is configured" do |
| 47 | let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:campfire_notification_service))} | 47 | let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:campfire_notification_service))} |
| 48 | let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } | 48 | let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } |
| 49 | + let(:backtrace) { Fabricate(:backtrace) } | ||
| 49 | 50 | ||
| 50 | before do | 51 | before do |
| 51 | Errbit::Config.per_app_email_at_notices = true | 52 | Errbit::Config.per_app_email_at_notices = true |
| @@ -59,13 +60,14 @@ describe NoticeObserver do | @@ -59,13 +60,14 @@ describe NoticeObserver do | ||
| 59 | app.notification_service.should_receive(:create_notification) | 60 | app.notification_service.should_receive(:create_notification) |
| 60 | 61 | ||
| 61 | Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | 62 | Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, |
| 62 | - :backtrace => [{ :error => 'Le Broken' }], :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | 63 | + :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) |
| 63 | end | 64 | end |
| 64 | end | 65 | end |
| 65 | 66 | ||
| 66 | describe "should not send a notification if a notification service is not configured" do | 67 | describe "should not send a notification if a notification service is not configured" do |
| 67 | let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:notification_service))} | 68 | let(:app) { Fabricate(:app, :email_at_notices => [1], :notification_service => Fabricate(:notification_service))} |
| 68 | let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } | 69 | let(:err) { Fabricate(:err, :problem => Fabricate(:problem, :app => app, :notices_count => 100)) } |
| 70 | + let(:backtrace) { Fabricate(:backtrace) } | ||
| 69 | 71 | ||
| 70 | before do | 72 | before do |
| 71 | Errbit::Config.per_app_email_at_notices = true | 73 | Errbit::Config.per_app_email_at_notices = true |
| @@ -79,7 +81,7 @@ describe NoticeObserver do | @@ -79,7 +81,7 @@ describe NoticeObserver do | ||
| 79 | app.notification_service.should_not_receive(:create_notification) | 81 | app.notification_service.should_not_receive(:create_notification) |
| 80 | 82 | ||
| 81 | Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, | 83 | Notice.create!(:err => err, :message => 'FooError: Too Much Bar', :server_environment => {'environment-name' => 'production'}, |
| 82 | - :backtrace => [{ :error => 'Le Broken' }], :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) | 84 | + :backtrace => backtrace, :notifier => { 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' }) |
| 83 | end | 85 | end |
| 84 | end | 86 | end |
| 85 | 87 |
spec/views/notices/_backtrace.html.haml_spec.rb
| @@ -1,18 +0,0 @@ | @@ -1,18 +0,0 @@ | ||
| 1 | -require 'spec_helper' | ||
| 2 | - | ||
| 3 | -describe "notices/_backtrace.html.haml" do | ||
| 4 | - describe 'missing file in backtrace' do | ||
| 5 | - let(:notice) do | ||
| 6 | - backtrace = { 'number' => rand(999), 'file' => nil, 'method' => ActiveSupport.methods.shuffle.first } | ||
| 7 | - Fabricate(:notice, :backtrace => [backtrace]) | ||
| 8 | - end | ||
| 9 | - | ||
| 10 | - it "should replace nil file with [unknown source]" do | ||
| 11 | - assign :app, notice.err.app | ||
| 12 | - | ||
| 13 | - render "notices/backtrace", :lines => notice.backtrace | ||
| 14 | - rendered.should match(/\[unknown source\]/) | ||
| 15 | - end | ||
| 16 | - end | ||
| 17 | -end | ||
| 18 | - |