Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Rack::BodyProxy should execute block even on failures. #313

Merged
merged 1 commit into from

2 participants

@josevalim
Owner

In general, Rack frameworks are moving logic to close hooks in order
to support async behavior increasing the chance an exception will
happen on close. Most servers will actually die if there is an exception
on close, but such exceptions can also happen in the test environment.
In such cases, we can accidentally leave a mutex locked, a database
connection not collected and so forth, therefore, we need to ensure
the block is called regardless closing the body failed.

@josevalim josevalim Rack::BodyProxy should execute block even on failures.
In general, Rack frameworks are moving logic to close hooks in order
to support async behavior increasing the chance an exception will
happen on close. Most servers will actually die if there is an exception
on close, but such exceptions can also happen in the test environment.
In such cases, we can accidentally leave a mutex locked, a database
connection not collected and so forth, therefore, we need to ensure
the block is called regardless closing the body failed.
b06ef82
@tenderlove tenderlove merged commit 6cb96fe into rack:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 13, 2012
  1. @josevalim

    Rack::BodyProxy should execute block even on failures.

    josevalim authored
    In general, Rack frameworks are moving logic to close hooks in order
    to support async behavior increasing the chance an exception will
    happen on close. Most servers will actually die if there is an exception
    on close, but such exceptions can also happen in the test environment.
    In such cases, we can accidentally leave a mutex locked, a database
    connection not collected and so forth, therefore, we need to ensure
    the block is called regardless closing the body failed.
This page is out of date. Refresh to see the latest.
Showing with 22 additions and 2 deletions.
  1. +5 −2 lib/rack/body_proxy.rb
  2. +17 −0 test/spec_body_proxy.rb
View
7 lib/rack/body_proxy.rb
@@ -11,8 +11,11 @@ def respond_to?(*args)
def close
return if @closed
@closed = true
- @body.close if @body.respond_to? :close
- @block.call
+ begin
+ @body.close if @body.respond_to? :close
+ ensure
+ @block.call
+ end
end
def closed?
View
17 test/spec_body_proxy.rb
@@ -1,4 +1,5 @@
require 'rack/body_proxy'
+require 'stringio'
describe Rack::BodyProxy do
should 'call each on the wrapped body' do
@@ -32,6 +33,22 @@
called.should.equal true
end
+ should 'call the passed block on close even if there is an exception' do
+ object = Object.new
+ def object.close() raise "No!" end
+ called = false
+
+ begin
+ proxy = Rack::BodyProxy.new(object) { called = true }
+ called.should.equal false
+ proxy.close
+ rescue RuntimeError => e
+ end
+
+ raise "Expected exception to have been raised" unless e
+ called.should.equal true
+ end
+
should 'not close more than one time' do
count = 0
proxy = Rack::BodyProxy.new([]) { count += 1; raise "Block invoked more than 1 time!" if count > 1 }
Something went wrong with that request. Please try again.