getsentry / getsentry/sentry-ruby

record_lost_event can raise "can't add a new key into hash during iteration"

Open
#3,037 1 comment 0 reactions 1 assignee Claimed by @sl0thentr0py View on GitHub
Bug Client Reports Ruby Waiting for: Product Owner
Dominant language
Ruby
Stars
987
Forks
541
Avg merge
17h 40m
Merged PRs (30d)
19

Description

`Transport#@discarded_events` is a plain Hash with no synchronization. `fetch_pending_client_report` iterates it with `map` (transport.rb:198) while `record_lost_event` inserts into it (transport.rb:174, :179).

Ruby's iteration guard raises in the **inserting** thread, so the error surfaces in `record_lost_event`, whose callers include `Client#capture_event` on request threads and the background worker.

Reproduced as `RuntimeError: can't add a new key into hash during iteration`, but only after widening `@discarded_events` to 20k keys to open the window. In production the hash holds a handful of reason/category pairs, so the odds are low. Filing for completeness, not urgency.

Failing spec

Drop in `sentry-ruby/spec/` and run `bundle exec rspec spec/transport_discarded_events_spec.rb`. Fails on master with `RuntimeError: can't add a new key into hash during iteration`.

```ruby
# Stress repro, not a suite-quality spec. The window is widened deliberately:
# @discarded_events normally holds a handful of pairs, which makes the real
# iteration too short to interleave reliably.
# Run from sentry-ruby/: bundle exec rspec path/to/transport_discarded_events_spec.rb
require "spec_helper"

RSpec.describe "Transport#record_lost_event concurrency" do
before { perform_basic_setup }

let(:transport) { Sentry.get_current_client.transport }

it "does not raise while a client report is being built" do
errors = Queue.new
done = false

writer = Thread.new do
i = 0
until done
begin
transport.record_lost_event(:network_error, "category#{i += 1}")
rescue => e
errors << e
end
end
end

500.times do
wide = Hash.new(0)
20_000.times { |i| wide[[:network_error, "seed#{i}"]] = 1 }
transport.instance_variable_set(:@discarded_events, wide)
transport.send(:fetch_pending_client_report, force: true)
end
done = true
writer.join

expect(errors).to be_empty, "record_lost_event raised: #{errors.size > 0 ? errors.pop.inspect : ''}"
end
end
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.