stayallive / wp-sentry

A (unofficial) WordPress plugin reporting PHP and JavaScript errors to Sentry.
https://wordpress.org/plugins/wp-sentry-integration/
MIT License
300 stars 48 forks source link

Combination of WP_PROXY_HOST and early wp-sentry.php include results in no errors #157

Closed LordSimal closed 1 year ago

LordSimal commented 1 year ago

Refs: https://github.com/stayallive/wp-sentry/pull/155

We have certain websites which can only send errors via a HTTP proxy. It works fine if we just add

define('WP_PROXY_HOST', 'my.proxy.com');
define('WP_PROXY_PORT', '3142');

BUT this doesn't work anymore if we additionally add

require_once __DIR__ . '/wp-content/plugins/wp-sentry-integration/wp-sentry.php';

to our wp-config.php because we are now too early for the WP_HTTP_Proxy class to exist - therefore breaking the sentry functionality.

But we also can't do something like

require_once __DIR__ . '/wp-includes/class-wp-http-proxy.php';
require_once __DIR__ . '/wp-content/plugins/wp-sentry-integration/wp-sentry.php';

because in the wp-settings.php wordpress itself only does a require and not a require_once.

So how does one combine the early

require_once __DIR__ . '/wp-content/plugins/wp-sentry-integration/wp-sentry.php';

to fetch early PHP errors like in plugin bootstrapping but also set a HTTP proxy?

stayallive commented 1 year ago

Hmm, interesting... so I don't think there is an elegant solution...

The only solution I can think of is to copy the class-wp-http-proxy.php or it's functionality to make sure it's available to us when we need it and not rely on the version that WordPress ships. This could mean trouble but it's also a pretty stable API so I might be comfortable doing this.

If you have a better solution I am all ears but the require thing in the WordPress core is going to throw a wrench in the works every time... so unless we convince WP core to change it to require_once or something I don't think we can load it when we need to.

LordSimal commented 1 year ago

Lets see... https://core.trac.wordpress.org/ticket/58332#no0

stayallive commented 1 year ago

Nice, I'm gonna follow that, looking at the code of the class the most interesting bit is where it's calculated if the Sentry URL is excluded from being proxied. I'm not sure we even really need or want that, we could simply add our own "do not proxy" setting for when you do have WP proxy setting but don't need Sentry to be proxies for some reason.

LordSimal commented 1 year ago

Since Wordpress won't change I leave it up to you how you want to handle it.

stayallive commented 1 year ago

Sounds good, I think I'll take some of the code for creating the proxy URL but have our own "no proxy" option to disable it if needed. Thanks for trying anyway!

stayallive commented 1 year ago

v6.15.0 was just released fixing this, we no longer need the WP_HTTP_Proxy to be available so you should be good to go!