rollbar / rollbar-php-wordpress

Official WordPress plugin from Rollbar, Inc.
https://rollbar.com/
GNU General Public License v2.0
15 stars 20 forks source link

Unnecessary user session with every request #85

Closed rubas closed 2 years ago

rubas commented 5 years ago

Related to #82. Introduced with commit github-36: support default settings.

The plugin starts (imo) an unnecessary user session with every request.

Starting a session for every user is an application anti-pattern. Serving pages to users with sessions cannot be done out of a cache, so creating a session for every visitor inherently makes your application unscalable.

equinoxmatt commented 5 years ago

Any plans to addresss this? Somewhat ironically, rollbar is being pinged with errors from this plugin:

E_WARNING: session_start(): Cannot start session when headers already sent

I have had to disable anything below E_WARNING to stop excessive API requests.

rubas commented 5 years ago

@equinoxmatt, I just disable the registerSession function to keep the E_WARNING notifications in rollbar.

E-VANCE commented 4 years ago

Same here, seeing warnings piling up in Rollbar with regards to session_start()... Seems like #88 could fix this – any plans for integrating?

fliespl commented 4 years ago

Any plans on fixing this issue? I don't see any reasons for using $_SESSION at all to begin with.

$_SESSION part could be easily moved to options if needed or even displayed flashMessage using current screen - no need to keep it between requests.

Wordpress is now reporting critical issue with this plugin:

An active PHP session was detected

A PHP session was created by a session_start() function call. This interferes with REST API and loopback requests. The session should be closed by session_write_close() before making any HTTP requests.

wilsonjackson commented 3 years ago

I agree with @fliespl — it doesn't seem like this plugin has any business starting a session. This also causes problems with caching proxy setups because the session cookie is stripped and never set on the client, so every request tries to start a new session, resulting in every response setting Cache-Control: no-store, no-cache, must-revalidate. I think session usage should be removed from this plugin entirely.

I'm willing to contribute a PR if that's acceptable. I think the idea to use wp_options for the flash message makes sense. As far as I could tell there's no other session usage in the plugin, is that correct?

anthropos9 commented 3 years ago

I'm seeing these errors happening using v.2.6.1 of the plugin. I'm not sure why the wp-cron.php needs to use sessions, but that's what I'm seeing.

brandoncc commented 3 years ago

I'm also getting an error every minute from wp-cron.php. 1440 errors a day on Rollbar adds up on my bill. Any idea when this might get fixed?

UPDATE: I manually copied the fixed function from #88 into Settings.php and it is working great.

ethanclevenger91 commented 3 years ago

The session seems to only be used for the sake of flashing messages to the user. Could this be swapped out for the admin_notices hook?

fliespl commented 2 years ago

For now I went with disabling this hook in my setup inside mu-plugins file.

remove_action('init', array('Rollbar\Wordpress\Settings', 'registerSession'));

The cron fix is fine, but it also starts session on frontend and in the end kills caching with some loadbalancers (cause it sets expired header in the past).