igorkasyanchuk / rails_performance

Monitor performance of you Rails applications (self-hosted and free)
https://www.railsjazz.com/
MIT License
957 stars 54 forks source link

Exception when `serve_static_files` is disabled #29

Closed ivanyv closed 3 years ago

ivanyv commented 3 years ago

In such case, ActionDispatch::Static does not exist, causing an exception in https://github.com/igorkasyanchuk/rails_performance/blob/master/lib/rails_performance/engine.rb#L16.

Not sure if conditionally using Rack::Sendfile when appropriate or using insert_before Rack::Lock would make more sense.

igorkasyanchuk commented 3 years ago

hello @ivanyv please try to clone repo and connect in your app and try if it works you can try to make a PR or I'll make a fix. I just want to know if this is a working solution

igorkasyanchuk commented 3 years ago

@ivanyv how you are disabling this setting? how to reproduce

ivanyv commented 3 years ago

@igorkasyanchuk in config/environments/X.rb, config.serve_static_files = false

igorkasyanchuk commented 3 years ago

I've in all apps config.serve_static_files = ENV['RAILS_SERVE_STATIC_FILES'].present?

and this RAILS_SERVE_STATIC_FILES is not present

so I think it's false by default.

I'm asking because maybe you have some code where you are deleting middleware.

image and this options is explicitly false in prod config.

ivanyv commented 3 years ago

There could be that some library is removing that middleware but I'm not doing it explicitly. FWIW, setting that config to true fixes the issue (forgot to mention this is Rails 4.2).

igorkasyanchuk commented 3 years ago

try the new version, it has new features plus I've added a fix for your problem

gem 'rails_performance', '1.0.0.beta5'

ivanyv commented 3 years ago

Will do, thanks!

igorkasyanchuk commented 3 years ago

does it work? @ivanyv

igorkasyanchuk commented 3 years ago

new version released, I think now should work

ivanyv commented 3 years ago

@igorkasyanchuk sorry it took me a bit to look at this again; it doesn't work yet because the middleware insertion happens later, so I think a better solution could be:

after = app.config.serve_static_files ? ActionDispatch::Static : Rack::SendFile
app.middleware.insert_after after, RailsPerformance::Rails::Middleware
salzig commented 3 years ago

@ivanyv is right. Still produces an error.

igorkasyanchuk commented 3 years ago

@ivanyv @salzig please it someone can create a PR it would be nice. I just can't handle it now. Thank you