Skip to content

Commit

Permalink
Ignore closed pipe error in response body wrapper (#1254)
Browse files Browse the repository at this point in the history
Do not report closed pipe errors when instrumenting the response body in
the body wrapper classes. We've gotten reports of this error being
reported when a user clicks a link, but before the page managed to load,
click away to another page. We shouldn't report this error when this
happens.
  • Loading branch information
tombruijn authored Aug 22, 2024
1 parent e05e2cb commit 8ad8a05
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: change
---

Ignore `Errno::EPIPE` errors when instrumenting response bodies. We've noticed this error gets reported when the connection is broken between server and client. This happens in normal scenarios so we'll ignore this error in this scenario to avoid error reports from errors that cannot be resolved.
12 changes: 12 additions & 0 deletions lib/appsignal/rack/body_wrapper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module Appsignal
module Rack
# @api private
class BodyWrapper
IGNORED_ERRORS = [Errno::EPIPE].freeze

def self.wrap(original_body, appsignal_transaction)
# The logic of how Rack treats a response body differs based on which methods
# the body responds to. This means that to support the Rack 3.x spec in full
Expand Down Expand Up @@ -49,6 +51,8 @@ def close
Appsignal.instrument("close_response_body.rack") { @body.close }
end
@body_already_closed = true
rescue *IGNORED_ERRORS # Do not report
raise
rescue Exception => error # rubocop:disable Lint/RescueException
@transaction.set_error(error)
raise error
Expand All @@ -75,6 +79,8 @@ def each(&blk)
Appsignal.instrument("process_response_body.rack", "Process Rack response body (#each)") do
@body.each(&blk)
end
rescue *IGNORED_ERRORS # Do not report
raise
rescue Exception => error # rubocop:disable Lint/RescueException
@transaction.set_error(error)
raise error
Expand All @@ -94,6 +100,8 @@ def call(stream)
Appsignal.instrument("process_response_body.rack", "Process Rack response body (#call)") do
@body.call(stream)
end
rescue *IGNORED_ERRORS # Do not report
raise
rescue Exception => error # rubocop:disable Lint/RescueException
@transaction.set_error(error)
raise error
Expand All @@ -118,6 +126,8 @@ def to_ary
) do
@body.to_ary
end
rescue *IGNORED_ERRORS # Do not report
raise
rescue Exception => error # rubocop:disable Lint/RescueException
@transaction.set_error(error)
raise error
Expand All @@ -134,6 +144,8 @@ def to_path
) do
@body.to_path
end
rescue *IGNORED_ERRORS # Do not report
raise
rescue Exception => error # rubocop:disable Lint/RescueException
@transaction.set_error(error)
raise error
Expand Down
48 changes: 48 additions & 0 deletions spec/lib/appsignal/rack/body_wrapper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,18 @@
expect(transaction).to have_error("ExampleException", "error message")
end

it "doesn't report EPIPE error" do
fake_body = double
expect(fake_body).to receive(:each).once.and_raise(Errno::EPIPE)

wrapped = described_class.wrap(fake_body, transaction)
expect do
expect { |b| wrapped.each(&b) }.to yield_control
end.to raise_error(Errno::EPIPE)

expect(transaction).to_not have_error
end

it "closes the body and tracks an instrumentation event when it gets closed" do
fake_body = double(:close => nil)
expect(fake_body).to receive(:each).once.and_yield("a").and_yield("b").and_yield("c")
Expand Down Expand Up @@ -134,6 +146,17 @@
expect(transaction).to have_error("ExampleException", "error message")
end

it "doesn't report EPIPE error" do
expect(fake_body).to receive(:each).once.and_raise(Errno::EPIPE)

wrapped = described_class.wrap(fake_body, transaction)
expect do
expect { |b| wrapped.each(&b) }.to yield_control
end.to raise_error(Errno::EPIPE)

expect(transaction).to_not have_error
end

it "reads out the body in full using to_ary" do
expect(fake_body).to receive(:to_ary).and_return(["one", "two", "three"])

Expand Down Expand Up @@ -207,6 +230,17 @@
expect(transaction).to have_error("ExampleException", "error message")
end

it "doesn't report EPIPE error" do
expect(fake_body).to receive(:to_path).once.and_raise(Errno::EPIPE)

wrapped = described_class.wrap(fake_body, transaction)
expect do
wrapped.to_path
end.to raise_error(Errno::EPIPE)

expect(transaction).to_not have_error
end

it "exposes to_path to the sender" do
allow(fake_body).to receive(:to_path).and_return("/tmp/file.bin")

Expand Down Expand Up @@ -259,5 +293,19 @@

expect(transaction).to have_error("ExampleException", "error message")
end

it "doesn't report EPIPE error" do
fake_rack_stream = double
expect(fake_body).to receive(:call)
.with(fake_rack_stream)
.and_raise(Errno::EPIPE)

wrapped = described_class.wrap(fake_body, transaction)
expect do
wrapped.call(fake_rack_stream)
end.to raise_error(Errno::EPIPE)

expect(transaction).to_not have_error
end
end
end

0 comments on commit 8ad8a05

Please sign in to comment.