Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance Hints error: null passed to delete_by_url() when string expected #6887

Closed
joejoe04 opened this issue Aug 21, 2024 · 9 comments · Fixed by #6996
Closed

Performance Hints error: null passed to delete_by_url() when string expected #6887

joejoe04 opened this issue Aug 21, 2024 · 9 comments · Fixed by #6996
Assignees
Labels
effort: [XS] < 1 day of estimated development time priority: high Issues which should be resolved as quickly as possible severity: moderate Feature isn't working as expected but has work around to get same value type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@joejoe04
Copy link

joejoe04 commented Aug 21, 2024

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version ✅
  • Used the search feature to ensure that the bug hasn’t been reported before ✅

Describe the bug
Users on 3.16.4 reporting the following error being logged:

[14-Aug-2024 17:34:32 UTC] PHP Fatal error: Uncaught TypeError: Argument 1 passed to WP_Rocket\Engine\Common\PerformanceHints\Admin\Controller::delete_by_url() must be of the type string, null given, called in /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php on line 81 and defined in /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php:177
Stack trace:
#0 /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php(81): WP_Rocket\Engine\Common\PerformanceHints\Admin\Controller->delete_by_url(NULL)
#1 /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Subscriber.php(64): WP_Rocket\Engine\Common\PerformanceHints\Admin\Controller->delete_post(187146)
#2 /home/customer/www/mabelkatz.com/public_html/espanol/wp-includes/class-wp-hook.php(326 in /home/customer/www/mabelkatz.com/public_html/espanol/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php on line 177

It's related to attachments and this bit of code:

$url = get_permalink( $post_id );
if ( false === $url ) {
return;
}
$this->delete_by_url( $url );
}

It seems some plugins are returning null for attachment $urls. The code above only returns if $url is exactly false, allowing null to get past.

Could instead do something like this:

if ( ! is_string( $url ) {
  return;
}

To Reproduce
Steps to reproduce the behavior:

  1. Be on version 3.16.4 and use any plugin that returns null for attachment URLs

We're not sure exactly which plugins are doing this, but we have 3 cases reported so far that use a variety of different plugins.

@wordpressfan said it can be reproduced with this:

add_filter( 'attachment_link', function ( $permalink, $post_id ) {
    if ( 'attachment' === get_post_type( $post_id ) ) {
        return;
    }
    return $permalink;
}, 10, 2 );

Expected behavior
Any time the $url value is not a string, we should return to stop processing.

Additional context
Slack: https://wp-media.slack.com/archives/C43T1AYMQ/p1723658183566389
Case1: https://secure.helpscout.net/conversation/2679645797/507641/
Case2: https://secure.helpscout.net/conversation/2684619765/508416/
Case3: https://secure.helpscout.net/conversation/2685184550/508504/

Acceptance Criteria (for WP Media team use only)
Clear instructions for developers, to be added before the grooming

@webtrainingwheels
Copy link

@sandyfigueroa
Copy link
Contributor

@kskonovalov
Copy link

kskonovalov commented Sep 7, 2024

Faced the same issue preventing upload any media or plugins, error
Uncaught TypeError: WP_Rocket\Engine\Common\PerformanceHints\Admin\Controller::delete_by_url(): Argument #1 ($url) must be of type string, null given, called in wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php

Uploading media work again if deactivate WPRocket

Another way is a hotfix until it's fixed by plugin's authors

in
wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php

replace the delete_by_url method with

private function delete_by_url( $url ) {
    if(is_string($url)) {
      foreach ($this->factories as $factory) {
        $factory->queries()->delete_by_url(untrailingslashit($url));
      }
    }
}

@alfonso100
Copy link
Contributor

@Adrianadla
Copy link

@joejoe04
Copy link
Author

@juricazuanovic
Copy link
Contributor

@joejoe04
Copy link
Author

@piotrbak piotrbak added type: bug Indicates an unexpected problem or unintended behavior priority: medium Issues which are important, but no one will go out of business. needs: grooming severity: moderate Feature isn't working as expected but has work around to get same value labels Sep 27, 2024
@alfonso100
Copy link
Contributor

In 2 additional cases, users can also see the following message in the media uploader:

When trying to upload an image, we get the error in the WP Media screen of:
The server cannot process the image. This can happen if the server is busy or does not have enough resources to complete the task. Uploading a smaller image may help. Suggested maximum size is 2560 pixels.

And the error reported appears in the logs:

[12-Sep-2024 19:18:22 UTC] PHP Fatal error:  Uncaught TypeError: WP_Rocket\\Engine\\Common\\PerformanceHints\\Admin\\Controller::delete_by_url(): Argument #1 ($url) must be of type string, null given, called in public_html/wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php on line 81 and defined in /wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php:177\
  thrown in /wp-content/plugins/wp-rocket/inc/Engine/Common/PerformanceHints/Admin/Controller.php on line 177\

tickets:
https://secure.helpscout.net/conversation/2706213063/512091
https://secure.helpscout.net/conversation/2718520629/514450/

@piotrbak piotrbak added priority: high Issues which should be resolved as quickly as possible and removed priority: medium Issues which are important, but no one will go out of business. labels Sep 27, 2024
@remyperona remyperona added effort: [XS] < 1 day of estimated development time and removed needs: grooming labels Sep 30, 2024
@remyperona remyperona self-assigned this Sep 30, 2024
@piotrbak piotrbak added this to the 3.17.1 milestone Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [XS] < 1 day of estimated development time priority: high Issues which should be resolved as quickly as possible severity: moderate Feature isn't working as expected but has work around to get same value type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants