heroku-deflater icon indicating copy to clipboard operation
heroku-deflater copied to clipboard

Likely issue with rails 6.1.0-alpha

Open josh-m-sharpe opened this issue 5 years ago • 9 comments

Using rails 00c2d3e1e61093451a00b1b70548cfd9ae549c53 as of this writing.

2020-06-13T18:37:54+00:00: Rack app error handling request { GET /assets/admin-c9569c117d1107474494012b3a255f49a3570eedd08e51123008d0412abc93cb.js }
#<NoMethodError: undefined method `match?' for #<ActionDispatch::FileHandler:0x000055dc97100c10>>
/app/vendor/bundle/ruby/2.6.0/gems/heroku-deflater-0.6.3/lib/heroku-deflater/serve_zipped_assets.rb:31:in `call'
/app/vendor/bundle/ruby/2.6.0/gems/rack-2.2.2/lib/rack/sendfile.rb:110:in `call'
/app/vendor/bundle/ruby/2.6.0/bundler/gems/rails-c13d46937445/actionpack/lib/action_dispatch/middleware/host_authorization.rb:76:in `call'
/app/vendor/bundle/ruby/2.6.0/gems/sentry-raven-2.13.0/lib/raven/integrations/rack.rb:51:in `call'
/app/vendor/bundle/ruby/2.6.0/gems/rack-cors-1.1.1/lib/rack/cors.rb:100:in `call'

I'd guess this is related to this commit, which not only removes the match? method - it does away with FileHandler in favor of Static

josh-m-sharpe avatar Jun 13 '20 18:06 josh-m-sharpe

I also got this issue when upgrading my Rails app to 6.1.0.rc1:

2020-11-04 16:26:31 +0100 Rack app ("GET /assets/application-d202e7f926ea26f1a77ad108db4187a078d7c86bebf0e97e4fe24242cdc9c487.css" - (127.0.0.1)): #<NoMethodError: undefined method `match?' for #<ActionDispatch::FileHandler:0x00007f9009c58d48>>
2020-11-04 16:26:31 +0100 Rack app ("GET /assets/application-3e190496dea178b3bbd482d5809118b541c5a13462d8fccbe725158cbeac7d92.js" - (127.0.0.1)): #<NoMethodError: undefined method `match?' for #<ActionDispatch::FileHandler:0x00007f9009c58d48>>
...

CSS/JS assets can't load so the system tests generate screenshots with only HTML and no styling applied. I also get errors such as error: ReferenceError: $ is not defined because of this. I did not notice the issue when running the app in development because I load this gem as follows: gem 'heroku-deflater', group: [:production, :test].

However I believe my app actually does not need this. I'm not sure which one does the job, but between the app being deployed on Heroku and having CloudFlare in front of this, I checked that when I remove this gem heroku-deflater, deploy to production, and check the traffic with the browser dev tools, there is still a response header content-encoding: br. Also, my vague understanding is that if compression needed to be done within Rails, it could be achieved by adding config.middleware.insert_after ActionDispatch::Static, Rack::Deflater to config/application.rb. I would just have liked to have a way to test to guarantee that compression is happening, something I could do before when using this gem (I had written a test taking inspiration from this post).

sedubois avatar Nov 04 '20 17:11 sedubois

I am experiencing this issue with Rails 6.1 on heroku.

atstockland avatar Dec 23 '20 02:12 atstockland

I don't have time to support this at the moment, but I'll be happy to receive a PR.

romanbsd avatar Dec 23 '20 08:12 romanbsd

If I'm understanding this correctly, the functionality provided by the class ServeZippedAssets is now built into ActionDispatch::Static. It seems probable that has been the case for some time, but since nothing was breaking we never noticed it.

@romanbsd introduced this feature in 2013, here, and Rails introduced this in 2014 and then improved it over the years to offer much of the same abilities as @romanbsd baked into his. But the recent refactor of that class removed the interface this gem expected. To maintain expected behavior, it seems like a simple major/minor version check could be added and bypass this in the case of Rails 6.1+.

I'll gladly put together a PR if I'm on the right track.

ps - It seems there may be other aspects of this gem that are baked into Rails now, but I don't have time to dig through.

pungerfreak avatar Jan 15 '21 22:01 pungerfreak

I got the same error, but I found I should have changed the setting below:

 # config/application.rb
-config.load_defaults 6.0
+config.load_defaults 6.1

I could solve the issue with this change.

Oh sorry, that was not the solution. The issue still exists😣

JunichiIto avatar Feb 13 '21 09:02 JunichiIto

Could someone please clarify if this gem is useful for Rails 6.1+?

We're also in a state where removing the gem makes our app work after upgrading to 6.1, but I'm trying to figure out if I should continue to follow this gem, or if it's obsolete.

xxx avatar Mar 20 '21 00:03 xxx

On our website with Rails 6.1 it became obsolete. We removed it. And added config.middleware.use Rack::Deflater in config/application.rb instead.

mtmail avatar Mar 20 '21 15:03 mtmail

I also removed it, and replaced it with:

config.middleware.use Rack::Deflater,
  include: Rack::Mime::MIME_TYPES.select{|k, v| v =~ /text|json|javascript/ }.values.uniq,
  if: lambda {|env, status, headers, body| body.body.length > 512 }

require 'rack/brotli'

config.middleware.use Rack::Brotli,
  include: Rack::Mime::MIME_TYPES.select{|k, v| v =~ /text|json|javascript/ }.values.uniq,
  if: lambda {|env, status, headers, body| body.body.length > 512 },
  deflater: {
    quality: 1
  }

at the end of application.rb. Note that the Brotli middleware requires the rack-brotli gem.

This encodes javascript, css, and html with Brotli (if supported as per the request's Accepts header), or GZip otherwise.

jsaraiva avatar Mar 20 '21 16:03 jsaraiva

other discussion: https://github.com/romanbsd/heroku-deflater/issues/32 https://github.com/romanbsd/heroku-deflater/issues/37

jjb avatar Mar 04 '22 01:03 jjb