Skip to content

Performance Lab Led to a Critical Error with Site Health #1237

Closed

Description

@bobbyferran

Bug Description

Steps to reproduce

  1. Go to 'tools'
  2. Click on 'site health'
  3. Scroll down to '....' (n/a)
  4. See error: There has been a critical error on this website.

Screenshots

Additional Context

  • PHP Version: 8.2.18
  • OS: [iOS]
  • Browser: [Firefox]
  • Plugin Version: [e.g. 22]
  • Device: [Macbook]

<This only occurs with site health. Otherwise, the site works and site health became available again when I disabled performance lab.>

Activity

  1. bobbyferran commented on May 22, 2024

    @bobbyferran
    Author

    Plugin Version: Performance Lab (version 3.1.0)

  2. bobbyferran commented on May 22, 2024

    @bobbyferran
    Author
    Screenshot 2024-05-22 at 10 00 25 AM
  3. westonruter commented on May 22, 2024

    @westonruter
    Member

    If you deactivate Performance Lab then the error goes away?

    We'll need to get more information from you about what is going on. Specifically we'll need the specific PHP error that is occuring. Can you get that from the error log?

  4. bobbyferran commented on May 22, 2024

    @bobbyferran
    Author

    Yes, and then site health works again.
    This is the error log:

    Error Details
    =============
    An error of type E_ERROR was caused in line 106 of the file /home3/flowergu/public_html/wp-content/plugins/performance-lab/includes/site-health/audit-autoloaded-options/helper.php. Error message: Uncaught TypeError: strlen(): Argument #1 ($string) must be of type string, array given in /home3/flowergu/public_html/wp-content/plugins/performance-lab/includes/site-health/audit-autoloaded-options/helper.php:106
    Stack trace:
    #0 /home3/flowergu/public_html/wp-content/plugins/performance-lab/includes/site-health/audit-autoloaded-options/helper.php(22): perflab_aao_autoloaded_options_size()
    #1 /home3/flowergu/public_html/wp-admin/includes/class-wp-site-health.php(194): perflab_aao_autoloaded_options_test()
    #2 /home3/flowergu/public_html/wp-admin/includes/class-wp-site-health.php(145): WP_Site_Health->perform_test('perflab_aao_aut...')
    #3 /home3/flowergu/public_html/wp-includes/class-wp-hook.php(324): WP_Site_Health->enqueue_scripts('site-health.php')
    #4 /home3/flowergu/public_html/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters(NULL, Array)
    #5 /home3/flowergu/public_html/wp-includes/plugin.php(517): WP_Hook->do_action(Array)
    #6 /home3/flowergu/public_html/wp-admin/admin-header.php(118): do_action('admin_enqueue_s...', 'site-health.php')
    #7 /home3/flowergu/public_html/wp-admin/site-health.php(96): require_once('/home3/flowergu...')
    #8 {main}
    thrown
    
  5. self-assigned this
    on May 22, 2024
  6. westonruter commented on May 22, 2024

    @westonruter
    Member

    Thank you. I can see this is a bug introduced in #1179. The code in question:

    foreach ( $all_options as $option_name => $option_value ) {
    $total_length += strlen( $option_value );
    }

    The issue is that you have an autoloaded option which is an array (which is perfectly fine) but the code is assuming all of the options are strings (I guess since previously it was only working with serialized data).

    Working on a fix...

  7. westonruter commented on May 22, 2024

    @westonruter
    Member

    Interesting. In testing locally, I'm seeing that wp_load_alloptions() only returns an array of strings, even when there are autoloaded arrays/objects they are returned as serialized data.

    @bobbyferran Do you have an external object cache perhaps?

  8. bobbyferran commented on May 22, 2024

    @bobbyferran
    Author

    I have Docket Cache but this was previously installed with performance lab and there was not this issue. This issue just started today without adding any new plugins. I also have WP Fastest Cache and the Bluehost assets only cache.

  9. westonruter commented on May 22, 2024

    @westonruter
    Member

    OK, thanks. I'm able to reproduce the issue with Docket Cache. It seems when Docket Cache is active it results in wp_load_alloptions() returning an array of mixed values, including arrays. I do not see this issue when I try another object cache plugin, namely SQLite Object Cache. When I check WordPress core, I see that it is intending to store an array of serialized (string) values in the alloptions cache:

    if ( 'yes' === $autoload ) {
    	$alloptions            = wp_load_alloptions( true );
    	$alloptions[ $option ] = $serialized_value;
    	wp_cache_set( 'alloptions', $alloptions, 'options' );
    }

    So this makes me think that indeed Docket Cache's object cache implementation is incorrect, as it seems to be storing unserialized values instead of serialized ones. I think this should be reported as a bug to Docket Cache, although we should also harden Performance Lab against such object cache implementation bugs.

    In the mean time, here is some workaround PHP plugin code that "fixes" the Docket Cache compatibility problem:

    add_filter(
    	'alloptions',
    	static function ( array $alloptions ): array {
    		return array_map(
    			static function ( $value ): string {
    				if ( ! is_string( $value ) ) {
    					return serialize( $value );
    				} else {
    					return $value;
    				}
    			},
    			$alloptions
    		);
    	}
    );
  10. bobbyferran commented on May 22, 2024

    @bobbyferran
    Author

    Ok, thank you. I will also report this issue to Docket Cache. Thank you again.

  11. westonruter commented on May 22, 2024

    @westonruter
    Member

    @bobbyferran Great. Could you share the link to your Docket Cache issue when you've reported?

    Also, I've opened #1238 to harden Performance Lab against this issue.

  12. mukeshpanchal27 commented on May 23, 2024

    @mukeshpanchal27
    Member

    @westonruter, does this plugin issue alter the behaviour of WP functions in a way that might break the default WP functionality?

  13. westonruter commented on May 23, 2024

    @westonruter
    Member

    @mukeshpanchal27 Yes, that seems likely.

  14. nawawi commented on May 23, 2024

    @nawawi

    Hi there,

    does this plugin issue alter the behaviour of WP functions in a way that might break the default WP functionality?

    Nope. It store cache as plain PHP to get performance benefit from OPcache.

    You may do a quick fix by adding maybe_serialize:

    File: includes/site-health/audit-autoloaded-options/helper.php
    Line: 106

    Before:

    foreach ( $all_options as $option_name => $option_value ) {
        $total_length += strlen( $option_value );
    }

    Fix:

    foreach ( $all_options as $option_name => $option_value ) {
        $total_length += strlen( maybe_serialize($option_value));
    }

    Thanks.

  15. westonruter commented on May 23, 2024

    @westonruter
    Member

    @nawawi Right, but WordPress core is returning an array of serialized options when calling wp_load_alloptions(). Therefore, to ensure there are no compatibility problems, it seems Docket Cache should also serialize the option values for consistency.

    We implemented a similar workaround in #1238.

  16. nawawi commented on May 23, 2024

    @nawawi

    Alright then, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

[Type] BugAn existing feature is broken

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions