Skip to content

Commit

Permalink
filters/thread_filter: don't serialise IO objects
Browse files Browse the repository at this point in the history
Fixes #204 (Airbrake gem segfaults on Circle CI)

The main reason to use `inspect` here was to avoid a bug with IO objects
in Rails apps.

I stumbled upon an edge case while running the thread filter against a
Rails app. The app would stuck because `io_obj.to_json` would never
return. The `io_obj` was open. Closed IO objects raise IOError, which
we already handle.

Given that we stop using `inspect`, I think this also fixes #204.
  • Loading branch information
kyrylo committed May 4, 2017
1 parent da21dfc commit c17d018
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
7 changes: 5 additions & 2 deletions lib/airbrake-ruby/filters/thread_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,15 @@ def call(notice)

def thread_variables(th)
th.thread_variables.map.with_object({}) do |var, h|
h[var] = th.thread_variable_get(var).inspect
h[var] = th.thread_variable_get(var)
end
end

def fiber_variables(th)
th.keys.map.with_object({}) { |key, h| h[key] = th[key].inspect }
th.keys.map.with_object({}) do |key, h|
next if (value = th[key]).is_a?(IO)
h[key] = value
end
end

def add_thread_info(th, thread_info)
Expand Down
21 changes: 19 additions & 2 deletions spec/filters/thread_filter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@
subject.call(notice)
th.thread_variable_set(:bingo, nil)

expect(notice[:params][:thread][:thread_variables][:bingo]).to eq(':bango')
expect(notice[:params][:thread][:thread_variables][:bingo]).to eq(:bango)
end

it "appends fiber variables" do
th[:bingo] = :bango
subject.call(notice)
th[:bingo] = nil

expect(notice[:params][:thread][:fiber_variables][:bingo]).to eq(':bango')
expect(notice[:params][:thread][:fiber_variables][:bingo]).to eq(:bango)
end

it "appends name", skip: !Thread.current.respond_to?(:name) do
Expand Down Expand Up @@ -49,4 +49,21 @@
subject.call(notice)
expect(notice[:params][:thread][:safe_level]).to eq(0)
end

context "when an IO-like object is stored" do
let(:io_obj) do
Class.new(IO) do
def initialize; end
end.new
end

it "doesn't append the IO object" do
expect(io_obj).to be_is_a(IO)
th.thread_variable_set(:io, io_obj)
subject.call(notice)
th.thread_variable_set(:io, nil)

expect(notice[:params][:thread][:fiber_variables][:io]).to be_nil
end
end
end

0 comments on commit c17d018

Please sign in to comment.