Skip to content

Commit af0b1e5

Browse files
unflxwunflxw
committed
Ignore Errno::ECONNRESET in Rack wrapper
1 parent ef8021b commit af0b1e5

2 files changed

Lines changed: 162 additions & 7 deletions

File tree

‎lib/appsignal/rack/body_wrapper.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ module Appsignal
44
module Rack
55
# @api private
66
class BodyWrapper
7-
IGNORED_ERRORS = [Errno::EPIPE].freeze
7+
IGNORED_ERRORS = [Errno::EPIPE, Errno::ECONNRESET].freeze
88

99
def self.wrap(original_body, appsignal_transaction)
1010
# The logic of how Rack treats a response body differs based on which methods

‎spec/lib/appsignal/rack/body_wrapper_spec.rb‎

Lines changed: 161 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,18 @@
9090
expect(transaction).to_not have_error
9191
end
9292

93+
it "doesn't report ECONNRESET error" do
94+
fake_body = double
95+
expect(fake_body).to receive(:each).once.and_raise(Errno::ECONNRESET)
96+
97+
wrapped = described_class.wrap(fake_body, transaction)
98+
expect do
99+
expect { |b| wrapped.each(&b) }.to yield_control
100+
end.to raise_error(Errno::ECONNRESET)
101+
102+
expect(transaction).to_not have_error
103+
end
104+
93105
it "does not report EPIPE error when it's the error cause" do
94106
error = error_with_cause(StandardError, "error message", Errno::EPIPE)
95107
fake_body = double
@@ -116,6 +128,32 @@
116128
expect(transaction).to_not have_error
117129
end
118130

131+
it "does not report ECONNRESET error when it's the error cause" do
132+
error = error_with_cause(StandardError, "error message", Errno::ECONNRESET)
133+
fake_body = double
134+
expect(fake_body).to receive(:each).once.and_raise(error)
135+
136+
wrapped = described_class.wrap(fake_body, transaction)
137+
expect do
138+
expect { |b| wrapped.each(&b) }.to yield_control
139+
end.to raise_error(StandardError, "error message")
140+
141+
expect(transaction).to_not have_error
142+
end
143+
144+
it "does not report ECONNRESET error when it's the nested error cause" do
145+
error = error_with_nested_cause(StandardError, "error message", Errno::ECONNRESET)
146+
fake_body = double
147+
expect(fake_body).to receive(:each).once.and_raise(error)
148+
149+
wrapped = described_class.wrap(fake_body, transaction)
150+
expect do
151+
expect { |b| wrapped.each(&b) }.to yield_control
152+
end.to raise_error(StandardError, "error message")
153+
154+
expect(transaction).to_not have_error
155+
end
156+
119157
it "closes the body and tracks an instrumentation event when it gets closed" do
120158
fake_body = double(:close => nil)
121159
expect(fake_body).to receive(:each).once.and_yield("a").and_yield("b").and_yield("c")
@@ -144,10 +182,16 @@
144182
expect(fake_body).to receive(:close).and_raise(Errno::EPIPE)
145183

146184
wrapped = described_class.wrap(fake_body, transaction)
147-
expect do
148-
wrapped.close
149-
end.to raise_error(Errno::EPIPE)
185+
expect { wrapped.close }.to raise_error(Errno::EPIPE)
186+
expect(transaction).to_not have_error
187+
end
150188

189+
it "doesn't report ECONNRESET error on close" do
190+
fake_body = double
191+
expect(fake_body).to receive(:close).and_raise(Errno::ECONNRESET)
192+
193+
wrapped = described_class.wrap(fake_body, transaction)
194+
expect { wrapped.close }.to raise_error(Errno::ECONNRESET)
151195
expect(transaction).to_not have_error
152196
end
153197

@@ -157,10 +201,17 @@
157201
expect(fake_body).to receive(:close).and_raise(error)
158202

159203
wrapped = described_class.wrap(fake_body, transaction)
160-
expect do
161-
wrapped.close
162-
end.to raise_error(StandardError, "error message")
204+
expect { wrapped.close }.to raise_error(StandardError, "error message")
205+
expect(transaction).to_not have_error
206+
end
207+
208+
it "does not report ECONNRESET error when it's the error cause on close" do
209+
error = error_with_cause(StandardError, "error message", Errno::ECONNRESET)
210+
fake_body = double
211+
expect(fake_body).to receive(:close).and_raise(error)
163212

213+
wrapped = described_class.wrap(fake_body, transaction)
214+
expect { wrapped.close }.to raise_error(StandardError, "error message")
164215
expect(transaction).to_not have_error
165216
end
166217
end
@@ -228,6 +279,17 @@
228279
expect(transaction).to_not have_error
229280
end
230281

282+
it "doesn't report ECONNRESET error" do
283+
expect(fake_body).to receive(:each).once.and_raise(Errno::ECONNRESET)
284+
285+
wrapped = described_class.wrap(fake_body, transaction)
286+
expect do
287+
expect { |b| wrapped.each(&b) }.to yield_control
288+
end.to raise_error(Errno::ECONNRESET)
289+
290+
expect(transaction).to_not have_error
291+
end
292+
231293
it "does not report EPIPE error when it's the error cause" do
232294
error = error_with_cause(StandardError, "error message", Errno::EPIPE)
233295
fake_body = double
@@ -241,6 +303,19 @@
241303
expect(transaction).to_not have_error
242304
end
243305

306+
it "does not report ECONNRESET error when it's the error cause" do
307+
error = error_with_cause(StandardError, "error message", Errno::ECONNRESET)
308+
fake_body = double
309+
expect(fake_body).to receive(:each).once.and_raise(error)
310+
311+
wrapped = described_class.wrap(fake_body, transaction)
312+
expect do
313+
expect { |b| wrapped.each(&b) }.to yield_control
314+
end.to raise_error(StandardError, "error message")
315+
316+
expect(transaction).to_not have_error
317+
end
318+
244319
it "reads out the body in full using to_ary" do
245320
expect(fake_body).to receive(:to_ary).and_return(["one", "two", "three"])
246321

@@ -281,6 +356,21 @@
281356

282357
expect(transaction).to_not have_error
283358
end
359+
360+
it "does not report ECONNRESET error when it's the error cause" do
361+
error = error_with_cause(StandardError, "error message", Errno::ECONNRESET)
362+
fake_body = double
363+
allow(fake_body).to receive(:each)
364+
expect(fake_body).to receive(:to_ary).once.and_raise(error)
365+
expect(fake_body).to_not receive(:close) # Per spec we expect the body has closed itself
366+
367+
wrapped = described_class.wrap(fake_body, transaction)
368+
expect do
369+
wrapped.to_ary
370+
end.to raise_error(StandardError, "error message")
371+
372+
expect(transaction).to_not have_error
373+
end
284374
end
285375

286376
describe "with a body supporting both to_path and each" do
@@ -340,6 +430,17 @@
340430
expect(transaction).to_not have_error
341431
end
342432

433+
it "doesn't report ECONNRESET error" do
434+
expect(fake_body).to receive(:to_path).once.and_raise(Errno::ECONNRESET)
435+
436+
wrapped = described_class.wrap(fake_body, transaction)
437+
expect do
438+
wrapped.to_path
439+
end.to raise_error(Errno::ECONNRESET)
440+
441+
expect(transaction).to_not have_error
442+
end
443+
343444
it "does not report EPIPE error from #each when it's the error cause" do
344445
error = error_with_cause(StandardError, "error message", Errno::EPIPE)
345446
expect(fake_body).to receive(:each).once.and_raise(error)
@@ -352,6 +453,18 @@
352453
expect(transaction).to_not have_error
353454
end
354455

456+
it "does not report ECONNRESET error from #each when it's the error cause" do
457+
error = error_with_cause(StandardError, "error message", Errno::ECONNRESET)
458+
expect(fake_body).to receive(:each).once.and_raise(error)
459+
460+
wrapped = described_class.wrap(fake_body, transaction)
461+
expect do
462+
expect { |b| wrapped.each(&b) }.to yield_control
463+
end.to raise_error(StandardError, "error message")
464+
465+
expect(transaction).to_not have_error
466+
end
467+
355468
it "does not report EPIPE error from #to_path when it's the error cause" do
356469
error = error_with_cause(StandardError, "error message", Errno::EPIPE)
357470
allow(fake_body).to receive(:to_path).once.and_raise(error)
@@ -364,6 +477,18 @@
364477
expect(transaction).to_not have_error
365478
end
366479

480+
it "does not report ECONNRESET error from #to_path when it's the error cause" do
481+
error = error_with_cause(StandardError, "error message", Errno::ECONNRESET)
482+
allow(fake_body).to receive(:to_path).once.and_raise(error)
483+
484+
wrapped = described_class.wrap(fake_body, transaction)
485+
expect do
486+
wrapped.to_path
487+
end.to raise_error(StandardError, "error message")
488+
489+
expect(transaction).to_not have_error
490+
end
491+
367492
it "exposes to_path to the sender" do
368493
allow(fake_body).to receive(:to_path).and_return("/tmp/file.bin")
369494

@@ -431,6 +556,20 @@
431556
expect(transaction).to_not have_error
432557
end
433558

559+
it "doesn't report ECONNRESET error" do
560+
fake_rack_stream = double
561+
expect(fake_body).to receive(:call)
562+
.with(fake_rack_stream)
563+
.and_raise(Errno::ECONNRESET)
564+
565+
wrapped = described_class.wrap(fake_body, transaction)
566+
expect do
567+
wrapped.call(fake_rack_stream)
568+
end.to raise_error(Errno::ECONNRESET)
569+
570+
expect(transaction).to_not have_error
571+
end
572+
434573
it "does not report EPIPE error from #call when it's the error cause" do
435574
error = error_with_cause(StandardError, "error message", Errno::EPIPE)
436575
fake_rack_stream = double
@@ -446,6 +585,22 @@
446585

447586
expect(transaction).to_not have_error
448587
end
588+
589+
it "does not report ECONNRESET error from #call when it's the error cause" do
590+
error = error_with_cause(StandardError, "error message", Errno::ECONNRESET)
591+
fake_rack_stream = double
592+
allow(fake_body).to receive(:call)
593+
.with(fake_rack_stream)
594+
.and_raise(error)
595+
596+
wrapped = described_class.wrap(fake_body, transaction)
597+
598+
expect do
599+
wrapped.call(fake_rack_stream)
600+
end.to raise_error(StandardError, "error message")
601+
602+
expect(transaction).to_not have_error
603+
end
449604
end
450605

451606
def error_with_cause(klass, message, cause)

0 commit comments

Comments
 (0)