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

Mini profiler not running on start #27

Conversation

eileencodes
Copy link
Contributor

Since updating the railtie to install after known dependencies (HerokuDeflater::SkipBinary and Rack::Deflater) I could not get MiniProfiler to render on start up (from the master branch). The javascript and views were not being inserted into my app.

It seems that because the insert is being run after_initialization that MiniProfiler isn't getting added. I added Rack::Deflator to see if it ran correctly with the new insert_position and the views still were not rendered.

I think the best method would be to instead insert_before ActionDispatch::Static - this is how both HerokuDeflater::SkipBinary and Rack::Deflater are inserted into the middleware as well. This results in the correct order without having to rely on an array of possible middleware.

…tatic

It seems that MiniProfiler stopped running on start after the pull
request that adds MiniProfiler to the middleware based on where
other Middleware was in the stack. It seems MiniProfiler needs to be
added during initialization not after or else it won't run. From the
research I did I think it would be better to add MiniProfiler before
ActionDispatch::Static instead of checking against the current stack.
That way if Rack::Deflater is alse added before ActionDispatch::Static
then Rack::Delflater will still be before MiniProfiler.
With the new insert_before ActionDispatch::Static these
attributes/instance vars are no longer necessary
@SamSaffron
Copy link
Member

ActionDispatch::Static is a problem though, it may not exist.

@eileencodes
Copy link
Contributor Author

Apologies I am not familiar with middleware stacks, this is relatively new territory for me. I think I found a better solution though.

Do you have an issue (or experienced an issue?) with using app.config.middleware.use(Rack::MiniProfiler) instead?

It will add it to the bottom of the stack instead of the top which will eliminate the problems those using Rack::Deflater or HerokuDeflater::SkipBinary which will get inserted at the top.

… exist

ActionDispatch::Static may not exist so I've replaced insert_before with
use. This will drop MiniProfiler to the bottom of the stack instead of
the top and alleviate the problems with Rack::Deflater, HerokuDeflater::SkipBinary
and MiniProfiler not running in the after_initialize block.
@SamSaffron
Copy link
Member

The trouble with that is that we want to register mini profiler first so it is able to measure how the rest of the middlewares in the stack perform.

If we register it last mini profiler will return inaccurate data.

@eileencodes
Copy link
Contributor Author

Hrm ok, that makes sense. Master as it is right now doesn't run at all and I'm not sure why.

Looking at app.config.middleware in the initialize method returns #Rails::Configuration::MiddlewareStackProxy:0x007fd1b5cc3a38 which won't allow for each_with_index so we can't use an array to check against the stack. I haven't found a way in the initialize method to access the stack list.

In the after_initialize method the app.config.middleware returns #ActionDispatch::MiddlewareStack:0x007f8d48169928 which is an array of the middleware in the stack. This makes sense but when it runs it seems that MiniProfiler isn't inserted in time.

Any ideas?

@SamSaffron
Copy link
Member

Sorry for the delay, I should have this fixed today

@eileencodes
Copy link
Contributor Author

Hey, no worries, I was trying to help. Seems there are a lot of similar issues on github dealing with the stack order of middleware. Definitely not a trivial issue to fix. :)

@SamSaffron
Copy link
Member

no worries, I ended up reverting the regression so all should be working
now.

On Fri, Nov 22, 2013 at 1:18 PM, Eileen notifications@github.com wrote:

Hey, no worries, I was trying to help. Seems there are a lot of similar
issues on github dealing with the stack order of middleware. Definitely not
a trivial issue to fix. :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/27#issuecomment-29044132
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants