johnbillion / query-monitor

The developer tools panel for WordPress
https://querymonitor.com
GNU General Public License v2.0
1.6k stars 212 forks source link

2.16.1: breaks WooCommerce (2.6.14) ?add-to-cart processing in some cases #277

Closed lkraav closed 6 years ago

lkraav commented 7 years ago

Hey. Just discovered that latest QM release is directly involved in breaking our store's checkout flow, for the first time ever. Disabling QM immediately helps.

The symptom is adding the product to the cart and going to checkout causes WC to display "Item that is no longer available has been removed from your cart" error and the cart remains empty.

I will do a quick bisect run here to find out what's happening and report back.

lkraav commented 7 years ago

Bisect shows the bad commit to be 877b69ec0b64cca4a5e81456ea82854495c2bd8e

I see the following PHP warning

[01-Nov-2017 13:08:29 UTC] PHP Notice:  Undefined variable: args in /home/warmpress/wp-content/plugins/query-monitor.git/output/html/caps.php on line 36

EDIT but this seems to be fixed later, so can't be this alone

EDIT primary hypothesis is there might be some premature output somewhere in the cap tracking system that messes up WC cart redirects / headers?

johnbillion commented 7 years ago

Thanks for the report. The Undefined variable: args notice was fixed in a later commit, so that's a red herring.

lkraav commented 7 years ago

No other errors. Isolated the problem to register_qm_collector_caps() thus far, so it's not the output layer. It's in the filter_user_has_cap filter.

This is a WooCommerce Memberships system, a plugin which does play heavily with caps.

lkraav commented 7 years ago

The issue is caused by $trace = new QM_Backtrace call inside the filter_user_has_cap filter.

If I comment this block out and just let it return $user_caps, everything works again.

Any thoughts on this angle?

EDIT class QM_Backtrace is complicated at a level I don't think I'll be able to succeed in bisecting that one.

johnbillion commented 7 years ago

Cheers.

QM_Backtrace is used in a bunch of places throughout QM (most stack traces you see are generated by QM_Backtrace).

It could be that this sort of data collection for every user capability check simply uses too much memory, but memory exhaustion would show up in your PHP error logs.

Idea: Can you comment out these four lines and see if it makes a difference? https://github.com/johnbillion/query-monitor/blob/74700bd1c0eb05cb4382e2dfaf696444124808a2/classes/Backtrace.php#L56-L59

johnbillion commented 7 years ago

I've recently done some work to greatly reduce the amount of memory that QM uses (around 70% in some cases). If you want to try it out and see if it fixes this issue, check out the develop branch.

lkraav commented 7 years ago

@johnbillion does 2.17.0 already include this memory reduction work?

johnbillion commented 7 years ago

Not yet, no. It'll be in 2.18.0.

johnbillion commented 6 years ago

Closing this as a duplicate of #288.