Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Puma sends empty response bodies even when low-level handler is specified #2341

Closed
cjlarose opened this issue Aug 18, 2020 · 4 comments · Fixed by #2731 or #3094
Closed

Puma sends empty response bodies even when low-level handler is specified #2341

cjlarose opened this issue Aug 18, 2020 · 4 comments · Fixed by #2731 or #3094
Assignees
Labels

Comments

@cjlarose
Copy link
Member

Describe the bug

Puma's README describes the lowlevel_error_handler as a way to specify how Puma should react in error conditions outside of the context of your application. Specifically, the docs show that you can return a Rack response with the expectation that Puma will write that response to the client socket.

In practice, though, Puma only writes that response in one particular kind of low-level error, server.rb:737. There are around six other places where the lowlevel_error_handler is invoked, but the return value is just discarded. In these cases, Puma writes the HTTP status line itself to the socket and closes it, without writing headers or a response body.

Since the lowlevel_error_handler is invoked in these cases, it's still better than nothing since application developers can still be notified of errors happening if they're reporting the error to their error collection system. But Puma offers no way to customize the response code, response headers, or response body in these cases. Clients just get blank responses.

Puma config:

lowlevel_error_handler do |e|
  puts "lowlevel_error_handler called"
  [500, {}, ["An error has occurred, and engineers have been informed. Please reload the page. If you continue to have problems, contact support@example.com\n"]]
end

Expected behavior

Puma writes the Rack response specified in the return value of the lowlevel_error_handler in every case that it's invoked.

Desktop (please complete the following information):

  • OS: MacOS, Linux
  • Puma Version: 4.3.5
@cjlarose cjlarose self-assigned this Jan 4, 2021
@dentarg
Copy link
Member

dentarg commented Sep 28, 2021

@cjlarose did you start on this? would be useful to be able to rescue Puma::HttpParserErrors IMHO and not have Puma log them (the requests are most often abuse)

@cjlarose
Copy link
Member Author

Haven't started on this myself. I think when I wrote it up originally, my team was actually finding that puma was giving 5xx error responses to some machine clients (other services in our system), but our error tracker didn't have anything relevant. I think it was related to a problem with phased restarts.

What I want is to at least give the option to application developers to handle the error and respond with their own custom error response if they want. Right now, there a few places where application developers can't even hook into the errors and know that they're happening.

I agree it's normally misbehaving HTTP clients that causes these kinds of errors, but in some cases we've found that legitimate users can run into these problems and it'd be nice to be able to report them.

@dentarg
Copy link
Member

dentarg commented Dec 15, 2022

Re-opening this issue as #2731 was reverted in #2809

@dentarg dentarg reopened this Dec 15, 2022
Vuta added a commit to Vuta/puma that referenced this issue Mar 12, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Mar 12, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Mar 12, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Mar 12, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Mar 12, 2023
…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.
@Vuta
Copy link
Contributor

Vuta commented Mar 12, 2023

I took a stab at this here #3094. The approach is quite the same as #2731, except that I re-use the prepare_response method. Hence no changes in the Puma::Request module.
Let me know if there's anything else needed to be tackled.

Vuta added a commit to Vuta/puma that referenced this issue Mar 13, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Mar 15, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Mar 15, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Mar 17, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue May 22, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Jun 17, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Jun 22, 2023
…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.
Vuta added a commit to Vuta/puma that referenced this issue Jun 24, 2023
…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.
nateberkopec pushed a commit that referenced this issue Jul 23, 2023
#3094)

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants