ericmann / wp-session-manager

Session manager for WordPress
GNU General Public License v2.0
268 stars 45 forks source link

"session_set_save_handler(): Cannot change save handler when headers already sent" on CRON runs #89

Open bluestarstudios opened 2 years ago

bluestarstudios commented 2 years ago

@ericmann Thanks for an incredible plugin. Following up on issue https://github.com/ericmann/wp-session-manager/issues/81. It looks like the latest version still throws the same warning.

The plugin throws a warning session_set_save_handler(): Cannot change save handler when headers already sent on every single cron run which is called via wget on our server. Here's the stack trace:

 in EAMann\Sessionz\Manager::initialize called at /wp-content/plugins/wp-session-manager/wp-session-manager.php (49)
 in wp_session_manager_initialize called at /wp-includes/class-wp-hook.php (301)
 in WP_Hook::apply_filters called at /wp-includes/class-wp-hook.php (327)
 in WP_Hook::do_action called at /wp-includes/plugin.php (470)
 in do_action called at /wp-settings.php (441)
 in require_once called at /wp-config.php (18)
 in require_once called at /wp-load.php (50)
 in require_once called at /wp-cron.php (44)

Looking through the code, in version 4.2.0 you made sure that session_start() doesn't run when DOING_CRON is defined. Which is great. However, maybe add_action('plugins_loaded', 'wp_session_manager_initialize', 1, 0); should be wrapped in the same logic? session_set_save_handler($manager); on line 133 in /wp-session-manager/vendor/ericmann/sessionz/php/Manager.php is the cause for the issue.

We're running PHP 7.4.30 via fpm-fcgi, WordPress 5.8.3, and WP Session Manager 4.2.0.

Let me know how I can help in further debugging this and getting a fix pushed.

Thanks!

davelavoie commented 2 years ago

I've got the same issue. I tried to apply the DOING_CRON check to the 'wp_session_manager_initialize' hook, but from what I understand of issue #81, it could interfere with the cleanup cron job?

It's not clear what happened, but it looks like that fix was commited here : https://github.com/ericmann/wp-session-manager/commit/58da8e92cc97660d8e5e48032c931142d30057d6

image

But it was then overriden by that commit: https://github.com/ericmann/wp-session-manager/commit/79a174e45ef22e4bb8e70ebcb120be7709d8bc9a

image

So I'm not sure what would be the best way to prevent this warning now...

bluestarstudios commented 1 year ago

@ericmann Hi Eric! Checking in on this. Would appreciate your expertise, after all, you know the code much better than any of us could.

It seems as though you must have had some reason why you chose not to wrap add_action('plugins_loaded', 'wp_session_manager_initialize', 1, 0); in the DOING_CRON check. Do you remember why that was?

How can the code be ever-so-slightly altered to make sure the session_set_save_handler doesn't throw a warning during CRON runs?

Thanks!

bluestarstudios commented 1 year ago

Accidentally closed the issue. Re-opening as it's still an active problem.