From 71c37c7ea6db3a9ad020440300b1945554678213 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sat, 20 Feb 2021 23:41:29 +0800 Subject: [PATCH 01/14] Move exception interface's initialization logic to it class --- sentry-ruby/lib/sentry/event.rb | 27 ++++++++----------- .../lib/sentry/interfaces/exception.rb | 4 ++- .../lib/sentry/interfaces/single_exception.rb | 12 +++++---- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index f05038496..782ced0f7 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -125,24 +125,19 @@ def add_exception_interface(exc) @extra.merge!(exc.sentry_context) end - @exception = Sentry::ExceptionInterface.new.tap do |exc_int| - exceptions = Sentry::Utils::ExceptionCauseChain.exception_to_array(exc).reverse - backtraces = Set.new - exc_int.values = exceptions.map do |e| - SingleExceptionInterface.new.tap do |int| - int.type = e.class.to_s - int.value = e.message.byteslice(0..MAX_MESSAGE_SIZE_IN_BYTES) - int.module = e.class.to_s.split('::')[0...-1].join('::') - int.thread_id = Thread.current.object_id - - int.stacktrace = - if e.backtrace && !backtraces.include?(e.backtrace.object_id) - backtraces << e.backtrace.object_id - initialize_stacktrace_interface(e.backtrace) - end + exceptions = Sentry::Utils::ExceptionCauseChain.exception_to_array(exc).reverse + backtraces = Set.new + + exceptions = exceptions.map do |e| + stacktrace = + if e.backtrace && !backtraces.include?(e.backtrace.object_id) + backtraces << e.backtrace.object_id + initialize_stacktrace_interface(e.backtrace) end - end + SingleExceptionInterface.new(e, stacktrace) end + + @exception = Sentry::ExceptionInterface.new(exceptions) end def initialize_stacktrace_interface(backtrace) diff --git a/sentry-ruby/lib/sentry/interfaces/exception.rb b/sentry-ruby/lib/sentry/interfaces/exception.rb index c498f10df..ae9401788 100644 --- a/sentry-ruby/lib/sentry/interfaces/exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/exception.rb @@ -1,6 +1,8 @@ module Sentry class ExceptionInterface < Interface - attr_accessor :values + def initialize(values) + @values = values + end def to_hash data = super diff --git a/sentry-ruby/lib/sentry/interfaces/single_exception.rb b/sentry-ruby/lib/sentry/interfaces/single_exception.rb index 6a2c796d2..d6764fb2b 100644 --- a/sentry-ruby/lib/sentry/interfaces/single_exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/single_exception.rb @@ -1,10 +1,12 @@ module Sentry class SingleExceptionInterface < Interface - attr_accessor :type - attr_accessor :value - attr_accessor :module - attr_accessor :thread_id - attr_accessor :stacktrace + def initialize(exception, stacktrace) + @type = exception.class.to_s + @value = exception.message.byteslice(0..Event::MAX_MESSAGE_SIZE_IN_BYTES) + @module = exception.class.to_s.split('::')[0...-1].join('::') + @thread_id = Thread.current.object_id + @stacktrace = stacktrace + end def to_hash data = super From 6ecf6f00c7f98c0ed730a3baa1647dc2ce67d53f Mon Sep 17 00:00:00 2001 From: st0012 Date: Sat, 20 Feb 2021 23:46:33 +0800 Subject: [PATCH 02/14] Remove unnecessary ivar usage --- sentry-ruby/lib/sentry/interfaces/stacktrace.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb index d4983a358..8bcddc604 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb @@ -3,7 +3,6 @@ class StacktraceInterface attr_reader :frames def initialize(backtrace:, project_root:, app_dirs_pattern:, linecache:, context_lines:, backtrace_cleanup_callback: nil) - @project_root = project_root @frames = [] parsed_backtrace_lines = Backtrace.parse( @@ -23,7 +22,7 @@ def to_hash private def convert_parsed_line_into_frame(line, project_root, linecache, context_lines) - frame = StacktraceInterface::Frame.new(@project_root, line) + frame = StacktraceInterface::Frame.new(project_root, line) frame.set_context(linecache, context_lines) if context_lines frame end From 6d3476a6885c3252ad95d55fce771245f6f2a1e9 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sat, 20 Feb 2021 23:51:43 +0800 Subject: [PATCH 03/14] Add StacktraceBuilder for building stacktrace This moves the construction logic away from StacktraceInterface and Event classes. It allows users to customize stacktrace construction easier. --- sentry-ruby/lib/sentry/configuration.rb | 12 +++++ sentry-ruby/lib/sentry/event.rb | 9 +--- .../lib/sentry/interfaces/stacktrace.rb | 19 +------ .../sentry/interfaces/stacktrace_builder.rb | 34 +++++++++++++ sentry-ruby/spec/sentry/event_spec.rb | 46 ----------------- .../interfaces/stacktrace_builder_spec.rb | 49 +++++++++++++++++++ 6 files changed, 98 insertions(+), 71 deletions(-) create mode 100644 sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb create mode 100644 sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 7ada6e687..0e833ffaa 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -4,6 +4,7 @@ require "sentry/dsn" require "sentry/transport/configuration" require "sentry/linecache" +require "sentry/interfaces/stacktrace_builder" module Sentry class Configuration @@ -195,6 +196,7 @@ def initialize @transport = Transport::Configuration.new @gem_specs = Hash[Gem::Specification.map { |spec| [spec.name, spec.version.to_s] }] if Gem::Specification.respond_to?(:map) + run_post_initialization_callbacks end @@ -290,6 +292,16 @@ def tracing_enabled? !!((@traces_sample_rate && @traces_sample_rate > 0.0) || @traces_sampler) end + def stacktrace_builder + @stacktrace_builder ||= StacktraceBuilder.new( + project_root: @project_root.to_s, + app_dirs_pattern: @app_dirs_pattern, + linecache: @linecache, + context_lines: @context_lines, + backtrace_cleanup_callback: @backtrace_cleanup_callback + ) + end + private def detect_release diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index 782ced0f7..c10d204bd 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -141,14 +141,7 @@ def add_exception_interface(exc) end def initialize_stacktrace_interface(backtrace) - StacktraceInterface.new( - backtrace: backtrace, - project_root: configuration.project_root.to_s, - app_dirs_pattern: configuration.app_dirs_pattern, - linecache: configuration.linecache, - context_lines: configuration.context_lines, - backtrace_cleanup_callback: configuration.backtrace_cleanup_callback - ) + configuration.stacktrace_builder.build(backtrace) end private diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb index 8bcddc604..f677b1e22 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb @@ -2,17 +2,8 @@ module Sentry class StacktraceInterface attr_reader :frames - def initialize(backtrace:, project_root:, app_dirs_pattern:, linecache:, context_lines:, backtrace_cleanup_callback: nil) - @frames = [] - - parsed_backtrace_lines = Backtrace.parse( - backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_callback - ).lines - - parsed_backtrace_lines.reverse.each_with_object(@frames) do |line, frames| - frame = convert_parsed_line_into_frame(line, project_root, linecache, context_lines) - frames << frame if frame.filename - end + def initialize(frames) + @frames = frames end def to_hash @@ -21,12 +12,6 @@ def to_hash private - def convert_parsed_line_into_frame(line, project_root, linecache, context_lines) - frame = StacktraceInterface::Frame.new(project_root, line) - frame.set_context(linecache, context_lines) if context_lines - frame - end - # Not actually an interface, but I want to use the same style class Frame < Interface attr_accessor :abs_path, :context_line, :function, :in_app, diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb new file mode 100644 index 000000000..c2406d700 --- /dev/null +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb @@ -0,0 +1,34 @@ +module Sentry + class StacktraceBuilder + attr_reader :project_root, :app_dirs_pattern, :linecache, :context_lines, :backtrace_cleanup_callback + + def initialize(project_root:, app_dirs_pattern:, linecache:, context_lines:, backtrace_cleanup_callback: nil) + @project_root = project_root + @app_dirs_pattern = app_dirs_pattern + @linecache = linecache + @context_lines = context_lines + @backtrace_cleanup_callback = backtrace_cleanup_callback + end + + def build(backtrace) + parsed_backtrace_lines = Backtrace.parse( + backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_callback + ).lines + + frames = [] + + parsed_backtrace_lines.reverse.each_with_object(frames) do |line, frames| + frame = convert_parsed_line_into_frame(line, project_root, linecache, context_lines) + frames << frame if frame.filename + end + + StacktraceInterface.new(frames) + end + + def convert_parsed_line_into_frame(line, project_root, linecache, context_lines) + frame = StacktraceInterface::Frame.new(project_root, line) + frame.set_context(linecache, context_lines) if context_lines + frame + end + end +end diff --git a/sentry-ruby/spec/sentry/event_spec.rb b/sentry-ruby/spec/sentry/event_spec.rb index 919b2f50e..db2bd8b1a 100644 --- a/sentry-ruby/spec/sentry/event_spec.rb +++ b/sentry-ruby/spec/sentry/event_spec.rb @@ -136,52 +136,6 @@ end end - describe "#initialize_stacktrace_interface" do - let(:fixture_root) { File.join(Dir.pwd, "spec", "support") } - let(:fixture_file) { File.join(fixture_root, "stacktrace_test_fixture.rb") } - let(:configuration) do - Sentry::Configuration.new.tap do |config| - config.project_root = fixture_root - end - end - - let(:backtrace) do - [ - "#{fixture_file}:6:in `bar'", - "#{fixture_file}:2:in `foo'" - ] - end - - subject do - described_class.new(configuration: configuration) - end - - it "returns an array of StacktraceInterface::Frames with correct information" do - interface = subject.initialize_stacktrace_interface(backtrace) - expect(interface).to be_a(Sentry::StacktraceInterface) - - frames = interface.frames - - first_frame = frames.first - - expect(first_frame.filename).to match(/stacktrace_test_fixture.rb/) - expect(first_frame.function).to eq("foo") - expect(first_frame.lineno).to eq(2) - expect(first_frame.pre_context).to eq([nil, nil, "def foo\n"]) - expect(first_frame.context_line).to eq(" bar\n") - expect(first_frame.post_context).to eq(["end\n", "\n", "def bar\n"]) - - second_frame = frames.last - - expect(second_frame.filename).to match(/stacktrace_test_fixture.rb/) - expect(second_frame.function).to eq("bar") - expect(second_frame.lineno).to eq(6) - expect(second_frame.pre_context).to eq(["end\n", "\n", "def bar\n"]) - expect(second_frame.context_line).to eq(" baz\n") - expect(second_frame.post_context).to eq(["end\n", nil, nil]) - end - end - describe '#to_json_compatible' do subject do Sentry::Event.new(configuration: configuration).tap do |event| diff --git a/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb b/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb new file mode 100644 index 000000000..9b4435fa1 --- /dev/null +++ b/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +RSpec.describe Sentry::StacktraceBuilder do + describe "#build" do + let(:fixture_root) { File.join(Dir.pwd, "spec", "support") } + let(:fixture_file) { File.join(fixture_root, "stacktrace_test_fixture.rb") } + let(:configuration) do + Sentry::Configuration.new.tap do |config| + config.project_root = fixture_root + end + end + + let(:backtrace) do + [ + "#{fixture_file}:6:in `bar'", + "#{fixture_file}:2:in `foo'" + ] + end + + subject do + configuration.stacktrace_builder + end + + it "returns an array of StacktraceInterface::Frames with correct information" do + interface = subject.build(backtrace) + expect(interface).to be_a(Sentry::StacktraceInterface) + + frames = interface.frames + + first_frame = frames.first + + expect(first_frame.filename).to match(/stacktrace_test_fixture.rb/) + expect(first_frame.function).to eq("foo") + expect(first_frame.lineno).to eq(2) + expect(first_frame.pre_context).to eq([nil, nil, "def foo\n"]) + expect(first_frame.context_line).to eq(" bar\n") + expect(first_frame.post_context).to eq(["end\n", "\n", "def bar\n"]) + + second_frame = frames.last + + expect(second_frame.filename).to match(/stacktrace_test_fixture.rb/) + expect(second_frame.function).to eq("bar") + expect(second_frame.lineno).to eq(6) + expect(second_frame.pre_context).to eq(["end\n", "\n", "def bar\n"]) + expect(second_frame.context_line).to eq(" baz\n") + expect(second_frame.post_context).to eq(["end\n", nil, nil]) + end + end +end From de1751e4b1c63826f9975773eb020ed2be3c5126 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 14:14:28 +0800 Subject: [PATCH 04/14] Add ExceptionInterface.build --- sentry-ruby/lib/sentry/event.rb | 26 ++++--------------- .../lib/sentry/interfaces/exception.rb | 16 ++++++++++++ 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index c10d204bd..f823ad255 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -117,31 +117,15 @@ def add_request_interface(env) def add_threads_interface(backtrace: nil, **options) @threads = ThreadsInterface.new(**options) - @threads.stacktrace = initialize_stacktrace_interface(backtrace) if backtrace + @threads.stacktrace = configuration.stacktrace_builder.build(caller) if backtrace end - def add_exception_interface(exc) - if exc.respond_to?(:sentry_context) - @extra.merge!(exc.sentry_context) + def add_exception_interface(exception) + if exception.respond_to?(:sentry_context) + @extra.merge!(exception.sentry_context) end - exceptions = Sentry::Utils::ExceptionCauseChain.exception_to_array(exc).reverse - backtraces = Set.new - - exceptions = exceptions.map do |e| - stacktrace = - if e.backtrace && !backtraces.include?(e.backtrace.object_id) - backtraces << e.backtrace.object_id - initialize_stacktrace_interface(e.backtrace) - end - SingleExceptionInterface.new(e, stacktrace) - end - - @exception = Sentry::ExceptionInterface.new(exceptions) - end - - def initialize_stacktrace_interface(backtrace) - configuration.stacktrace_builder.build(backtrace) + @exception = Sentry::ExceptionInterface.build(exception: exception, stacktrace_builder: configuration.stacktrace_builder) end private diff --git a/sentry-ruby/lib/sentry/interfaces/exception.rb b/sentry-ruby/lib/sentry/interfaces/exception.rb index ae9401788..c78cc25a3 100644 --- a/sentry-ruby/lib/sentry/interfaces/exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/exception.rb @@ -9,5 +9,21 @@ def to_hash data[:values] = data[:values].map(&:to_hash) if data[:values] data end + + def self.build(exception:, stacktrace_builder:) + exceptions = Sentry::Utils::ExceptionCauseChain.exception_to_array(exception).reverse + backtraces = Set.new + + exceptions = exceptions.map do |e| + stacktrace = + if e.backtrace && !backtraces.include?(e.backtrace.object_id) + backtraces << e.backtrace.object_id + stacktrace_builder.build(e.backtrace) + end + SingleExceptionInterface.new(e, stacktrace) + end + + new(exceptions) + end end end From 66b3236c9eb5ff8f178e4dfd1d671a9cdf606e71 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 14:29:59 +0800 Subject: [PATCH 05/14] Rename RequestInterface.from_rack to .build --- sentry-ruby/lib/sentry/event.rb | 2 +- sentry-ruby/lib/sentry/interfaces/request.rb | 2 +- .../interfaces/request_interface_spec.rb | 38 +++++++++---------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index f823ad255..f1a9b808e 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -112,7 +112,7 @@ def to_json_compatible end def add_request_interface(env) - @request = Sentry::RequestInterface.from_rack(env) + @request = Sentry::RequestInterface.build(env) end def add_threads_interface(backtrace: nil, **options) diff --git a/sentry-ruby/lib/sentry/interfaces/request.rb b/sentry-ruby/lib/sentry/interfaces/request.rb index fd412d4f6..9773426f9 100644 --- a/sentry-ruby/lib/sentry/interfaces/request.rb +++ b/sentry-ruby/lib/sentry/interfaces/request.rb @@ -17,7 +17,7 @@ class RequestInterface < Interface attr_accessor :url, :method, :data, :query_string, :cookies, :headers, :env - def self.from_rack(env) + def self.build(env) env = clean_env(env) req = ::Rack::Request.new(env) self.new(req) diff --git a/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb b/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb index a1992c03b..c745c35c9 100644 --- a/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb +++ b/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb @@ -6,7 +6,7 @@ let(:exception) { ZeroDivisionError.new("divided by 0") } let(:additional_headers) { {} } let(:env) { Rack::MockRequest.env_for("/test", additional_headers) } - let(:interface) { described_class.from_rack(env) } + let(:interface) { described_class.build(env) } before do Sentry.init do |config| @@ -18,7 +18,7 @@ it 'excludes non whitelisted params from rack env' do additional_env = { "random_param" => "text", "query_string" => "test" } new_env = env.merge(additional_env) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.env).to_not include(additional_env) end @@ -27,14 +27,14 @@ Sentry.configuration.rack_env_whitelist = %w(random_param query_string) additional_env = { "random_param" => "text", "query_string" => "test" } new_env = env.merge(additional_env) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.env).to eq(additional_env) end it 'keeps the original env intact when an empty whitelist is provided' do Sentry.configuration.rack_env_whitelist = [] - interface = described_class.from_rack(env) + interface = described_class.build(env) expect(interface.env).to eq(env) end @@ -44,7 +44,7 @@ let(:additional_headers) { { "HTTP_VERSION" => "HTTP/1.1", "HTTP_COOKIE" => "test", "HTTP_X_REQUEST_ID" => "12345678" } } it 'transforms headers to conform with the interface' do - interface = described_class.from_rack(env) + interface = described_class.build(env) expect(interface.headers).to eq("Content-Length" => "0", "Version" => "HTTP/1.1", "X-Request-Id" => "12345678") end @@ -53,7 +53,7 @@ let(:additional_headers) { { "action_dispatch.request_id" => "12345678" } } it 'transforms headers to conform with the interface' do - interface = described_class.from_rack(env) + interface = described_class.build(env) expect(interface.headers).to eq("Content-Length" => "0", "X-Request-Id" => "12345678") end @@ -66,7 +66,7 @@ it 'does not call #to_s for unnecessary env variables' do expect(mock).not_to receive(:to_s) - interface = described_class.from_rack(env) + interface = described_class.build(env) end end end @@ -76,7 +76,7 @@ ::Rack::RACK_REQUEST_COOKIE_HASH => "cookies!" ) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.cookies).to eq(nil) expect(interface.env["COOKIE"]).to eq(nil) @@ -88,7 +88,7 @@ "HTTP_COOKIE" => "cookies!" ) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.headers["Cookie"]).to eq(nil) end @@ -103,7 +103,7 @@ "CONTENT_TYPE" => "text/html" ) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.headers["Content-Length"]).to eq("10") expect(interface.headers["Content-Type"]).to eq("text/html") @@ -112,14 +112,14 @@ it 'does not ignore version headers which do not match SERVER_PROTOCOL' do new_env = env.merge("SERVER_PROTOCOL" => "HTTP/1.1", "HTTP_VERSION" => "HTTP/2.0") - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.headers["Version"]).to eq("HTTP/2.0") end it 'retains any literal "HTTP-" in the actual header name' do new_env = env.merge("HTTP_HTTP_CUSTOM_HTTP_HEADER" => "test") - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.headers).to include("Http-Custom-Http-Header" => "test") end @@ -133,7 +133,7 @@ def to_s new_env = env.merge("HTTP_FOO" => "BAR", "rails_object" => obj) - expect { interface = described_class.from_rack(new_env) }.to_not raise_error + expect { interface = described_class.build(new_env) }.to_not raise_error end end @@ -144,7 +144,7 @@ def to_s ::Rack::RACK_INPUT => StringIO.new("data=ignore me") ) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.data).to eq(nil) end @@ -154,7 +154,7 @@ def to_s it "doesn't store request body by default" do new_env = env.merge(::Rack::RACK_INPUT => StringIO.new("ignore me")) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.data).to eq(nil) end @@ -170,7 +170,7 @@ def to_s ::Rack::RACK_REQUEST_COOKIE_HASH => "cookies!" ) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.cookies).to eq("cookies!") end @@ -181,7 +181,7 @@ def to_s ::Rack::RACK_INPUT => StringIO.new("data=catch me") ) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.data).to eq({ "data" => "catch me" }) end @@ -189,7 +189,7 @@ def to_s it "stores request body" do new_env = env.merge(::Rack::RACK_INPUT => StringIO.new("catch me")) - interface = described_class.from_rack(new_env) + interface = described_class.build(new_env) expect(interface.data).to eq("catch me") end @@ -204,7 +204,7 @@ def to_s "HTTP_X_FORWARDED_FOR" => ip ) - interface = described_class.from_rack(env) + interface = described_class.build(env) expect(interface.env).to include("REMOTE_ADDR") expect(interface.headers.keys).to include("Client-Ip") From 76e6dc5563b0a75de03c3a63b3803599b41f2a1d Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 14:41:48 +0800 Subject: [PATCH 06/14] Align ThreadInterface's construction interface with others' --- sentry-ruby/lib/sentry/event.rb | 7 +++++-- sentry-ruby/lib/sentry/interfaces/threads.rb | 10 +++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index f1a9b808e..d4f57972e 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -116,8 +116,11 @@ def add_request_interface(env) end def add_threads_interface(backtrace: nil, **options) - @threads = ThreadsInterface.new(**options) - @threads.stacktrace = configuration.stacktrace_builder.build(caller) if backtrace + @threads = ThreadsInterface.build( + backtrace: backtrace, + stacktrace_builder: configuration.stacktrace_builder, + **options + ) end def add_exception_interface(exception) diff --git a/sentry-ruby/lib/sentry/interfaces/threads.rb b/sentry-ruby/lib/sentry/interfaces/threads.rb index 09297a2ef..42d329318 100644 --- a/sentry-ruby/lib/sentry/interfaces/threads.rb +++ b/sentry-ruby/lib/sentry/interfaces/threads.rb @@ -1,12 +1,11 @@ module Sentry class ThreadsInterface - attr_accessor :stacktrace - - def initialize(crashed: false) + def initialize(crashed: false, stacktrace: nil) @id = Thread.current.object_id @name = Thread.current.name @current = true @crashed = crashed + @stacktrace = stacktrace end def to_hash @@ -22,5 +21,10 @@ def to_hash ] } end + + def self.build(backtrace:, stacktrace_builder:, **options) + stacktrace = stacktrace_builder.build(backtrace) if backtrace + new(**options, stacktrace: stacktrace) + end end end From edccbe9d67d03287fbda20af2169934db711d9e3 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 14:43:33 +0800 Subject: [PATCH 07/14] Remove unused stacktrace declaration The stacktrace interface should be attached on exception or thread interface, not directly the event. --- sentry-ruby/lib/sentry/event.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index d4f57972e..134325bdd 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -20,7 +20,7 @@ class Event MAX_MESSAGE_SIZE_IN_BYTES = 1024 * 8 attr_accessor(*ATTRIBUTES) - attr_reader :configuration, :request, :exception, :stacktrace, :threads + attr_reader :configuration, :request, :exception, :threads def initialize(configuration:, integration_meta: nil, message: nil) # this needs to go first because some setters rely on configuration @@ -99,7 +99,6 @@ def type def to_hash data = serialize_attributes data[:breadcrumbs] = breadcrumbs.to_hash if breadcrumbs - data[:stacktrace] = stacktrace.to_hash if stacktrace data[:request] = request.to_hash if request data[:exception] = exception.to_hash if exception data[:threads] = threads.to_hash if threads From 08425555e6feabb75f41c71e91c5bbea10a80945 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 15:13:32 +0800 Subject: [PATCH 08/14] Update example app's config --- .../examples/rails-6.0/config/initializers/sentry.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sentry-rails/examples/rails-6.0/config/initializers/sentry.rb b/sentry-rails/examples/rails-6.0/config/initializers/sentry.rb index a620d6c76..571083b5f 100644 --- a/sentry-rails/examples/rails-6.0/config/initializers/sentry.rb +++ b/sentry-rails/examples/rails-6.0/config/initializers/sentry.rb @@ -4,7 +4,9 @@ config.traces_sample_rate = 1.0 # set a float between 0.0 and 1.0 to enable performance monitoring config.dsn = 'https://2fb45f003d054a7ea47feb45898f7649@o447951.ingest.sentry.io/5434472' config.release = `git branch --show-current` - config.async = lambda do |event, hint| - Sentry::SendEventJob.perform_later(event, hint) - end + # you can use the pre-defined job for the async callback + # + # config.async = lambda do |event, hint| + # Sentry::SendEventJob.perform_later(event, hint) + # end end From ad31231bed7ecff6bb4c94a33d59b072a0c6ac3b Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 15:28:17 +0800 Subject: [PATCH 09/14] Refactor StacktraceBuilder --- .../lib/sentry/interfaces/stacktrace_builder.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb index c2406d700..6c0c80c2b 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb @@ -15,17 +15,17 @@ def build(backtrace) backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_callback ).lines - frames = [] - - parsed_backtrace_lines.reverse.each_with_object(frames) do |line, frames| - frame = convert_parsed_line_into_frame(line, project_root, linecache, context_lines) + frames = parsed_backtrace_lines.reverse.each_with_object([]) do |line, frames| + frame = convert_parsed_line_into_frame(line) frames << frame if frame.filename end StacktraceInterface.new(frames) end - def convert_parsed_line_into_frame(line, project_root, linecache, context_lines) + private + + def convert_parsed_line_into_frame(line) frame = StacktraceInterface::Frame.new(project_root, line) frame.set_context(linecache, context_lines) if context_lines frame From 08740aaaab07f672b5df6cd0f6476d5ebb954cff Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 15:39:57 +0800 Subject: [PATCH 10/14] Refactor StacktraceInterface::Frame and StacktraceBuilder --- sentry-ruby/lib/sentry/interfaces/stacktrace.rb | 15 +++++++-------- .../lib/sentry/interfaces/stacktrace_builder.rb | 16 +++++++++------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb index f677b1e22..57dba4dc4 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb @@ -14,22 +14,22 @@ def to_hash # Not actually an interface, but I want to use the same style class Frame < Interface - attr_accessor :abs_path, :context_line, :function, :in_app, - :lineno, :module, :pre_context, :post_context, :vars + attr_reader :abs_path, :context_line, :function, :in_app, :filename, + :lineno, :module, :pre_context, :post_context, :vars def initialize(project_root, line) @project_root = project_root - @abs_path = line.file if line.file + @abs_path = line.file @function = line.method if line.method @lineno = line.number @in_app = line.in_app @module = line.module_name if line.module_name + @filename = compute_filename end - def filename + def compute_filename return if abs_path.nil? - return @filename if instance_variable_defined?(:@filename) prefix = if under_project_root? && in_app @@ -40,19 +40,18 @@ def filename longest_load_path end - @filename = prefix ? abs_path[prefix.to_s.chomp(File::SEPARATOR).length + 1..-1] : abs_path + prefix ? abs_path[prefix.to_s.chomp(File::SEPARATOR).length + 1..-1] : abs_path end def set_context(linecache, context_lines) return unless abs_path - self.pre_context, self.context_line, self.post_context = \ + @pre_context, @context_line, @post_context = \ linecache.get_file_context(abs_path, lineno, context_lines) end def to_hash(*args) data = super(*args) - data[:filename] = filename data.delete(:vars) unless vars && !vars.empty? data.delete(:pre_context) unless pre_context && !pre_context.empty? data.delete(:post_context) unless post_context && !post_context.empty? diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb index 6c0c80c2b..0ab276612 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb @@ -11,13 +11,9 @@ def initialize(project_root:, app_dirs_pattern:, linecache:, context_lines:, bac end def build(backtrace) - parsed_backtrace_lines = Backtrace.parse( - backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_callback - ).lines - - frames = parsed_backtrace_lines.reverse.each_with_object([]) do |line, frames| - frame = convert_parsed_line_into_frame(line) - frames << frame if frame.filename + parsed_lines = parse_backtrace_lines(backtrace).select(&:file) + frames = parsed_lines.reverse.map do |line| + convert_parsed_line_into_frame(line) end StacktraceInterface.new(frames) @@ -30,5 +26,11 @@ def convert_parsed_line_into_frame(line) frame.set_context(linecache, context_lines) if context_lines frame end + + def parse_backtrace_lines(backtrace) + Backtrace.parse( + backtrace, project_root, app_dirs_pattern, &backtrace_cleanup_callback + ).lines + end end end From 4f101f6cfebe600066d472e1abbaba5fc5bbb24b Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 17:05:31 +0800 Subject: [PATCH 11/14] Refactor SingleExceptionInterface's construction logic Users can just patch SingleExceptionInterface.build_with_stacktrace to change an exception's stacktrace. --- sentry-ruby/lib/sentry/interfaces/exception.rb | 14 +++++++------- .../lib/sentry/interfaces/single_exception.rb | 8 +++++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/sentry-ruby/lib/sentry/interfaces/exception.rb b/sentry-ruby/lib/sentry/interfaces/exception.rb index c78cc25a3..2623fcadd 100644 --- a/sentry-ruby/lib/sentry/interfaces/exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/exception.rb @@ -12,15 +12,15 @@ def to_hash def self.build(exception:, stacktrace_builder:) exceptions = Sentry::Utils::ExceptionCauseChain.exception_to_array(exception).reverse - backtraces = Set.new + processed_backtrace_ids = Set.new exceptions = exceptions.map do |e| - stacktrace = - if e.backtrace && !backtraces.include?(e.backtrace.object_id) - backtraces << e.backtrace.object_id - stacktrace_builder.build(e.backtrace) - end - SingleExceptionInterface.new(e, stacktrace) + if e.backtrace && !processed_backtrace_ids.include?(e.backtrace.object_id) + processed_backtrace_ids << e.backtrace.object_id + SingleExceptionInterface.build_with_stacktrace(e, stacktrace_builder: stacktrace_builder) + else + SingleExceptionInterface.new(exception) + end end new(exceptions) diff --git a/sentry-ruby/lib/sentry/interfaces/single_exception.rb b/sentry-ruby/lib/sentry/interfaces/single_exception.rb index d6764fb2b..60660ac39 100644 --- a/sentry-ruby/lib/sentry/interfaces/single_exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/single_exception.rb @@ -1,6 +1,8 @@ module Sentry class SingleExceptionInterface < Interface - def initialize(exception, stacktrace) + attr_reader :type, :value, :module, :thread_id, :stacktrace + + def initialize(exception, stacktrace = nil) @type = exception.class.to_s @value = exception.message.byteslice(0..Event::MAX_MESSAGE_SIZE_IN_BYTES) @module = exception.class.to_s.split('::')[0...-1].join('::') @@ -13,5 +15,9 @@ def to_hash data[:stacktrace] = data[:stacktrace].to_hash if data[:stacktrace] data end + + def self.build_with_stacktrace(exception, stacktrace_builder:) + new(exception, stacktrace_builder.build(exception.backtrace)) + end end end From a4adb6860162f3f4240db794f0681e002db6d149 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 17:38:04 +0800 Subject: [PATCH 12/14] Allow customizing frame with frame_callback ```ruby builder.build(backtrace) do |frame| if frame.module.match?(/a_gem/) nil else frame end end ``` --- .../lib/sentry/interfaces/single_exception.rb | 5 ++- .../lib/sentry/interfaces/stacktrace.rb | 2 +- .../sentry/interfaces/stacktrace_builder.rb | 20 +++++++++-- sentry-ruby/lib/sentry/interfaces/threads.rb | 2 ++ .../interfaces/stacktrace_builder_spec.rb | 36 +++++++++++++++++++ 5 files changed, 60 insertions(+), 5 deletions(-) diff --git a/sentry-ruby/lib/sentry/interfaces/single_exception.rb b/sentry-ruby/lib/sentry/interfaces/single_exception.rb index 60660ac39..784e36fde 100644 --- a/sentry-ruby/lib/sentry/interfaces/single_exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/single_exception.rb @@ -16,8 +16,11 @@ def to_hash data end + # patch this method if you want to change an exception's stacktrace frames + # also see `StacktraceBuilder.build`. def self.build_with_stacktrace(exception, stacktrace_builder:) - new(exception, stacktrace_builder.build(exception.backtrace)) + stacktrace = stacktrace_builder.build(exception.backtrace) + new(exception, stacktrace) end end end diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb index 57dba4dc4..8569793fa 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb @@ -14,7 +14,7 @@ def to_hash # Not actually an interface, but I want to use the same style class Frame < Interface - attr_reader :abs_path, :context_line, :function, :in_app, :filename, + attr_accessor :abs_path, :context_line, :function, :in_app, :filename, :lineno, :module, :pre_context, :post_context, :vars def initialize(project_root, line) diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb index 0ab276612..b98d6e5ec 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb @@ -10,11 +10,25 @@ def initialize(project_root:, app_dirs_pattern:, linecache:, context_lines:, bac @backtrace_cleanup_callback = backtrace_cleanup_callback end - def build(backtrace) + # you can pass a block to customize/exclude frames: + # + # ```ruby + # builder.build(backtrace) do |frame| + # if frame.module.match?(/a_gem/) + # nil + # else + # frame + # end + # end + # ``` + def build(backtrace, &frame_callback) parsed_lines = parse_backtrace_lines(backtrace).select(&:file) + frames = parsed_lines.reverse.map do |line| - convert_parsed_line_into_frame(line) - end + frame = convert_parsed_line_into_frame(line) + frame = frame_callback.call(frame) if frame_callback + frame + end.compact StacktraceInterface.new(frames) end diff --git a/sentry-ruby/lib/sentry/interfaces/threads.rb b/sentry-ruby/lib/sentry/interfaces/threads.rb index 42d329318..4d3f941a7 100644 --- a/sentry-ruby/lib/sentry/interfaces/threads.rb +++ b/sentry-ruby/lib/sentry/interfaces/threads.rb @@ -22,6 +22,8 @@ def to_hash } end + # patch this method if you want to change a threads interface's stacktrace frames + # also see `StacktraceBuilder.build`. def self.build(backtrace:, stacktrace_builder:, **options) stacktrace = stacktrace_builder.build(backtrace) if backtrace new(**options, stacktrace: stacktrace) diff --git a/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb b/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb index 9b4435fa1..6ec106479 100644 --- a/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb +++ b/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb @@ -21,6 +21,11 @@ configuration.stacktrace_builder end + it "ignores frames without filename" do + interface = subject.build([":6:in `foo'"]) + expect(interface.frames).to be_empty + end + it "returns an array of StacktraceInterface::Frames with correct information" do interface = subject.build(backtrace) expect(interface).to be_a(Sentry::StacktraceInterface) @@ -45,5 +50,36 @@ expect(second_frame.context_line).to eq(" baz\n") expect(second_frame.post_context).to eq(["end\n", nil, nil]) end + + context "with block argument" do + it "removes the frame if it's evaluated as nil" do + interface = subject.build(backtrace) do |frame| + nil + end + + expect(interface.frames).to be_empty + end + it "yields frame to the block" do + interface = subject.build(backtrace) do |frame| + frame.vars = { foo: "bar" } + frame + end + + frames = interface.frames + + first_frame = frames.first + + expect(first_frame.filename).to match(/stacktrace_test_fixture.rb/) + expect(first_frame.function).to eq("foo") + expect(first_frame.vars).to eq({ foo: "bar" }) + + second_frame = frames.last + + expect(second_frame.filename).to match(/stacktrace_test_fixture.rb/) + expect(second_frame.function).to eq("bar") + expect(second_frame.lineno).to eq(6) + expect(second_frame.vars).to eq({ foo: "bar" }) + end + end end end From e5f22ad8d406513000a5a20125d4d1ae5839aed3 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 17:52:45 +0800 Subject: [PATCH 13/14] Use kargs in interface's construction methods Keyword arguments give a more clear message on what parameters are expected. This will help extension writers use those classes. --- sentry-ruby/lib/sentry/event.rb | 2 +- .../lib/sentry/interfaces/exception.rb | 8 ++-- sentry-ruby/lib/sentry/interfaces/request.rb | 20 +++++----- .../lib/sentry/interfaces/single_exception.rb | 8 ++-- .../lib/sentry/interfaces/stacktrace.rb | 2 +- .../sentry/interfaces/stacktrace_builder.rb | 4 +- sentry-ruby/lib/sentry/interfaces/threads.rb | 2 +- .../interfaces/request_interface_spec.rb | 38 +++++++++---------- .../interfaces/stacktrace_builder_spec.rb | 8 ++-- 9 files changed, 46 insertions(+), 46 deletions(-) diff --git a/sentry-ruby/lib/sentry/event.rb b/sentry-ruby/lib/sentry/event.rb index 134325bdd..20cf6a4a3 100644 --- a/sentry-ruby/lib/sentry/event.rb +++ b/sentry-ruby/lib/sentry/event.rb @@ -111,7 +111,7 @@ def to_json_compatible end def add_request_interface(env) - @request = Sentry::RequestInterface.build(env) + @request = Sentry::RequestInterface.build(env: env) end def add_threads_interface(backtrace: nil, **options) diff --git a/sentry-ruby/lib/sentry/interfaces/exception.rb b/sentry-ruby/lib/sentry/interfaces/exception.rb index 2623fcadd..057a5e32b 100644 --- a/sentry-ruby/lib/sentry/interfaces/exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/exception.rb @@ -1,6 +1,6 @@ module Sentry class ExceptionInterface < Interface - def initialize(values) + def initialize(values:) @values = values end @@ -17,13 +17,13 @@ def self.build(exception:, stacktrace_builder:) exceptions = exceptions.map do |e| if e.backtrace && !processed_backtrace_ids.include?(e.backtrace.object_id) processed_backtrace_ids << e.backtrace.object_id - SingleExceptionInterface.build_with_stacktrace(e, stacktrace_builder: stacktrace_builder) + SingleExceptionInterface.build_with_stacktrace(exception: e, stacktrace_builder: stacktrace_builder) else - SingleExceptionInterface.new(exception) + SingleExceptionInterface.new(exception: exception) end end - new(exceptions) + new(values: exceptions) end end end diff --git a/sentry-ruby/lib/sentry/interfaces/request.rb b/sentry-ruby/lib/sentry/interfaces/request.rb index 9773426f9..d3f94baa3 100644 --- a/sentry-ruby/lib/sentry/interfaces/request.rb +++ b/sentry-ruby/lib/sentry/interfaces/request.rb @@ -17,10 +17,10 @@ class RequestInterface < Interface attr_accessor :url, :method, :data, :query_string, :cookies, :headers, :env - def self.build(env) + def self.build(env:) env = clean_env(env) - req = ::Rack::Request.new(env) - self.new(req) + request = ::Rack::Request.new(env) + self.new(request: request) end def self.clean_env(env) @@ -34,17 +34,17 @@ def self.clean_env(env) env end - def initialize(req) - env = req.env + def initialize(request:) + env = request.env if Sentry.configuration.send_default_pii - self.data = read_data_from(req) - self.cookies = req.cookies + self.data = read_data_from(request) + self.cookies = request.cookies end - self.url = req.scheme && req.url.split('?').first - self.method = req.request_method - self.query_string = req.query_string + self.url = request.scheme && request.url.split('?').first + self.method = request.request_method + self.query_string = request.query_string self.headers = filter_and_format_headers(env) self.env = filter_and_format_env(env) diff --git a/sentry-ruby/lib/sentry/interfaces/single_exception.rb b/sentry-ruby/lib/sentry/interfaces/single_exception.rb index 784e36fde..4ff68dd17 100644 --- a/sentry-ruby/lib/sentry/interfaces/single_exception.rb +++ b/sentry-ruby/lib/sentry/interfaces/single_exception.rb @@ -2,7 +2,7 @@ module Sentry class SingleExceptionInterface < Interface attr_reader :type, :value, :module, :thread_id, :stacktrace - def initialize(exception, stacktrace = nil) + def initialize(exception:, stacktrace: nil) @type = exception.class.to_s @value = exception.message.byteslice(0..Event::MAX_MESSAGE_SIZE_IN_BYTES) @module = exception.class.to_s.split('::')[0...-1].join('::') @@ -18,9 +18,9 @@ def to_hash # patch this method if you want to change an exception's stacktrace frames # also see `StacktraceBuilder.build`. - def self.build_with_stacktrace(exception, stacktrace_builder:) - stacktrace = stacktrace_builder.build(exception.backtrace) - new(exception, stacktrace) + def self.build_with_stacktrace(exception:, stacktrace_builder:) + stacktrace = stacktrace_builder.build(backtrace: exception.backtrace) + new(exception: exception, stacktrace: stacktrace) end end end diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb index 8569793fa..480f47847 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace.rb @@ -2,7 +2,7 @@ module Sentry class StacktraceInterface attr_reader :frames - def initialize(frames) + def initialize(frames:) @frames = frames end diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb index b98d6e5ec..fe0b63664 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb @@ -21,7 +21,7 @@ def initialize(project_root:, app_dirs_pattern:, linecache:, context_lines:, bac # end # end # ``` - def build(backtrace, &frame_callback) + def build(backtrace:, &frame_callback) parsed_lines = parse_backtrace_lines(backtrace).select(&:file) frames = parsed_lines.reverse.map do |line| @@ -30,7 +30,7 @@ def build(backtrace, &frame_callback) frame end.compact - StacktraceInterface.new(frames) + StacktraceInterface.new(frames: frames) end private diff --git a/sentry-ruby/lib/sentry/interfaces/threads.rb b/sentry-ruby/lib/sentry/interfaces/threads.rb index 4d3f941a7..8c5ee5f69 100644 --- a/sentry-ruby/lib/sentry/interfaces/threads.rb +++ b/sentry-ruby/lib/sentry/interfaces/threads.rb @@ -25,7 +25,7 @@ def to_hash # patch this method if you want to change a threads interface's stacktrace frames # also see `StacktraceBuilder.build`. def self.build(backtrace:, stacktrace_builder:, **options) - stacktrace = stacktrace_builder.build(backtrace) if backtrace + stacktrace = stacktrace_builder.build(backtrace: backtrace) if backtrace new(**options, stacktrace: stacktrace) end end diff --git a/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb b/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb index c745c35c9..cce836ebb 100644 --- a/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb +++ b/sentry-ruby/spec/sentry/interfaces/request_interface_spec.rb @@ -6,7 +6,7 @@ let(:exception) { ZeroDivisionError.new("divided by 0") } let(:additional_headers) { {} } let(:env) { Rack::MockRequest.env_for("/test", additional_headers) } - let(:interface) { described_class.build(env) } + let(:interface) { described_class.build(env: env) } before do Sentry.init do |config| @@ -18,7 +18,7 @@ it 'excludes non whitelisted params from rack env' do additional_env = { "random_param" => "text", "query_string" => "test" } new_env = env.merge(additional_env) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.env).to_not include(additional_env) end @@ -27,14 +27,14 @@ Sentry.configuration.rack_env_whitelist = %w(random_param query_string) additional_env = { "random_param" => "text", "query_string" => "test" } new_env = env.merge(additional_env) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.env).to eq(additional_env) end it 'keeps the original env intact when an empty whitelist is provided' do Sentry.configuration.rack_env_whitelist = [] - interface = described_class.build(env) + interface = described_class.build(env: env) expect(interface.env).to eq(env) end @@ -44,7 +44,7 @@ let(:additional_headers) { { "HTTP_VERSION" => "HTTP/1.1", "HTTP_COOKIE" => "test", "HTTP_X_REQUEST_ID" => "12345678" } } it 'transforms headers to conform with the interface' do - interface = described_class.build(env) + interface = described_class.build(env: env) expect(interface.headers).to eq("Content-Length" => "0", "Version" => "HTTP/1.1", "X-Request-Id" => "12345678") end @@ -53,7 +53,7 @@ let(:additional_headers) { { "action_dispatch.request_id" => "12345678" } } it 'transforms headers to conform with the interface' do - interface = described_class.build(env) + interface = described_class.build(env: env) expect(interface.headers).to eq("Content-Length" => "0", "X-Request-Id" => "12345678") end @@ -66,7 +66,7 @@ it 'does not call #to_s for unnecessary env variables' do expect(mock).not_to receive(:to_s) - interface = described_class.build(env) + interface = described_class.build(env: env) end end end @@ -76,7 +76,7 @@ ::Rack::RACK_REQUEST_COOKIE_HASH => "cookies!" ) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.cookies).to eq(nil) expect(interface.env["COOKIE"]).to eq(nil) @@ -88,7 +88,7 @@ "HTTP_COOKIE" => "cookies!" ) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.headers["Cookie"]).to eq(nil) end @@ -103,7 +103,7 @@ "CONTENT_TYPE" => "text/html" ) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.headers["Content-Length"]).to eq("10") expect(interface.headers["Content-Type"]).to eq("text/html") @@ -112,14 +112,14 @@ it 'does not ignore version headers which do not match SERVER_PROTOCOL' do new_env = env.merge("SERVER_PROTOCOL" => "HTTP/1.1", "HTTP_VERSION" => "HTTP/2.0") - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.headers["Version"]).to eq("HTTP/2.0") end it 'retains any literal "HTTP-" in the actual header name' do new_env = env.merge("HTTP_HTTP_CUSTOM_HTTP_HEADER" => "test") - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.headers).to include("Http-Custom-Http-Header" => "test") end @@ -133,7 +133,7 @@ def to_s new_env = env.merge("HTTP_FOO" => "BAR", "rails_object" => obj) - expect { interface = described_class.build(new_env) }.to_not raise_error + expect { interface = described_class.build(env: new_env) }.to_not raise_error end end @@ -144,7 +144,7 @@ def to_s ::Rack::RACK_INPUT => StringIO.new("data=ignore me") ) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.data).to eq(nil) end @@ -154,7 +154,7 @@ def to_s it "doesn't store request body by default" do new_env = env.merge(::Rack::RACK_INPUT => StringIO.new("ignore me")) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.data).to eq(nil) end @@ -170,7 +170,7 @@ def to_s ::Rack::RACK_REQUEST_COOKIE_HASH => "cookies!" ) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.cookies).to eq("cookies!") end @@ -181,7 +181,7 @@ def to_s ::Rack::RACK_INPUT => StringIO.new("data=catch me") ) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.data).to eq({ "data" => "catch me" }) end @@ -189,7 +189,7 @@ def to_s it "stores request body" do new_env = env.merge(::Rack::RACK_INPUT => StringIO.new("catch me")) - interface = described_class.build(new_env) + interface = described_class.build(env: new_env) expect(interface.data).to eq("catch me") end @@ -204,7 +204,7 @@ def to_s "HTTP_X_FORWARDED_FOR" => ip ) - interface = described_class.build(env) + interface = described_class.build(env: env) expect(interface.env).to include("REMOTE_ADDR") expect(interface.headers.keys).to include("Client-Ip") diff --git a/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb b/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb index 6ec106479..d1cc5257a 100644 --- a/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb +++ b/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb @@ -22,12 +22,12 @@ end it "ignores frames without filename" do - interface = subject.build([":6:in `foo'"]) + interface = subject.build(backtrace: [":6:in `foo'"]) expect(interface.frames).to be_empty end it "returns an array of StacktraceInterface::Frames with correct information" do - interface = subject.build(backtrace) + interface = subject.build(backtrace: backtrace) expect(interface).to be_a(Sentry::StacktraceInterface) frames = interface.frames @@ -53,14 +53,14 @@ context "with block argument" do it "removes the frame if it's evaluated as nil" do - interface = subject.build(backtrace) do |frame| + interface = subject.build(backtrace: backtrace) do |frame| nil end expect(interface.frames).to be_empty end it "yields frame to the block" do - interface = subject.build(backtrace) do |frame| + interface = subject.build(backtrace: backtrace) do |frame| frame.vars = { foo: "bar" } frame end From 881837303dbd3a3688165c4413ad314b7875b831 Mon Sep 17 00:00:00 2001 From: st0012 Date: Sun, 21 Feb 2021 21:52:35 +0800 Subject: [PATCH 14/14] Update changelog --- sentry-ruby/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sentry-ruby/CHANGELOG.md b/sentry-ruby/CHANGELOG.md index 09f6f41ec..6c8a75c3d 100644 --- a/sentry-ruby/CHANGELOG.md +++ b/sentry-ruby/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## Unreleased + +- Refactor interface construction [#1296](https://github.com/getsentry/sentry-ruby/pull/1296) + ## 4.2.2 - Add thread_id to Exception interface [#1291](https://github.com/getsentry/sentry-ruby/pull/1291)