SEMrush_Client thrown an Error (Empty_Property_Exception) & Unused validation code

Agreeement

[x] I've read and understood the contribution guidelines.
[x] I've searched for any related issues and avoided creating a duplicate issue.

Versions

[x] WordPress 5.8.1 (latest and or previous)
[x] Yoast Seo (17.1) (latest and or previous 16.xx)
[x] WordPress SEO 17.1.2 (latest and or previous)

Note

This maybe common issue on yoast that have been posted on wordpress(org) plugin support: Yoast – XML Update & Critical Error Issues

I've found that semrush feature still being faces, maybe this bug ( or maybe feature?) that need manually resolve and thrown error before.

Case

When user try to create/list/add/edit (on post-(new).php|edit.php) yoast thrown the error with WPSEO Premium

Array
(
    [type] => 1
    [message] => Uncaught Yoast\WP\SEO\Exceptions\SEMrush\Tokens\Empty_Property_Exception: Token creation failed. Property `access_token` cannot be empty. in [ABSPATH]/wp-content/plugins/wordpress-seo/src/values/semrush/semrush-token.php:62
Stack trace:
#0 [ABSPATH]/wp-content/plugins/wordpress-seo/src/config/semrush-client.php(187): Yoast\WP\SEO\Values\SEMrush\SEMrush_Token->__construct()
#1 [ABSPATH]/wp-content/plugins/wordpress-seo/src/config/semrush-client.php(75): Yoast\WP\SEO\Config\SEMrush_Client->get_token_from_storage()
#2 [ABSPATH]/wp-content/plugins/wordpress-seo/src/generated/container.php(1703): Yoast\WP\SEO\Config\SEMrush_Client->__construct()
#3 [ABSPATH]/wp-content/plugins/wordpress-seo/src/generated/container.php(880): Yoast\WP\SEO\Generated\Cached_Container->getSEMrushClientService()
#4 [ABSPATH]/wp-content/plugins/wordpress-seo/src/generated/container.php(3563): Yoast\WP\SEO\Generated\Cached_Container->getSEMrushLoginActionService()
#5 [ABSPATH]/wp-content/plugins/wordpress-seo/vendor_prefixed/symfony/dependency-injection/Container.php(271): Yoast\WP\SEO\Generated\Cached_Container->getSEMrushRouteService()
#6 [ABSPATH]/wp-content/plugins/wordpress-seo/src/loader.php(216): YoastSEO_Vendor\Symfony\Component\DependencyInjection\Container->get()
#7 [ABSPATH]/wp-includes/class-wp-hook.php(303): Yoast\WP\SEO\Loader->load_routes()
#8 [ABSPATH]/wp-includes/class-wp-hook.php(327): WP_Hook->apply_filters()
#9 [ABSPATH]/wp-includes/plugin.php(470): WP_Hook->do_action()
#10 [ABSPATH]/wp-includes/rest-api.php(537): do_action()
#11 [ABSPATH]/wp-content/plugins/wpseo-premium/classes/premium-metabox.php(69): rest_get_server()
#12 [ABSPATH]/wp-content/plugins/wpseo-premium/classes/premium-metabox.php(225): WPSEO_Premium_Metabox::are_content_endpoints_available()
#13 [ABSPATH]/wp-content/plugins/wpseo-premium/classes/premium-metabox.php(123): WPSEO_Premium_Metabox->get_rest_api_config()
#14 [ABSPATH]/wp-content/plugins/wpseo-premium/classes/premium-metabox.php(111): WPSEO_Premium_Metabox->send_data_to_assets()
#15 [ABSPATH]/wp-includes/class-wp-hook.php(303): WPSEO_Premium_Metabox->enqueue_assets()
#16 [ABSPATH]/wp-includes/class-wp-hook.php(327): WP_Hook->apply_filters()
#17 [ABSPATH]/wp-includes/plugin.php(470): WP_Hook->do_action()
#18 [ABSPATH]/wp-admin/admin-header.php(102): do_action()
#19 [ABSPATH]/wp-admin/edit.php(396): require_once('...')
#20 {main}
  thrown
    [file] => [ABSPATH]/wp-content/plugins/wordpress-seo/src/values/semrush/semrush-token.php
    [line] => 62
)

How can we reproduce this behavior?

Method:

https://github.com/Yoast/wordpress-seo/blob/813bf2e50575b69b47175e5d153748100595c520/src/config/semrush-client.php#L175

[Trace] - Produced By

public function get_token_from_storage() {
    $tokens = $this->options_helper->get( self::TOKEN_OPTION );
    if ( empty( $tokens ) ) {
        return null;
    }
    return new SEMrush_Token(
        $tokens['access_token'],
        $tokens['refresh_token'],
        $tokens['expires'],
        $tokens['has_expired'],
        $tokens['created_at']
    );
}

[Trace] - Constructor

https://github.com/Yoast/wordpress-seo/blob/813bf2e50575b69b47175e5d153748100595c520/src/config/semrush-client.php#L75

Propose

1. NOTE

Adding try - catch on property set constructor, because there are of dependency uses SEMrush_Client. eg: SEMrush_Client ( also called on generated container / dependency injection services )

[Trace] - Login Action

https://github.com/Yoast/wordpress-seo/blob/813bf2e50575b69b47175e5d153748100595c520/src/actions/semrush/semrush-login-action.php#L25

Called by metabox or WPSEO Premium container services on Route Registration and Metabox

[Trace] - Login Action

The classes that required SEMRush_Client have method that check of loggedin.

https://github.com/Yoast/wordpress-seo/blob/813bf2e50575b69b47175e5d153748100595c520/src/actions/semrush/semrush-login-action.php#L53

[Trace] - Validation

https://github.com/Yoast/wordpress-seo/blob/813bf2e50575b69b47175e5d153748100595c520/src/config/semrush-client.php#L145

https://github.com/Yoast/wordpress-seo/blob/813bf2e50575b69b47175e5d153748100595c520/src/config/semrush-client.php#L156

validations of empty( $this->token ) will be useless, because constructor already thrown an error.

2. Code Proposal

public function __construct()
{
    /// code before ...
    try {
        $this->token  = $this->get_token_from_storage();
    } catch(Empty_Property_Exception $e) {
         $this->token = null;
    }
}

Correct Me If I'am Wrong

Thanks

@ArrayIterator can you provide clear steps on how to reproduce your error from an empty installation? From your stacktrace it looks like the whole stack is within a hook call? Do you have a valid SEMRush account connected to your user?

I'm sorry I don't know how to exactly produce an error how to make it thrown error. Cause it suddenly happen to my clients site after update the wpseo plugin.

Reproduce

I suspect the error started from a failure to save data, or conflict with other plugins. But when you want to produce error by code, I've already use this.

// fake options
WPSEO_Options::save_option('semrush_tokens', [
    'access_token' => '', // do empty
    'refresh_token' => 'random', // maybe random
    'expires' => time()+100,
    'has_expired' => false,
    'created_at' => time(),
]);

On bugs report about unused validations empty() event that will be useless, cause constructor already thrown an error, and validation about empty token will be useless.

Connection Status

Auth / Connection about SEMRush already valid untill plugins update, but suddenly thrown an error.

Stack Trace

[ABSPATH]/wp-content/plugins/wordpress-seo/src/generated/container.php(1703): Yoast\WP\SEO\Config\SEMrush_Client->__construct()

This is cause about generated container service direct call to the service.

Another Error Reference / Issues

WordPress : WordPress Recovery Alert – Yoast Error
Unresolved Issue : https://github.com/Yoast/wordpress-seo/issues/16295

Small line but affected to another methods / code. The error thrown on editor (and post list) page in dashboard.

Yoast / wordpress-seo