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

This breaks functionality that relies on shutdown functions #19

Closed
johnbillion opened this issue Jan 30, 2019 · 11 comments · Fixed by #28 or #34
Closed

This breaks functionality that relies on shutdown functions #19

johnbillion opened this issue Jan 30, 2019 · 11 comments · Fixed by #28 or #34
Assignees

Comments

@johnbillion
Copy link
Contributor

This is an interesting bug: perftools/xhgui-collector#19

The most visible problem this causes is that Query Monitor doesn't work when Chassis-XHGui is in use, because it uses the shutdown action in WordPress, which is itself attached to a shutdown function which doesn't fire due to the above bug.

Opening this here for visibility and as a reminder that the fix will need to be pulled downstream.

@johnbillion
Copy link
Contributor Author

Alright, there's a fix in place that requires a config option to be set that disables the call to fastcgi_finish_request().

perftools/xhgui-collector#23

I think it makes sense to set that option to prevent Query Monitor from disappearing. What do you think @BronsonQuick ?

@BronsonQuick
Copy link
Member

@johnbillion Awesome. I was just checking my inbox and saw that fix come through.

Yeah that makes sense to me!

@BronsonQuick
Copy link
Member

@johnbillion Sorry for the delay on this. I've updated the submodule and the config so it's working again with Query Monitor.

Chassis_Site__Just_another_WordPress_site_2019-08-19_12-52-58

@svandragt
Copy link
Contributor

Unfortunately this is an issue currently.

I install this plugin by adding it to the extensions list in the config.yaml for Chassis. When I load any WP screen with an admin bar, the Query Monitor menu is empty.
When I add - chassis/chassis_xhgui to the disabed_extensinos list and run vagrant provision and reload, the menu appears.

@joemcgill
Copy link

Can confirm what @svandragt reported. This still seems to be broken. I've tried deleting and reinstalling all extensions and Query Monitor is still broken unless I disable the Chassis XHGUI extension.

@BronsonQuick BronsonQuick reopened this Nov 15, 2019
@BronsonQuick BronsonQuick self-assigned this Nov 15, 2019
@BronsonQuick
Copy link
Member

Yeah I'm gonna have to see if I can figure out some kinda logic so that both can work together. I'll have a bit of a play and see if I can come up with something.

@BronsonQuick
Copy link
Member

Turns out I needed to add a sha upstream to made sure this was fixed: Chassis/xhgui@daaae44...b263243#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780

@joemcgill
Copy link

@BronsonQuick how should I test this? I tried git pull && git submodule update --recursive inside the extensions/chassis_xhgui directory, but QM is still not showing any output.

@BronsonQuick
Copy link
Member

@joemcgill You'll want to delete the vendor directory so then run vagrant provision so that composer installs the Chassis Xhgui Collector fork at the correct Sha

@joemcgill
Copy link

Thanks, B. I tried doing this and it didn't work, but deleting both the chassis_xhgui and xhprof directories from extensions and then running vagrant provision did the job 🎉. So nice to have this working!

@BronsonQuick
Copy link
Member

Ahh right. Sounds like you needed to do a git submodule sync then. I'm happy to have it working again as well. I still don't really get why composer needed the sha. I should've probably tagged it with a release instead. It's a shame flamegraphs were removed but hey, I guess that gives me another performance project to work on haha

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