Skip to content

Commit

Permalink
Closes puma#2341 - Make sure fallback Rack response are sent to the c…
Browse files Browse the repository at this point in the history
…lient

Currently, when Client I/O operations raise any exceptions, we build a fallback rack response (either by lowlevel_error_handler or by default puma's hard-coded response), but doesn't do anything with it.
This commit ensures that this response is sent back to client.
Use `bundle exec puma -C test/config/lowlevel_error_handler.rb test/rackup/hello.ru` to see the behavior.
  • Loading branch information
Vuta committed Mar 12, 2023
1 parent c8be369 commit a90b6b3
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 4 deletions.
14 changes: 11 additions & 3 deletions lib/puma/server.rb
Expand Up @@ -464,7 +464,7 @@ def process_client(client)
end
true
rescue StandardError => e
client_error(e, client)
client_error(e, client, requests)
# The ensure tries to close +client+ down
requests > 0
ensure
Expand Down Expand Up @@ -492,21 +492,29 @@ def with_force_shutdown(client, &block)
# :nocov:

# Handle various error types thrown by Client I/O operations.
def client_error(e, client)
def client_error(e, client, requests = 1)
# Swallow, do not log
return if [ConnectionError, EOFError].include?(e.class)

lowlevel_error(e, client.env)
case e
when MiniSSL::SSLError
@log_writer.ssl_error e, client.io
when HttpParserError
status, headers, res_body = lowlevel_error(e, client.env, 400)
prepare_response(status, headers, res_body, requests, client)

client.write_error(400)
@log_writer.parse_error e, client
when HttpParserError501
status, headers, res_body = lowlevel_error(e, client.env, 501)
prepare_response(status, headers, res_body, requests, client)

client.write_error(501)
@log_writer.parse_error e, client
else
status, headers, res_body = lowlevel_error(e, client.env, 500)
prepare_response(status, headers, res_body, requests, client)

client.write_error(500)
@log_writer.unknown_error e, nil, "Read"
end
Expand Down
7 changes: 7 additions & 0 deletions test/config/lowlevel_error_handler.rb
@@ -0,0 +1,7 @@
app do |env|
[200, nil, []]
end

lowlevel_error_handler do |err|
[500, {}, ["something wrong happened"]]
end
2 changes: 1 addition & 1 deletion test/test_config.rb
Expand Up @@ -586,7 +586,7 @@ def assert_run_hooks(hook_name, options = {})

def assert_warning_for_hooks_defined_in_single_mode(hook_name)
out, _ = capture_io do
conf = Puma::Configuration.new do |c|
Puma::Configuration.new do |c|
c.send(hook_name)
end
end
Expand Down
15 changes: 15 additions & 0 deletions test/test_puma_server.rb
Expand Up @@ -1545,4 +1545,19 @@ def test_streaming_enum_body_2
assert_equal str * loops, resp_body
assert_operator times.last - times.first, :>, 1.0
end

def test_lowlevel_error_handler_response
options = {
lowlevel_error_handler: ->(_error) do
[500, {}, ["something wrong happened"]]
end
}
broken_app = ->(_env) { [200, nil, []] }

server_run(**options, &broken_app)

data = send_http_and_read "GET / HTTP/1.1\r\n\r\n"

assert_match(/something wrong happened/, data)
end
end

0 comments on commit a90b6b3

Please sign in to comment.