Givewp: Give Sessions Enhancement - Fix Cache Breaking and Research DB Sessions

Created on 19 Apr 2017  路  13Comments  路  Source: impress-org/givewp

Issue Overview

Give seems to be triggering some errors with the Dreamhost Varnish check URL tool. This may or may not be related to how we trigger sessions, or what we do with caching.

Current Behavior

Here's the test site:
https://givewp.breakwpdh.com

As soon as Give is activated, the Dreamhost Varnish tool throws some errors, reporting that it cannot properly cache the website:
image

Expected Behavior

Give should not be triggering those errors. It should be only setting a PHP Session after the visitor clicks the DONATE button. It should NOT be affecting the cache header in any way (except maybe on the Donation Confirmation page).

Steps to Reproduce (for bugs)

I have credentials for that test site available upon request -- won't post them publicly here.

All 13 comments

Update from @Ipstenu

I checked on the site and I see this:
Cookie: PHPSESSID=Cf4CmQXpJvvl76aaNuy9U0

image

And that's certainly something the Varnish plugin is picking up as a reason why cache won't happen.
A quick look at the code and I saw this: https://github.com/WordImpress/Give/blob/master/give.php#L208

So my guess would be the logic in here - https://github.com/WordImpress/Give/blob/master/includes/class-give-session.php - is not catching that nothing's yet going on.

If I had to pick a place to start, it would be here: https://github.com/WordImpress/Give/blob/master/includes/class-give-session.php#L270

That looks like the default is "If you can use sessions, we use sessions!" which may be triggering this? https://github.com/WordImpress/Give/blob/master/includes/class-give-session.php#L137

But basically yes, the plugin is totally setting sessions.

FWIW, the no-cache in cache control may be a false-negative. I suspect once you fix the sessions, it will vanish. It often does in my debugging adventures!

I'm researching what it would take to take a more WooCommerce-like approach by rolling out own session API that can be configured to only start when an action is taken on a donation form. Because of that, I'm bumping it off 1.8.7 to 1.8.8 for now. Depending on the complexity, it could be moved into 2.0. It is a priority, though, and I want to get taken care of soon rather than later but is also a significant change that will require extensive QA and updated unit tests.

I have seen it as well and I tried define( 'GIVE_USE_PHP_SESSIONS' , false);. For me it does solve part of the issue as I'm running a multiserver setup. Plus side is that it then sets a give_wp_session cookie.

Same problem with Redis Page Cache https://github.com/pressjitsu/pj-page-cache-red but they have the option to configure
cache_config['ignore_cookies'] = array( 'wordpress_test_cookie', 'openstat_ref', 'give_wp_session' );
So I included define( 'GIVE_USE_PHP_SESSIONS' , false); in my wp-config.php file and added give_wp_session cookie to the statement above in the config file and all seems to work.

Random idea: what if we show the confirmation message directly within the form area via AJAX. And it has a link with session information in it that will take the donor to their donation history if they prefer. Something like that might help us (1) have more reliable confirmation messages; (2) perhaps give a more direct way to transmit the session across pages via query parameters in the url.

https://github.com/markoheijnen/Give/commit/b3adb93cbd1ded74e1e0afdcc44f46b2122b3348 has been my approach to solve this by doing the load session logic later so checking which page it's on is possible.

@markoheijnen thanks for the reference. The issue with this method is you're only loading sessions on the CPT single and archive pages. If someone were to embed a donation form via a shortcode outside those locations it create an issue.

I have updated the code to also incorporate taxonomies and shortcodes

Hoping this might be helpful to work out between EDD, Give, Ninja Forms & WP Simple Pay. Possibly others I don't know about. Reposting this in existing related issues on each repo to discuss.

Up until now, like you we've been using the core of WP Session Manager plugin to manage sessions within WP Simple Pay (Lite & Pro).

However, more recently this hasn't been working on some hosts such as Flywheel & Patheon. After doing some digging & testing, we're now beta testing the core of Patheon's WP Native PHP Sessions plugin. So far beta tests on various hosts are working out great.

Any reason not to go this direction and include WP Native PHP Sessions like this? Is there an easier more straightforward way to handle sessions across modern managed hosting in our plugins?

Update: We're back to using WP Session Manager because of conflicts with other plugins using session_start().

We're also using WP Session Manager's latest version, which now stores session data in a custom table. We're also defaulting to this, but allowing override by using a constant _or_ if Pantheon's session plugin is detected.

Details at https://github.com/moonstonemedia/WP-Simple-Pay-Lite-for-Stripe/issues/38#issuecomment-355664226

Suggestions, discussion, etc always welcome.

We have another customer at DH with this issue and I tried adding the give_wp_session cookie to our special whitelist (the same way we allow Google Analytics) but while that seems to work for the cookie, it's throwing down a page Age header of 0, which ... well logically it doesn't cache that. I tried digging around but I don't see any reason why THAT would be happening. :head scratch:

Was this page helpful?
0 / 5 - 0 ratings