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

Early hints + javascript_include_tag = Puma:ConnectionError #1640

Closed
leastbad opened this issue Aug 26, 2018 · 2 comments · Fixed by #1822
Closed

Early hints + javascript_include_tag = Puma:ConnectionError #1640

leastbad opened this issue Aug 26, 2018 · 2 comments · Fixed by #1822
Labels

Comments

@leastbad
Copy link

Steps to reproduce

  1. Procfile enables early hints eg. web: bundle exec puma -C config/puma.rb --early-hints

  2. Make use of javascript_include_tag helper

  3. Production environment on Heroku

Expected behavior

Testing out --early-hints on Heroku, I would expect the hints to be generated as described in Implement H2 Early Hints for Rails.

Actual behavior

I'm seeing some intermittent socket timeout errors when I have --early-hints enabled on my app, which is hosted on Heroku. It's only showing up about once a day, and only in production.

Some lucky visitor's request is throwing one Puma:ConnectionError for each javascript_include_tag in my application layout. If one call fails, they all fail in the order they appear in the template. I've never experienced the end-result personally, leading me to guess that the visitor probably won't notice as their script tags would just load normally.

Aug 16 12:34:37 /app/vendor/bundle/ruby/2.4.0/gems/puma-3.11.4/lib/puma/server.rb:972:in 'rescue in fast_write' 
Aug 16 12:34:37 /app/vendor/bundle/ruby/2.4.0/gems/puma-3.11.4/lib/puma/server.rb:963:in 'fast_write' 
Aug 16 12:34:37 /app/vendor/bundle/ruby/2.4.0/gems/puma-3.11.4/lib/puma/server.rb:609:in 'block in handle_request' 
Aug 16 12:34:37 /app/vendor/bundle/ruby/2.4.0/gems/actionpack-5.2.0/lib/action_dispatch/http/request.rb:217:in 'send_early_hints' 
Aug 16 12:34:37 /app/vendor/bundle/ruby/2.4.0/gems/actionview-5.2.0/lib/action_view/helpers/asset_tag_helper.rb:96:in 'javascript_include_tag' 
Aug 16 12:34:37 /app/vendor/bundle/ruby/2.4.0/gems/sprockets-rails-3.2.1/lib/sprockets/rails/helper.rb:156:in 'block in javascript_include_tag' 
Aug 16 12:34:37 /app/vendor/bundle/ruby/2.4.0/gems/sprockets-rails-3.2.1/lib/sprockets/rails/helper.rb:154:in 'map' 
Aug 16 12:34:37 /app/vendor/bundle/ruby/2.4.0/gems/sprockets-rails-3.2.1/lib/sprockets/rails/helper.rb:154:in 'javascript_include_tag' 
Aug 16 12:34:37 /app/app/views/layouts/application.html.erb:97:in 'block in _app_views_layouts_application_html_erb__4118899528439397404_70152019751760' 
Aug 16 12:34:37 /app/app/views/layouts/application.html.erb:95:in 'each' 
Aug 16 12:34:37 /app/app/views/layouts/application.html.erb:95:in '_app_views_layouts_application_html_erb__4118899528439397404_70152019751760'

System configuration

Ruby version: 2.4.1
Rails version: 5.2.0
Puma version: 3.11.4

@johngallagher
Copy link

I'm also seeing this error - any ideas to fix would be greatly appreciated!

@Jesus
Copy link
Contributor

Jesus commented Jun 18, 2019

This error happens when the connection is lost before we're able to send any early hints.

We could have the same situation even without early hints if the connection is lost before any response headers are sent, however Puma swallows these errors which I think is fine.

A simple way to reproduce this is running this against your web server:

require 'socket'

request = [
  "GET /buildings HTTP/1.1",
  "Host: 127.0.0.1:3000",
  "\r\n"
]

socket = TCPSocket.new '127.0.0.1', 3000
socket.puts request.join("\r\n")

# Close socket without reading anything back from the server...
socket.close

No error will appear in the client but the reported error will appear in the app logs.

Jesus added a commit to Jesus/puma that referenced this issue Jun 18, 2019
I've been thinking on how Puma should behave when the early hints
can't be sent because the connection has been lost.

I first thought that it should behave the same way it does when the
connection is lost before main response is sent, I've seen that it
swallows those errors (https://git.io/fjVL8) so perhaps we should do
the same here.

There's one consideration though. Because the early hints are sent at
the very beginning of the app execution, we may want to abort this
request altogether and save resources by throwing an exception and
causing Rails to interrupt execution.

Finally I decided to overlook that case just because I believe it's
least surprising from the Rack app point of view. Also
I haven't seen any other example of the web server interrupting
the apps execution due to an error on the lower layer.

Fixes puma#1640.
Jesus added a commit to Jesus/puma that referenced this issue Jun 18, 2019
I've been thinking on how Puma should behave when the early hints
can't be sent because the connection has been lost.

I first thought that it should behave the same way it does when the
connection is lost before main response is sent, I've seen that it
swallows those errors (https://git.io/fjVtg) so perhaps we should do
the same here.

There's one consideration though. Because the early hints are sent at
the very beginning of the app execution, we may want to abort this
request altogether and save resources by throwing an exception and
causing Rails to interrupt execution.

Finally I decided to overlook that case just because I believe it's
least surprising from the Rack app point of view. Also
I haven't seen any other example of the web server interrupting
the apps execution due to an error on the lower layer.

Fixes puma#1640.
nateberkopec pushed a commit that referenced this issue Aug 1, 2019
* Swallow connection errors when sending early hints

I've been thinking on how Puma should behave when the early hints
can't be sent because the connection has been lost.

I first thought that it should behave the same way it does when the
connection is lost before main response is sent, I've seen that it
swallows those errors (https://git.io/fjVtg) so perhaps we should do
the same here.

There's one consideration though. Because the early hints are sent at
the very beginning of the app execution, we may want to abort this
request altogether and save resources by throwing an exception and
causing Rails to interrupt execution.

Finally I decided to overlook that case just because I believe it's
least surprising from the Rack app point of view. Also
I haven't seen any other example of the web server interrupting
the apps execution due to an error on the lower layer.

Fixes #1640.

* Adds a test

* Updates test as per @nateberkopec's suggestion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants