From 65604e115c56c98bac058e7414494407ddb776ed Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Fri, 7 Jun 2024 19:59:22 +0300 Subject: [PATCH 1/5] Add #root_directory as an option --- lib/telebugs/config.rb | 8 +++++++- test/test_config.rb | 6 ++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/telebugs/config.rb b/lib/telebugs/config.rb index 7bf0e08..bb22e30 100644 --- a/lib/telebugs/config.rb +++ b/lib/telebugs/config.rb @@ -6,7 +6,9 @@ module Telebugs class Config ERROR_API_URL = "https://api.telebugs.com/2024-03-28/errors" - attr_accessor :api_key + attr_accessor :api_key, + :root_directory + attr_reader :api_url class << self @@ -28,6 +30,10 @@ def api_url=(url) def reset self.api_key = nil self.api_url = ERROR_API_URL + self.root_directory = File.realpath( + (defined?(Bundler) && Bundler.root) || + Dir.pwd, + ) end end end diff --git a/test/test_config.rb b/test/test_config.rb index 3a79076..4d4e7e4 100644 --- a/test/test_config.rb +++ b/test/test_config.rb @@ -18,4 +18,10 @@ def test_error_api_url assert_equal URI("example.com"), Telebugs::Config.instance.api_url end + + def test_root_directory + Telebugs.configure { |c| c.root_directory = "/tmp" } + + assert_equal "/tmp", Telebugs::Config.instance.root_directory + end end From 2e5a84835edc029a3dbe9c133c902f10d33da636 Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Fri, 7 Jun 2024 20:56:26 +0300 Subject: [PATCH 2/5] Add FileCache --- lib/telebugs.rb | 1 + lib/telebugs/file_cache.rb | 41 ++++++++++++++++++++++++++++++++++++++ test/test_file_cache.rb | 30 ++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+) create mode 100644 lib/telebugs/file_cache.rb create mode 100644 test/test_file_cache.rb diff --git a/lib/telebugs.rb b/lib/telebugs.rb index 0bfdb91..1fa84e9 100644 --- a/lib/telebugs.rb +++ b/lib/telebugs.rb @@ -13,6 +13,7 @@ require_relative "telebugs/notice" require_relative "telebugs/error_message" require_relative "telebugs/backtrace" +require_relative "telebugs/file_cache" module Telebugs # The general error that this library uses when it wants to raise. diff --git a/lib/telebugs/file_cache.rb b/lib/telebugs/file_cache.rb new file mode 100644 index 0000000..d88ae13 --- /dev/null +++ b/lib/telebugs/file_cache.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +module Telebugs + module FileCache + MAX_SIZE = 50 + MUTEX = Mutex.new + + # Associates the value given by +value+ with the key given by +key+. Deletes + # entries that exceed +MAX_SIZE+. + def self.[]=(key, value) + MUTEX.synchronize do + data[key] = value + data.delete(data.keys.first) if data.size > MAX_SIZE + end + end + + # Retrieve an object from the cache. + def self.[](key) + MUTEX.synchronize do + data[key] + end + end + + # Checks whether the cache is empty. Needed only for the test suite. + def self.empty? + MUTEX.synchronize do + data.empty? + end + end + + def self.reset + MUTEX.synchronize do + @data = {} + end + end + + private_class_method def self.data + @data ||= {} + end + end +end diff --git a/test/test_file_cache.rb b/test/test_file_cache.rb new file mode 100644 index 0000000..56b4426 --- /dev/null +++ b/test/test_file_cache.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require "test_helper" + +class TestFileCache < Minitest::Test + def teardown + Telebugs::FileCache.reset + end + + def test_set_when_cache_limit_is_not_reached + max_size = Telebugs::FileCache::MAX_SIZE + max_size.times do |i| + Telebugs::FileCache["key#{i}"] = "value#{i}" + end + + assert_equal "value0", Telebugs::FileCache["key0"] + assert_equal "value#{max_size-1}", Telebugs::FileCache["key#{max_size-1}"] + end + + def test_set_when_cache_over_limit + max_size = 2*Telebugs::FileCache::MAX_SIZE + max_size.times do |i| + Telebugs::FileCache["key#{i}"] = "value#{i}" + end + + assert_nil Telebugs::FileCache["key49"] + assert_equal "value50", Telebugs::FileCache["key50"] + assert_equal "value#{max_size-1}", Telebugs::FileCache["key#{max_size-1}"] + end +end From d5a88079c64b17f91a395339267e4aa6c9382b27 Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Fri, 7 Jun 2024 23:47:39 +0300 Subject: [PATCH 3/5] Send code with errors --- lib/telebugs.rb | 1 + lib/telebugs/code_hunk.rb | 44 +++++++++++++++++++ lib/telebugs/notice.rb | 10 ++++- test/fixtures/code.rb | 39 +++++++++++++++++ test/fixtures/empty_file.rb | 0 test/fixtures/long_line.rb | 1 + test/fixtures/one_line.rb | 1 + test/test_code_hunk.rb | 87 +++++++++++++++++++++++++++++++++++++ 8 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 lib/telebugs/code_hunk.rb create mode 100644 test/fixtures/code.rb create mode 100644 test/fixtures/empty_file.rb create mode 100644 test/fixtures/long_line.rb create mode 100644 test/fixtures/one_line.rb create mode 100644 test/test_code_hunk.rb diff --git a/lib/telebugs.rb b/lib/telebugs.rb index 1fa84e9..2b99af2 100644 --- a/lib/telebugs.rb +++ b/lib/telebugs.rb @@ -14,6 +14,7 @@ require_relative "telebugs/error_message" require_relative "telebugs/backtrace" require_relative "telebugs/file_cache" +require_relative "telebugs/code_hunk" module Telebugs # The general error that this library uses when it wants to raise. diff --git a/lib/telebugs/code_hunk.rb b/lib/telebugs/code_hunk.rb new file mode 100644 index 0000000..fa00e35 --- /dev/null +++ b/lib/telebugs/code_hunk.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module Telebugs + # Represents a small hunk of code consisting of a base line and a couple lines + # around it + class CodeHunk + MAX_LINE_LEN = 200 + + # How many lines should be read around the base line. + AROUND_LINES = 2 + + def self.get(file, line) + start_line = [line - AROUND_LINES, 1].max + + lines = get_lines(file, start_line, line + AROUND_LINES) + return { start_line: 0, lines: [] } if lines.empty? + + { + start_line: start_line, + lines: lines + } + end + + private_class_method def self.get_lines(file, start_line, end_line) + lines = [] + return lines unless (cached_file = get_from_cache(file)) + + cached_file.with_index(1) do |l, i| + next if i < start_line + break if i > end_line + + lines << l[0...MAX_LINE_LEN].rstrip + end + + lines + end + + private_class_method def self.get_from_cache(file) + Telebugs::FileCache[file] ||= File.foreach(file) + rescue StandardError + nil + end + end +end diff --git a/lib/telebugs/notice.rb b/lib/telebugs/notice.rb index e29eacd..40719f5 100644 --- a/lib/telebugs/notice.rb +++ b/lib/telebugs/notice.rb @@ -29,6 +29,8 @@ def to_json(*_args) loop do begin json = @payload.to_json + require 'pp' + pp @payload rescue *JSON_EXCEPTIONS # TODO else @@ -46,7 +48,13 @@ def errors_as_json(error) { type: e.class.name, message: ErrorMessage.parse(e), - backtrace: Backtrace.parse(e) + backtrace: Backtrace.parse(e).each do |frame| + next unless frame[:file] + next unless File.exist?(frame[:file]) + next unless frame[:line] + + frame[:code] = CodeHunk.get(frame[:file], frame[:line]) + end } end end diff --git a/test/fixtures/code.rb b/test/fixtures/code.rb new file mode 100644 index 0000000..8d7e2d0 --- /dev/null +++ b/test/fixtures/code.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +class Botley + RESPONSES = { + /hello/i => "Oh, hello there! Are you ready to have some fun?", + /how are you/i => "I'm just a bunch of code, but thanks for asking! How about you?", + /what is your name/i => "I'm Botley, your friendly (and sometimes cheeky) virtual assistant!", + /joke/i => "Why don't scientists trust atoms? Because they make up everything!", + /bye|goodbye|exit/i => "Goodbye! Don't miss me too much!", + /thank you|thanks/i => "You're welcome! I'm here all week.", + /default/ => "I'm not sure what you mean, but it sounds intriguing!" + } + + def initialize + puts "Botley: Hello! I'm Botley, your virtual assistant. Type 'goodbye' to exit." + end + + def start + loop do + print "You: " + user_input = gets.chomp + response = respond_to(user_input) + puts "Botley: #{response}" + break if user_input.match?(/bye|goodbye|exit/i) + end + end + + private + + def respond_to(input) + RESPONSES.each do |pattern, response| + return response if input.match?(pattern) + end + RESPONSES[:default] + end +end + +# Start the conversation with Botley +Botley.new.start diff --git a/test/fixtures/empty_file.rb b/test/fixtures/empty_file.rb new file mode 100644 index 0000000..e69de29 diff --git a/test/fixtures/long_line.rb b/test/fixtures/long_line.rb new file mode 100644 index 0000000..1973950 --- /dev/null +++ b/test/fixtures/long_line.rb @@ -0,0 +1 @@ +loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong line diff --git a/test/fixtures/one_line.rb b/test/fixtures/one_line.rb new file mode 100644 index 0000000..e421822 --- /dev/null +++ b/test/fixtures/one_line.rb @@ -0,0 +1 @@ +Boom.new.call diff --git a/test/test_code_hunk.rb b/test/test_code_hunk.rb new file mode 100644 index 0000000..5f23b9b --- /dev/null +++ b/test/test_code_hunk.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require "test_helper" + +class TestCodeHunk < Minitest::Test + def test_get_when_file_is_empty + hunk = Telebugs::CodeHunk.get("test/fixtures/empty_file.rb", 1) + + assert_equal({start_line: 0, lines: []}, hunk) + end + + def test_get_when_file_has_one_line + hunk = Telebugs::CodeHunk.get("test/fixtures/one_line.rb", 1) + + assert_equal( + { + start_line: 1, + lines: ["Boom.new.call"] + }, + hunk + ) + end + + def test_get + hunk = Telebugs::CodeHunk.get("test/fixtures/code.rb", 18) + + assert_equal( + { + start_line: 16, + lines: [ + " end", + "", + " def start", + " loop do", + " print \"You: \"" + ] + }, + hunk + ) + end + + def test_get_with_edge_case_first_line + hunk = Telebugs::CodeHunk.get("test/fixtures/code.rb", 1) + + assert_equal( + { + start_line: 1, + lines: [ + "# frozen_string_literal: true", + "", + "class Botley" + ] + }, + hunk + ) + end + + def test_get_with_edge_case_last_line + hunk = Telebugs::CodeHunk.get("test/fixtures/code.rb", 39) + + assert_equal( + { + start_line: 37, + lines: [ + "", + "# Start the conversation with Botley", + "Botley.new.start" + ] + }, + hunk + ) + end + + def test_get_when_code_line_is_too_long + hunk = Telebugs::CodeHunk.get("test/fixtures/long_line.rb", 1) + + assert_equal( + { + start_line: 1, + lines: [ + "loooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong" + ] + }, + hunk + ) + end +end From c3a0a3ff00b6d1e3c9a54faca265f6ac707886ae Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Sat, 8 Jun 2024 21:52:15 +0300 Subject: [PATCH 4/5] Do not collect code for irrelevant frames `vendor/bundle` needs to be blacklisted because if it's inside `root_directory`, then code hunks are collected for every single frame (including gems). The `vendor/bundle` path is not random. This is what Bundler docs say: > Install your dependencies, even gems that are already installed to your system > gems, to a location other than your system's gem repository. In this case, > install them to vendor/bundle. As result, some Heroku users may see code hunks for every single frame, which often causes unwanted truncation of the notice. --- lib/telebugs/notice.rb | 26 +++++---- test/fixtures/{ => project_root}/code.rb | 0 .../fixtures/{ => project_root}/empty_file.rb | 0 test/fixtures/{ => project_root}/long_line.rb | 0 test/fixtures/{ => project_root}/one_line.rb | 0 test/test_code_hunk.rb | 12 ++--- test/test_notice.rb | 53 +++++++++++++++++++ 7 files changed, 76 insertions(+), 15 deletions(-) rename test/fixtures/{ => project_root}/code.rb (100%) rename test/fixtures/{ => project_root}/empty_file.rb (100%) rename test/fixtures/{ => project_root}/long_line.rb (100%) rename test/fixtures/{ => project_root}/one_line.rb (100%) diff --git a/lib/telebugs/notice.rb b/lib/telebugs/notice.rb index 40719f5..df987aa 100644 --- a/lib/telebugs/notice.rb +++ b/lib/telebugs/notice.rb @@ -29,8 +29,6 @@ def to_json(*_args) loop do begin json = @payload.to_json - require 'pp' - pp @payload rescue *JSON_EXCEPTIONS # TODO else @@ -48,17 +46,27 @@ def errors_as_json(error) { type: e.class.name, message: ErrorMessage.parse(e), - backtrace: Backtrace.parse(e).each do |frame| - next unless frame[:file] - next unless File.exist?(frame[:file]) - next unless frame[:line] - - frame[:code] = CodeHunk.get(frame[:file], frame[:line]) - end + backtrace: attach_code(Backtrace.parse(e)) } end end + def attach_code(b) + b.each do |frame| + next unless frame[:file] + next unless File.exist?(frame[:file]) + next unless frame[:line] + next unless frame_belogns_to_root_directory?(frame) + next if frame[:file] =~ %r{vendor/bundle} + + frame[:code] = CodeHunk.get(frame[:file], frame[:line]) + end + end + + def frame_belogns_to_root_directory?(frame) + frame[:file].start_with?(Telebugs::Config.instance.root_directory) + end + def truncate 0 end diff --git a/test/fixtures/code.rb b/test/fixtures/project_root/code.rb similarity index 100% rename from test/fixtures/code.rb rename to test/fixtures/project_root/code.rb diff --git a/test/fixtures/empty_file.rb b/test/fixtures/project_root/empty_file.rb similarity index 100% rename from test/fixtures/empty_file.rb rename to test/fixtures/project_root/empty_file.rb diff --git a/test/fixtures/long_line.rb b/test/fixtures/project_root/long_line.rb similarity index 100% rename from test/fixtures/long_line.rb rename to test/fixtures/project_root/long_line.rb diff --git a/test/fixtures/one_line.rb b/test/fixtures/project_root/one_line.rb similarity index 100% rename from test/fixtures/one_line.rb rename to test/fixtures/project_root/one_line.rb diff --git a/test/test_code_hunk.rb b/test/test_code_hunk.rb index 5f23b9b..4a74642 100644 --- a/test/test_code_hunk.rb +++ b/test/test_code_hunk.rb @@ -4,13 +4,13 @@ class TestCodeHunk < Minitest::Test def test_get_when_file_is_empty - hunk = Telebugs::CodeHunk.get("test/fixtures/empty_file.rb", 1) + hunk = Telebugs::CodeHunk.get("test/fixtures/project_root/empty_file.rb", 1) assert_equal({start_line: 0, lines: []}, hunk) end def test_get_when_file_has_one_line - hunk = Telebugs::CodeHunk.get("test/fixtures/one_line.rb", 1) + hunk = Telebugs::CodeHunk.get("test/fixtures/project_root/one_line.rb", 1) assert_equal( { @@ -22,7 +22,7 @@ def test_get_when_file_has_one_line end def test_get - hunk = Telebugs::CodeHunk.get("test/fixtures/code.rb", 18) + hunk = Telebugs::CodeHunk.get("test/fixtures/project_root/code.rb", 18) assert_equal( { @@ -40,7 +40,7 @@ def test_get end def test_get_with_edge_case_first_line - hunk = Telebugs::CodeHunk.get("test/fixtures/code.rb", 1) + hunk = Telebugs::CodeHunk.get("test/fixtures/project_root/code.rb", 1) assert_equal( { @@ -56,7 +56,7 @@ def test_get_with_edge_case_first_line end def test_get_with_edge_case_last_line - hunk = Telebugs::CodeHunk.get("test/fixtures/code.rb", 39) + hunk = Telebugs::CodeHunk.get("test/fixtures/project_root/code.rb", 39) assert_equal( { @@ -72,7 +72,7 @@ def test_get_with_edge_case_last_line end def test_get_when_code_line_is_too_long - hunk = Telebugs::CodeHunk.get("test/fixtures/long_line.rb", 1) + hunk = Telebugs::CodeHunk.get("test/fixtures/project_root/long_line.rb", 1) assert_equal( { diff --git a/test/test_notice.rb b/test/test_notice.rb index 4b6f091..1a8f6ee 100644 --- a/test/test_notice.rb +++ b/test/test_notice.rb @@ -3,6 +3,24 @@ require "test_helper" class TestNotice < Minitest::Test + def fixture_path(filename) + File.expand_path(File.join('test', 'fixtures', filename)) + end + + def project_root_path(filename) + fixture_path(File.join('project_root', filename)) + end + + def setup + Telebugs.configure do |c| + c.root_directory = project_root_path('') + end + end + + def teardown + Telebugs::Config.instance.reset + end + def test_to_json_with_nested_errors begin raise StandardError.new("error 1") @@ -22,10 +40,45 @@ def test_to_json_with_nested_errors assert_equal "error 2", error1["message"] assert error1.key?("backtrace") assert error1["backtrace"].size > 0 + assert_nil error1["backtrace"][0]["code"] assert_equal "StandardError", error2["type"] assert_equal "error 1", error2["message"] assert error2.key?("backtrace") assert error2["backtrace"].size > 0 + assert_nil error2["backtrace"][0]["code"] + end + + def test_to_json_code + error = RuntimeError.new + error.set_backtrace([ + "#{project_root_path('code.rb')}:18:in `start'", + fixture_path("notroot.txt:3:in `pineapple'"), + "#{project_root_path('vendor/bundle/ignored_file.rb')}:2:in `ignore_me'", + ]) + + + n = Telebugs::Notice.new(error) + json = JSON.parse(n.to_json) + backtrace = json["errors"][0]["backtrace"] + + assert_equal 3, backtrace.size + + assert_equal( + { + "start_line" => 16, + "lines" => [ + " end", + "", + " def start", + " loop do", + " print \"You: \"" + ] + }, + backtrace[0]["code"] + ) + + assert_nil backtrace[1]["code"] + assert_nil backtrace[2]["code"] end end From 525e1c87d738523a28e4a8d828eefdda723a15e4 Mon Sep 17 00:00:00 2001 From: Kyrylo Silin Date: Sat, 8 Jun 2024 21:56:04 +0300 Subject: [PATCH 5/5] StandardRB fixes --- lib/telebugs/code_hunk.rb | 4 ++-- lib/telebugs/config.rb | 4 ++-- lib/telebugs/notice.rb | 2 +- test/test_file_cache.rb | 6 +++--- test/test_notice.rb | 11 +++++------ 5 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/telebugs/code_hunk.rb b/lib/telebugs/code_hunk.rb index fa00e35..3f386e8 100644 --- a/lib/telebugs/code_hunk.rb +++ b/lib/telebugs/code_hunk.rb @@ -13,7 +13,7 @@ def self.get(file, line) start_line = [line - AROUND_LINES, 1].max lines = get_lines(file, start_line, line + AROUND_LINES) - return { start_line: 0, lines: [] } if lines.empty? + return {start_line: 0, lines: []} if lines.empty? { start_line: start_line, @@ -37,7 +37,7 @@ def self.get(file, line) private_class_method def self.get_from_cache(file) Telebugs::FileCache[file] ||= File.foreach(file) - rescue StandardError + rescue nil end end diff --git a/lib/telebugs/config.rb b/lib/telebugs/config.rb index bb22e30..a0c6274 100644 --- a/lib/telebugs/config.rb +++ b/lib/telebugs/config.rb @@ -7,7 +7,7 @@ class Config ERROR_API_URL = "https://api.telebugs.com/2024-03-28/errors" attr_accessor :api_key, - :root_directory + :root_directory attr_reader :api_url @@ -32,7 +32,7 @@ def reset self.api_url = ERROR_API_URL self.root_directory = File.realpath( (defined?(Bundler) && Bundler.root) || - Dir.pwd, + Dir.pwd ) end end diff --git a/lib/telebugs/notice.rb b/lib/telebugs/notice.rb index df987aa..4d62f03 100644 --- a/lib/telebugs/notice.rb +++ b/lib/telebugs/notice.rb @@ -57,7 +57,7 @@ def attach_code(b) next unless File.exist?(frame[:file]) next unless frame[:line] next unless frame_belogns_to_root_directory?(frame) - next if frame[:file] =~ %r{vendor/bundle} + next if %r{vendor/bundle}.match?(frame[:file]) frame[:code] = CodeHunk.get(frame[:file], frame[:line]) end diff --git a/test/test_file_cache.rb b/test/test_file_cache.rb index 56b4426..b8ae010 100644 --- a/test/test_file_cache.rb +++ b/test/test_file_cache.rb @@ -14,17 +14,17 @@ def test_set_when_cache_limit_is_not_reached end assert_equal "value0", Telebugs::FileCache["key0"] - assert_equal "value#{max_size-1}", Telebugs::FileCache["key#{max_size-1}"] + assert_equal "value#{max_size - 1}", Telebugs::FileCache["key#{max_size - 1}"] end def test_set_when_cache_over_limit - max_size = 2*Telebugs::FileCache::MAX_SIZE + max_size = 2 * Telebugs::FileCache::MAX_SIZE max_size.times do |i| Telebugs::FileCache["key#{i}"] = "value#{i}" end assert_nil Telebugs::FileCache["key49"] assert_equal "value50", Telebugs::FileCache["key50"] - assert_equal "value#{max_size-1}", Telebugs::FileCache["key#{max_size-1}"] + assert_equal "value#{max_size - 1}", Telebugs::FileCache["key#{max_size - 1}"] end end diff --git a/test/test_notice.rb b/test/test_notice.rb index 1a8f6ee..3aabe78 100644 --- a/test/test_notice.rb +++ b/test/test_notice.rb @@ -4,16 +4,16 @@ class TestNotice < Minitest::Test def fixture_path(filename) - File.expand_path(File.join('test', 'fixtures', filename)) + File.expand_path(File.join("test", "fixtures", filename)) end def project_root_path(filename) - fixture_path(File.join('project_root', filename)) + fixture_path(File.join("project_root", filename)) end def setup Telebugs.configure do |c| - c.root_directory = project_root_path('') + c.root_directory = project_root_path("") end end @@ -52,12 +52,11 @@ def test_to_json_with_nested_errors def test_to_json_code error = RuntimeError.new error.set_backtrace([ - "#{project_root_path('code.rb')}:18:in `start'", + "#{project_root_path("code.rb")}:18:in `start'", fixture_path("notroot.txt:3:in `pineapple'"), - "#{project_root_path('vendor/bundle/ignored_file.rb')}:2:in `ignore_me'", + "#{project_root_path("vendor/bundle/ignored_file.rb")}:2:in `ignore_me'" ]) - n = Telebugs::Notice.new(error) json = JSON.parse(n.to_json) backtrace = json["errors"][0]["backtrace"]