diff --git a/sentry-ruby/Gemfile b/sentry-ruby/Gemfile index 35cc658d2..592fe4858 100644 --- a/sentry-ruby/Gemfile +++ b/sentry-ruby/Gemfile @@ -33,3 +33,4 @@ gem "yard", github: "lsegal/yard" gem "webrick" gem "faraday" gem "excon" +gem "webmock" diff --git a/sentry-ruby/lib/sentry/transport/http_transport.rb b/sentry-ruby/lib/sentry/transport/http_transport.rb index e79bd0634..835867dac 100644 --- a/sentry-ruby/lib/sentry/transport/http_transport.rb +++ b/sentry-ruby/lib/sentry/transport/http_transport.rb @@ -23,7 +23,6 @@ class HTTPTransport < Transport Zlib::BufError, Errno::EHOSTUNREACH, Errno::ECONNREFUSED ].freeze - def initialize(*args) super log_debug("Sentry HTTP Transport will connect to #{@dsn.server}") if @dsn diff --git a/sentry-ruby/spec/contexts/with_request_mock.rb b/sentry-ruby/spec/contexts/with_request_mock.rb index e6c480256..9a326376d 100644 --- a/sentry-ruby/spec/contexts/with_request_mock.rb +++ b/sentry-ruby/spec/contexts/with_request_mock.rb @@ -12,7 +12,13 @@ def setsockopt(*args); end allow(TCPSocket).to receive(:open).and_return(FakeSocket.new) end - def stub_request(fake_response, &block) + around do |example| + WebMock.disable! + example.run + WebMock.enable! + end + + def sentry_stub_request(fake_response, &block) allow_any_instance_of(Net::HTTP).to receive(:connect) allow_any_instance_of(Net::HTTP).to receive(:transport_request) do |http_obj, request| block.call(request, http_obj) if block @@ -32,10 +38,10 @@ def build_fake_response(status, body: {}, headers: {}) def stub_sentry_response # use bad request as an example is easier for verifying with error messages - stub_request(build_fake_response("400", body: { data: "bad sentry DSN public key" })) + sentry_stub_request(build_fake_response("400", body: { data: "bad sentry DSN public key" })) end def stub_normal_response(code: "200", &block) - stub_request(build_fake_response(code), &block) + sentry_stub_request(build_fake_response(code), &block) end end diff --git a/sentry-ruby/spec/sentry/client/event_sending_spec.rb b/sentry-ruby/spec/sentry/client/event_sending_spec.rb index 4950cb7c8..f87821ae9 100644 --- a/sentry-ruby/spec/sentry/client/event_sending_spec.rb +++ b/sentry-ruby/spec/sentry/client/event_sending_spec.rb @@ -10,6 +10,11 @@ config.transport.transport_class = Sentry::DummyTransport end end + + before do + stub_request(:post, Sentry::TestHelper::DUMMY_DSN) + end + subject(:client) { Sentry::Client.new(configuration) } let(:hub) do @@ -470,13 +475,16 @@ before do configuration.background_worker_threads = 0 Sentry.background_worker = Sentry::BackgroundWorker.new(configuration) + + stub_request(:post, "http://sentry.localdomain/sentry/api/42/envelope/") + .to_raise(Timeout::Error) end it "swallows and logs Sentry::ExternalError (caused by transport's networking error)" do expect(client.capture_event(event, scope)).to be_nil - expect(string_io.string).to match(/Event sending failed: Failed to open TCP connection/) - expect(string_io.string).to match(/Event capturing failed: Failed to open TCP connection/) + expect(string_io.string).to match(/Event sending failed: Exception from WebMock/) + expect(string_io.string).to match(/Event capturing failed: Exception from WebMock/) end it "swallows and logs errors caused by the user (like in before_send)" do @@ -502,13 +510,16 @@ context "when sending events in background causes error", retry: 3 do before do Sentry.background_worker = Sentry::BackgroundWorker.new(configuration) + + stub_request(:post, "http://sentry.localdomain/sentry/api/42/envelope/") + .to_raise(Timeout::Error) end it "swallows and logs Sentry::ExternalError (caused by transport's networking error)" do expect(client.capture_event(event, scope)).to be_a(Sentry::ErrorEvent) sleep(0.2) - expect(string_io.string).to match(/Event sending failed: Failed to open TCP connection/) + expect(string_io.string).to match(/Event sending failed: Exception from WebMock/) end it "swallows and logs errors caused by the user (like in before_send)" do @@ -560,11 +571,14 @@ describe "#send_event" do context "error happens when sending the event" do it "raises the error" do + stub_request(:post, "http://sentry.localdomain/sentry/api/42/envelope/") + .to_raise(Timeout::Error) + expect do client.send_event(event) end.to raise_error(Sentry::ExternalError) - expect(string_io.string).to match(/Event sending failed: Failed to open TCP connection/) + expect(string_io.string).to match(/Event sending failed: Exception from WebMock/) end end @@ -635,6 +649,9 @@ end it "records lost span delta client reports" do + stub_request(:post, "http://sentry.localdomain/sentry/api/42/envelope/") + .to_raise(Timeout::Error) + expect { client.send_event(transaction_event) }.to raise_error(Sentry::ExternalError) expect(client.transport).to have_recorded_lost_event(:before_send, 'span', num: 2) end diff --git a/sentry-ruby/spec/sentry/faraday_spec.rb b/sentry-ruby/spec/sentry/faraday_spec.rb index 77e90db0a..d6b98d5a7 100644 --- a/sentry-ruby/spec/sentry/faraday_spec.rb +++ b/sentry-ruby/spec/sentry/faraday_spec.rb @@ -271,7 +271,7 @@ let(:url) { "http://example.com" } - it "skips instrumentation" do + it "skips instrumentation", webmock: false do transaction = Sentry.start_transaction Sentry.get_current_scope.set_span(transaction) diff --git a/sentry-ruby/spec/sentry/net/http_spec.rb b/sentry-ruby/spec/sentry/net/http_spec.rb index 9de0a6f76..3e50f9cbe 100644 --- a/sentry-ruby/spec/sentry/net/http_spec.rb +++ b/sentry-ruby/spec/sentry/net/http_spec.rb @@ -11,38 +11,32 @@ ::Logger.new(string_io) end - context "with IPv6 addresses" do + context "with tracing enabled" do before do perform_basic_setup do |config| config.traces_sample_rate = 1.0 + config.transport.transport_class = Sentry::HTTPTransport + config.logger = logger + # the dsn needs to have a real host so we can make a real connection before sending a failed request + config.dsn = 'http://foobarbaz@o447951.ingest.sentry.io/5434472' end end - it "correctly parses the short-hand IPv6 addresses" do - stub_normal_response - - transaction = Sentry.start_transaction - Sentry.get_current_scope.set_span(transaction) + context "with IPv6 addresses" do + it "correctly parses the short-hand IPv6 addresses" do + stub_normal_response - _ = Net::HTTP.get("::1", "/path", 8080) + transaction = Sentry.start_transaction + Sentry.get_current_scope.set_span(transaction) - expect(transaction.span_recorder.spans.count).to eq(2) + _ = Net::HTTP.get("::1", "/path", 8080) - request_span = transaction.span_recorder.spans.last - expect(request_span.data).to eq( - { "url" => "http://[::1]/path", "http.request.method" => "GET", "http.response.status_code" => 200 } - ) - end - end + expect(transaction.span_recorder.spans.count).to eq(2) - context "with tracing enabled" do - before do - perform_basic_setup do |config| - config.traces_sample_rate = 1.0 - config.transport.transport_class = Sentry::HTTPTransport - config.logger = logger - # the dsn needs to have a real host so we can make a real connection before sending a failed request - config.dsn = 'http://foobarbaz@o447951.ingest.sentry.io/5434472' + request_span = transaction.span_recorder.spans.last + expect(request_span.data).to eq( + { "url" => "http://[::1]/path", "http.request.method" => "GET", "http.response.status_code" => 200 } + ) end end diff --git a/sentry-ruby/spec/sentry/rspec/matchers_spec.rb b/sentry-ruby/spec/sentry/rspec/matchers_spec.rb index 45e914f52..c20a7cfd3 100644 --- a/sentry-ruby/spec/sentry/rspec/matchers_spec.rb +++ b/sentry-ruby/spec/sentry/rspec/matchers_spec.rb @@ -12,6 +12,7 @@ config.dsn = 'https://2fb45f003d054a7ea47feb45898f7649@o447951.ingest.sentry.io/5434472' config.enabled_environments = ["production"] config.environment = :test + config.transport.transport_class = Sentry::DummyTransport end setup_sentry_test diff --git a/sentry-ruby/spec/sentry/transport/http_transport_rate_limiting_spec.rb b/sentry-ruby/spec/sentry/transport/http_transport_rate_limiting_spec.rb index 30dc42bbb..aff057345 100644 --- a/sentry-ruby/spec/sentry/transport/http_transport_rate_limiting_spec.rb +++ b/sentry-ruby/spec/sentry/transport/http_transport_rate_limiting_spec.rb @@ -107,7 +107,7 @@ describe "rate limit header processing" do before do - stub_request(fake_response) + sentry_stub_request(fake_response) end shared_examples "rate limiting headers handling" do diff --git a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb index a0973ef75..b73c7dd93 100644 --- a/sentry-ruby/spec/sentry/transport/http_transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport/http_transport_spec.rb @@ -22,7 +22,7 @@ subject { client.transport } it "logs a debug message only during initialization" do - stub_request(build_fake_response("200")) + sentry_stub_request(build_fake_response("200")) string_io = StringIO.new configuration.logger = Logger.new(string_io) @@ -39,7 +39,7 @@ end it "initializes new Net::HTTP instance for every request" do - stub_request(build_fake_response("200")) do |request| + sentry_stub_request(build_fake_response("200")) do |request| expect(request["User-Agent"]).to eq("sentry-ruby/#{Sentry::VERSION}") end @@ -87,7 +87,7 @@ let(:fake_response) { build_fake_response("200") } it 'sets default User-Agent' do - stub_request(fake_response) do |request| + sentry_stub_request(fake_response) do |request| expect(request["User-Agent"]).to eq("sentry-ruby/#{Sentry::VERSION}") end @@ -97,7 +97,7 @@ it "accepts custom proxy" do configuration.transport.proxy = { uri: URI("https://example.com"), user: "stan", password: "foobar" } - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.proxy_address).to eq("example.com") expect(http_obj.proxy_user).to eq("stan") expect(http_obj.proxy_pass).to eq("foobar") @@ -109,7 +109,7 @@ it "accepts a custom proxy string" do configuration.transport.proxy = "https://stan:foobar@example.com:8080" - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.proxy_address).to eq("example.com") expect(http_obj.proxy_user).to eq("stan") expect(http_obj.proxy_pass).to eq("foobar") @@ -122,7 +122,7 @@ it "accepts a custom proxy URI" do configuration.transport.proxy = URI("https://stan:foobar@example.com:8080") - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.proxy_address).to eq("example.com") expect(http_obj.proxy_user).to eq("stan") expect(http_obj.proxy_pass).to eq("foobar") @@ -136,7 +136,7 @@ begin ENV["http_proxy"] = "https://stan:foobar@example.com:8080" - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.proxy_address).to eq("example.com") expect(http_obj.proxy_port).to eq(8080) @@ -155,7 +155,7 @@ it "accepts custom timeout" do configuration.transport.timeout = 10 - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.read_timeout).to eq(10) if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("2.6") @@ -169,7 +169,7 @@ it "accepts custom open_timeout" do configuration.transport.open_timeout = 10 - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.open_timeout).to eq(10) end @@ -178,7 +178,7 @@ describe "ssl configurations" do it "has the corrent default" do - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.verify_mode).to eq(1) expect(http_obj.ca_file).to eq(nil) end @@ -189,7 +189,7 @@ it "accepts custom ssl_verification configuration" do configuration.transport.ssl_verification = false - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.verify_mode).to eq(0) expect(http_obj.ca_file).to eq(nil) end @@ -200,7 +200,7 @@ it "accepts custom ssl_ca_file configuration" do configuration.transport.ssl_ca_file = "/tmp/foo" - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.verify_mode).to eq(1) expect(http_obj.ca_file).to eq("/tmp/foo") end @@ -211,7 +211,7 @@ it "accepts custom ssl configuration" do configuration.transport.ssl = { verify: false, ca_file: "/tmp/foo" } - stub_request(fake_response) do |_, http_obj| + sentry_stub_request(fake_response) do |_, http_obj| expect(http_obj.verify_mode).to eq(0) expect(http_obj.ca_file).to eq("/tmp/foo") end @@ -225,7 +225,7 @@ let(:fake_response) { build_fake_response("200") } it "compresses data by default" do - stub_request(fake_response) do |request| + sentry_stub_request(fake_response) do |request| expect(request["Content-Type"]).to eq("application/x-sentry-envelope") expect(request["Content-Encoding"]).to eq("gzip") @@ -238,7 +238,7 @@ end it "doesn't compress small event" do - stub_request(fake_response) do |request| + sentry_stub_request(fake_response) do |request| expect(request["Content-Type"]).to eq("application/x-sentry-envelope") expect(request["Content-Encoding"]).to eq("") @@ -255,7 +255,7 @@ it "doesn't compress data if the encoding is not gzip" do configuration.transport.encoding = "json" - stub_request(fake_response) do |request| + sentry_stub_request(fake_response) do |request| expect(request["Content-Type"]).to eq("application/x-sentry-envelope") expect(request["Content-Encoding"]).to eq("") @@ -296,7 +296,7 @@ let(:fake_response) { build_fake_response("404") } it "raises an error" do - stub_request(fake_response) + sentry_stub_request(fake_response) expect { subject.send_data(data) }.to raise_error(Sentry::ExternalError, /the server responded with status 404/) end @@ -306,7 +306,7 @@ let(:fake_response) { build_fake_response("500") } it "raises an error" do - stub_request(fake_response) + sentry_stub_request(fake_response) expect { subject.send_data(data) }.to raise_error(Sentry::ExternalError, /the server responded with status 500/) end @@ -318,7 +318,7 @@ end it "raises an error with header" do - stub_request(error_response) + sentry_stub_request(error_response) expect { subject.send_data(data) }.to raise_error(Sentry::ExternalError, /error_in_header/) end diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb index 059a8109b..bcef588fe 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport_spec.rb @@ -531,11 +531,14 @@ end end - context "with Faraday::Error" do - it "raises the error" do - expect do - subject.send_event(event) - end.to raise_error(Sentry::ExternalError) + context "with an HTTP exception" do + before do + stub_request(:post, "http://sentry.localdomain:80/sentry/api/42/envelope/"). + to_raise(Timeout::Error) + end + + it "raises Sentry::ExternalError" do + expect { subject.send_event(event) }.to raise_error(Sentry::ExternalError) end end end diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index 9a188e9f3..856e37d52 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -183,8 +183,8 @@ context "with spotlight" do before { perform_basic_setup { |c| c.spotlight = true } } - it "sends the event to spotlight too" do - stub_request(build_fake_response("200")) do |request, http_obj| + it "sends the event to spotlight too", webmock: false do + sentry_stub_request(build_fake_response("200")) do |request, http_obj| expect(request["Content-Type"]).to eq("application/x-sentry-envelope") expect(request["Content-Encoding"]).to eq("gzip") expect(http_obj.address).to eq("localhost") diff --git a/sentry-ruby/spec/spec_helper.rb b/sentry-ruby/spec/spec_helper.rb index 00b3b310e..4a72657ba 100644 --- a/sentry-ruby/spec/spec_helper.rb +++ b/sentry-ruby/spec/spec_helper.rb @@ -25,6 +25,7 @@ require "sentry-ruby" require "sentry/test_helper" +require "webmock/rspec" require_relative "support/profiler" require_relative "support/stacktrace_test_fixture" @@ -69,6 +70,12 @@ skip("Skipping because one or more guards `#{guards.inspect}` returned false") if skip_examples end + config.around(:each, webmock: false) do |example| + WebMock.disable! + example.run + WebMock.enable! + end + RSpec::Matchers.define :have_recorded_lost_event do |reason, data_category, num: 1| match do |transport| expect(transport.discarded_events[[reason, data_category]]).to eq(num)