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

WP_SENTRY_VERSION not using theme version any longer #140

Closed visnetje closed 1 year ago

visnetje commented 1 year ago

Hi Alex,

As per the docs I am not defining WP_SENTRY_VERSION on my websites to make sure the theme version will be used:

(Optionally) define a version of your site; by default the theme version will be used. This is used for tracking at which version of your site the error occurred. When combined with release tracking this is a very powerful feature.

define( 'WP_SENTRY_VERSION', 'v6.0.0' );

It seems that at least the last 90 days the version is not reported any longer to Sentry events. Other variables like browser, environment and the WordPress version are still working fine.

I have defined the following in the WordPress config file (with the DSN keys redacted):

/** Configure Sentry */
define( 'WP_SENTRY_PHP_DSN', 'https://**************@********.ingest.sentry.io/*******' );
define( 'WP_SENTRY_BROWSER_DSN', 'https://**************@********.ingest.sentry.io/*******' );
define( 'WP_SENTRY_ENV', 'staging' );
define( 'WP_SENTRY_ERROR_TYPES', E_ALL & ~E_NOTICE );

As you can see WP Sentry does not recognize the default version from the theme:

wp-sentry-staging

If you're not able to reproduce this behavior, could you please suggest how to debug this?

Thanks!

stayallive commented 1 year ago

So this was removed by me on purpose, however I must admit I dropped the ball in a few ways on this.

First of all I did not document this change properly hence is why it's not mentioned in the changelogs for example.

Second... I don't exactly remember why I removed it in the first place and I did a bad job on documenting it!

This is the commit removing it: https://github.com/stayallive/wp-sentry/commit/dbd348af94b660185bc3dff7d57c70857e771dd0

I'll keep this open since either the documentation should be changed and a snippet provided allowing you to do it yourself or this should be put back in!

Very sorry for the confusion in the meantime!

visnetje commented 1 year ago

Appreciate the clarification! :)

Looking forward to the snippet, because wp_get_theme() is not loaded yet in the WordPress config file, so the behavior of defaulting to the theme version performed by the WP Sentry plugin was actually pretty practical.

piscis commented 1 year ago

Same here. We mimic the old behavior by implementing a filter that sets the release version based on the theme version or a constant.

<?php

add_filter('wp_sentry_options', function (\Sentry\Options $options) {

  if (defined('WP_SENTRY_VERSION')) {
    $options->setRelease(WP_SENTRY_VERSION);
  } else {

    $theme = wp_get_theme();

    if ($theme) {
      $options->setRelease($theme->get('Version'));
    } else {
      $options->setRelease('unkown');
    }
  }

  return $options;
});
visnetje commented 1 year ago

Hi @piscis , that solution works for getting PHP events to use the theme version, but unfortunately it doesn't work for JavaScript events, nor is the version detected properly in the WP Sentry admin page (under Tools).

piscis commented 1 year ago

@Visnetje you are right, but looking at: https://github.com/stayallive/wp-sentry/blob/b687648880c30b905d87c718c5e0c63e6bee8c89/src/class-wp-sentry-js-tracker.php#L80-L82

it should also be possible to implement a similar filter for the JS part.

OArnarsson commented 1 year ago

@stayallive would you be opposed to adding this back in, essentially reverting https://github.com/stayallive/wp-sentry/commit/dbd348af94b660185bc3dff7d57c70857e771dd0? This really would be an ideal default as others have mentioned.

Edit: I've taken the liberty of adding a PR regarding this issue, see https://github.com/stayallive/wp-sentry/pull/161

stayallive commented 1 year ago

Thanks everyone for chipping in (and @OArnarsson for making the PR), I've released the change as version 6.21.0 where the theme version will once again be the default Sentry release number (unless you specified one yourself ofcourse).

OArnarsson commented 1 year ago

@stayallive will you be releasing the newly updated version to the WP svn as well so we update via plugins?

stayallive commented 1 year ago

@OArnarsson I apologise, I have been making a few releases but apparently tags were not published... I've fixed it and all releases should be out and available. Sorry for the hold-up and thanks for the heads-up!