MiniProfiler / rack-mini-profiler

Profiler for your development and production Ruby rack apps.
MIT License
3.73k stars 406 forks source link

Storage service is called for every request in production, and will raise unhandled exception #427

Closed jeffblake closed 4 years ago

jeffblake commented 4 years ago

I think I'm on the right track with this one...

I noticed 50 or so failed (status 500) requests due to a communication error with Memcached. I found that strange, because they were end-user requests where Miniprofiler was not turned on.

Stack trace:

Screen Shot 2019-12-11 at 7 34 34 PM

I see two problems here,

  1. https://github.com/MiniProfiler/rack-mini-profiler/blob/0a747e590ee8723bb3c20502554c9883989dd816/lib/mini_profiler/profiler.rb#L184 calls into Memcached even if the request has not been authorized

  2. handle_cookie (above) is called outside of where storage service exceptions are handled, e.g. here https://github.com/MiniProfiler/rack-mini-profiler/blob/0a747e590ee8723bb3c20502554c9883989dd816/lib/mini_profiler/profiler.rb#L372

I would expect MiniProfiler to not communicate with Memcached unless I've authorized MiniProfiler to run on that request, and I'd also expect Miniprofiler to do nothing and pass along to the next rack app if any exceptions were raised

Settings:

Rack::MiniProfiler.config.authorization_mode = :whitelist

If a request is authorized, I call

Rack::MiniProfiler.authorize_request if admin_user_signed_in? in a before_action in the ApplicationController

Platform Rails Heroku latest versions

SamSaffron commented 4 years ago

We have not choice but to call the storage engine here:

https://github.com/MiniProfiler/rack-mini-profiler/blob/0a747e590ee8723bb3c20502554c9883989dd816/lib/mini_profiler/client_settings.rb#L93-L93

If the client presents us with a cookie saying it is authorized we need to check in with the storage engine to see if it is indeed authorized.

That said, we can rescue this exception, I will check something in now.

jeffblake commented 4 years ago

Thanks!