Should fcgi catch EPIPE?
Christian Neukirchen
15:17:37 < stbuehler> chris2: hi! i've been told to ask you... i'd
like to know how to get rid of the EPIPE backtraces in my
fastcgi.crash.log :)
15:17:52 < stbuehler> fastcgi-spec: "When a Web server is not
multiplexing requests over a transport connection, the Web server can
abort a request by closing the request's transport connection." - so i
think rack should handle EPIPE somehow
15:17:56 < stbuehler> some backtraces from EPIPE:
http://paste.lighttpd.net/727
Any good reason to not catch EPIPE?
--
Christian Neukirchen <chneuk...@gmail.com> http://chneukirchen.org
James Tucker
On 23 Dec 2009, at 16:31, Christian Neukirchen wrote:
>
> 15:17:37 < stbuehler> chris2: hi! i've been told to ask you... i'd
> like to know how to get rid of the EPIPE backtraces in my
> fastcgi.crash.log :)
> 15:17:52 < stbuehler> fastcgi-spec: "When a Web server is not
> multiplexing requests over a transport connection, the Web server can
> abort a request by closing the request's transport connection." - so i
> think rack should handle EPIPE somehow
> 15:17:56 < stbuehler> some backtraces from EPIPE:
> http://paste.lighttpd.net/727
>
> Any good reason to not catch EPIPE?
At the top level - maybe - maybe not.
Consider this (awful pseudo code):
run lambda do |env|
thing = Persistence.fetch(:a_thing)
begin; lock = thing.acquire_lock; end until lock
print thing.some_property
thing.change!
print thing.some_other_property
lock.release
end
The slightly more correct version, would be:
run lambda do |env|
begin
thing = Persistence.fetch(:a_thing)
begin; lock = thing.acquire_lock; end until lock
print thing.some_property
thing.change!
print thing.some_other_property
ensure
lock.release
end
end
I'm sure you get the principle of intent here. This is relevant because, in example 1 (how IME the bulk of rubyists would write their apps), the problem of unresolved locks is almost impossible to debug unless the persistence layer can give you some kind of trace on when / where the lock was acquired. If we've silenced EPIPE completely, then there's no indication of error to the application developer, and so many developers would take a long time to realise that example 2 is how they need to deal with the problem.
The question is, do we want to protect those folks (given that we're framework-like)?
Christian Neukirchen
> If we've silenced EPIPE completely, then there's no indication of
> error to the application developer
I thought of something like:
def self.send_headers(out, status, headers)
out.print "Status: #{status}\r\n"
headers.each { |k, vs|
vs.split("\n").each { |v|
out.print "#{k}: #{v}\r\n"
}
}
out.print "\r\n"
out.flush
+ rescue Errno::EPIPE
end
def self.send_body(out, body)
body.each { |part|
out.print part
out.flush
}
+ rescue Errno::EPIPE
end
Eric Wong
> James Tucker <jftu...@gmail.com> writes:
> > If we've silenced EPIPE completely, then there's no indication of
> > error to the application developer
>
> I thought of something like:
>
> def self.send_headers(out, status, headers)
> out.print "Status: #{status}\r\n"
> headers.each { |k, vs|
> vs.split("\n").each { |v|
> out.print "#{k}: #{v}\r\n"
> }
> }
> out.print "\r\n"
> out.flush
> + rescue Errno::EPIPE
> end
>
> def self.send_body(out, body)
> body.each { |part|
> out.print part
> out.flush
> }
> + rescue Errno::EPIPE
> end
To be pedantic, body#each (and headers#each) may also hit EPIPE by
themselves (body could be a pipe/socket itself) and the application
developer may want to know to track down the error with a backtrace.
But avoiding excessive logging/backtraces is a good idea if the
client initiated the disconnect, since it's a potential DoS
(and at the best case, makes a lot of noise).
The following diff is similar to what I did with the Unicorn::TeeInput
class since that can expose EOFError to the application, which (being
client initiated and out of the app's control) would just mean
unnecessary noise for anybody inspecting logs.
Warning: totally untested, possible typos/syntax errors
--- a/lib/rack/handler/fastcgi.rb
+++ b/lib/rack/handler/fastcgi.rb
@@ -53,11 +53,15 @@ module Rack
env.delete "CONTENT_TYPE" if env["CONTENT_TYPE"] == ""
env.delete "CONTENT_LENGTH" if env["CONTENT_LENGTH"] == ""
+ class ClientDisconnect < Errno::EPIPE
+ end
+
begin
status, headers, body = app.call(env)
begin
send_headers request.out, status, headers
send_body request.out, body
+ rescue ClientDisconnect
ensure
body.close if body.respond_to? :close
end
@@ -67,21 +71,26 @@ module Rack
end
end
+ def self.print_safely(out, part, flush = false)
+ out.print part
+ out.flush if flush
+ rescue Errno::EPIPE => e
+ raise ClientDisconnect, "client disconnected", []
+ end
+
def self.send_headers(out, status, headers)
- out.print "Status: #{status}\r\n"
+ print_safely(out, "Status: #{status}\r\n")
headers.each { |k, vs|
vs.split("\n").each { |v|
- out.print "#{k}: #{v}\r\n"
+ print_safely(out, "#{k}: #{v}\r\n")
}
}
- out.print "\r\n"
- out.flush
+ print_safely(out, "\r\n", true)
end
def self.send_body(out, body)
body.each { |part|
- out.print part
- out.flush
+ print_safely(out, part, true)
}
end
end
--
Eric Wong