Skip to content

Commit

Permalink
Fix issues with stopping Vernier (#2429)
Browse files Browse the repository at this point in the history
  • Loading branch information
solnic authored Oct 11, 2024
1 parent 499cbac commit f3ed31e
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

- Add `include_sentry_event` matcher for RSpec [#2424](https://github.com/getsentry/sentry-ruby/pull/2424)

### Bug Fixes

- Fix Vernier profiler not stopping when already stopped [#2429](https://github.com/getsentry/sentry-ruby/pull/2429)

## 5.21.0

### Features
Expand Down
9 changes: 7 additions & 2 deletions sentry-ruby/lib/sentry/vernier/profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ def start
return unless @sampled
return if @started

::Vernier.start_profile
@started = true
@started = ::Vernier.start_profile

log("Started")

Expand All @@ -77,6 +76,12 @@ def stop
@result = ::Vernier.stop_profile

log("Stopped")
rescue RuntimeError => e
if e.message.include?("Profile not started")
log("Not stopped since not started")
else
log("Failed to stop Vernier: #{e.message}")
end
end

def active_thread_id
Expand Down
14 changes: 14 additions & 0 deletions sentry-ruby/spec/sentry/vernier/profiler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,20 @@
expect(Vernier).to receive(:stop_profile)
profiler.stop
end

it 'does not crash when Vernier was already stopped' do
profiler.set_initial_sample_decision(true)
profiler.start
Vernier.stop_profile
profiler.stop
end

it 'does not crash when stopping Vernier crashed' do
profiler.set_initial_sample_decision(true)
profiler.start
expect(Vernier).to receive(:stop_profile).and_raise(RuntimeError.new("Profile not started"))
profiler.stop
end
end

describe "#to_hash" do
Expand Down

0 comments on commit f3ed31e

Please sign in to comment.