wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
690 stars 216 forks source link

Cache creation generating 'mkdir(): No such file or directory' error message in the logs #6049

Open mviniciosbarreto opened 1 year ago

mviniciosbarreto commented 1 year ago

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

Describe the bug When the cache is created/generated, there are constant 'mkdir(): No such file or directory' error message in the logs, It most likely happen due to our version of the WP core function wp_mkdir_p() is not creating the directories recursively as the core one does.

Expected behavior We shouldn't see error message regarding directories creation in the logs.

Additional context Slack: https://wp-media.slack.com/archives/C43T1AYMQ/p1689601308340879 Tickets: https://secure.helpscout.net/conversation/2297886007/429797?folderId=7546764 https://secure.helpscout.net/conversation/2206273657/412518?folderId=2683093

Acceptance Criteria

  1. The error should not popup in the logs
  2. If something deletes parent directory during the cache creation, child directory should be recreated recursively
  3. WP Rocket should still be able to create directories and cache files without regression, including query string cache files and all configuration files during activation

Backlog Grooming (for WP Media dev team use only)

piotrbak commented 1 year ago

We need to investigate why it happened in 3.14.1, the AC can be added afterwards

Tabrisrp commented 1 year ago

Are we able to reproduce the issue?

I could not find any change in 3.14.1 that would impact the cache creation. The one filesystem-related change is linked to the WPML compatibility, and it's for cache deletion.

I can see some differences between our mkdir_p version and the core one, notably related to permissions, it might be interesting to investigate a change there?

piotrbak commented 1 year ago

@Tabrisrp I can see similar ticket from the past, not sure why for this specific user it caused a problem after updating to 3.14.1.

@mviniciusbarreto Would it be possible to get access to the affected website, so we could investigate this further?

piotrbak commented 1 year ago

@Tabrisrp We have access here, with steps.

@rosamillan Could you ask customer if we could investigate this problem on his website?

JanThiel commented 1 year ago

@Tabrisrp This is not a regression. The issue is more a conceptual issue when your code encounters parallel processing. Like using a web-server-cluster or a WP-Cron runner like Cavalcade which can run parallel processes.

To reproduce: Use the Page Cache "Preload" for a rather big and deep hierarchical page and delete the cache root directory for that page ( wp-content/cache/wp-rocket/<domain> ) from the filesystem while the preload job runs and after it started creating the first child level (like <CACHE-ROOT>/products/services/ ). This will reproduce the scenario where a cache flush will do exactly that and delete the whole directory path. Partly including the cache root for the domain as well.

Your custom recursive mkdir will be at any given level of the page and is not able to create a directory there as the parent directories are missing. Thus the reported warning. Using the native mkdir with the recursive flag would be able to handle this scenario. It would simply create the missing parent folder hierarchy.

But I belive I wrote most of this in the support tickets already.

danieliser commented 1 year ago

I want to add (as one of the reporters), that this wasn't limited to a single version or recent version. These have been showing up in our logs for a very long time. Was easy to ignore them as we found no real damage, but they are making it difficult now to find the other more limited errors as they occur dozens of times per day.

I officially tracked them to WP Rocket about 12 months ago, was hoping they would resolve after some time (updates), due to others reporting them, but alax.

Checking our slack logs (where we send fatal errors), oldest indications are likely in 2020

Context for the image below, I searched this error exactly, sorted oldest first. Popup Maker is in reference to the site it occurred on.

The error messages/logs were identical for each of those to the one I sent in my email report.

image
danieliser commented 1 year ago

Will also add, that my testing required modifying core files to log out statements and make @mkdir not silently error.

Using the native mkdir with the recursive flag would be able to handle this scenario. It would simply create the missing parent folder hierarchy.

In doing so I found that, yes the recurrsive flag in mkdir likely should resolve it. I didn't not test that flag on our live sites wp files, but theoretically it was the primary change I suggested in my ticket.

The other option was using normal mkdir, but checking every parent folder exists and creating them first.

CrochetFeve0251 commented 1 year ago

Reproduce the problem

I got the issue but that would be quite complex to reproduce manually as we need to remove the parent folder while it created the parent but not the child yet. I am still trying to find how to reproduce that. However I will still provide a solution.

Identify the root cause

The root issue is that we are not ensuring that the parent folder exists before creating the child.

Scope a solution

A solution would be to guard our rocket_mkdir method by checking if the parent exists before writing the child folder. This way we would have a check that is done in a really close timeframe to the moment it is created.

Tests related to that method should be updated too.

Estimate the effort

Effort XS

Tabrisrp commented 1 year ago

I would add that we could check the wp_mkdir_p() to see the differences with our version and updated ours to match if needed.

viobru commented 1 year ago

Another customer is facing similar issues. Apart from the mkdir ones, she is also getting warnings about rmdir and open_basedir.

Slack: https://wp-media.slack.com/archives/C43T1AYMQ/p1692718697448869 HS ticket: https://secure.helpscout.net/conversation/2337094960/438438

danieliser commented 1 year ago

Curious if any patches have already been released on this one? I went over changelogs and found nothing that seemed related, but I noticed that we were seeing these dozens of times per day but suddenly none since the 16th. 2 releases seem to be possible in that time-line.

piotrbak commented 1 year ago

@danieliser We didn't release anything that would be fixing this bug directly. However, the issue could be related to the fact of excessive cache clearing.

@CrochetFeve0251 If we guard rocket_mkdir against this error, we'd still not create the cache in this scenario. Do you see any disadvantages of making sure that mkdir creates directories recursively? Updated the AC there, let's see if we can figure that out

danieliser commented 1 year ago

@piotrbak can you explain that in more detail? These errors were occurring even on days nobody logged in to the admin (no cache clearing at least manually).

Also the only viable solution is using mkdir recursively, or manually looping over all folders in the path to check & create if they don't exist. I've tested the mkdir change on our sites already and it does eliminate the issue. So unless there is some side effect I'm not aware of, its the best path forward.

CrochetFeve0251 commented 1 year ago

Scope a solution

Another way to solve this issue would be to create our custom Filesystem extending the WordPress one.

For that we will have to do the following class Rocket_Filesystem:

class Rocket_Filesystem extends WP_Filesystem_Direct {
/**
     * Creates a directory.
     *
     * @since 2.5.0
     *
     * @param string           $path  Path for new directory.
     * @param int|false        $chmod Optional. The permissions as octal number (or false to skip chmod).
     *                                Default false.
     * @param string|int|false $chown Optional. A user name or number (or false to skip chown).
     *                                Default false.
     * @param string|int|false $chgrp Optional. A group name or number (or false to skip chgrp).
     *                                Default false.
     * @return bool True on success, false on failure.
     */
    public function mkdir( $path, $chmod = false, $chown = false, $chgrp = false, $recursive = false ) {
        // Safe mode fails with a trailing slash under certain PHP versions.
        $path = untrailingslashit( $path );

        if ( empty( $path ) ) {
            return false;
        }

        if ( ! $chmod ) {
            $chmod = FS_CHMOD_DIR;
        }

        if ( ! @mkdir( $path, 0777, $recursive ) ) {
            return false;
        }

        $this->chmod( $path, $chmod );

        if ( $chown ) {
            $this->chown( $path, $chown );
        }

        if ( $chgrp ) {
            $this->chgrp( $path, $chgrp );
        }

        return true;
    }
}

Once we did that we will then have to change the filesystem class used inside rocket_direct_filesystem to our new class.

Finally we could get rid of the old logic inside wp_mkdir_p to use our new filesystem.

Estimate effort Effort S

Tabrisrp commented 1 year ago

looks good to me, on the long term having our own Filesystem class was something we always wanted to do, and would help us solve this kind of issues

dlinstedt commented 3 months ago

This appears to be back - using wpRocket 3.16, using Rocket.net servers, latest wordpress Error Level: E_WARNING Message: mkdir(): No such file or directory File: /wp-admin/includes/class-wp-filesystem-direct.php Line: 558

TussendoorHQ commented 2 months ago

Just wanted to let you know that we too are still experiencing this problem while using WP Rocket version 3.16.2.1.

Stacktrace:

PHP Warning mkdir(): No such file or directory [internal] mkdir /wp/wp-admin/includes/class-wp-filesystem-direct.php:558 WP_Filesystem_Direct::mkdir /app/plugins/wp-rocket/inc/functions/files.php:1187 rocket_mkdir /app/plugins/wp-rocket/inc/functions/files.php:1228 rocket_mkdir_p /app/plugins/wp-rocket/inc/classes/Buffer/class-cache.php:286 WP_Rocket\Buffer\Cache::maybe_process_buffer [internal] ob_end_flush /wp/wp-includes/functions.php:5420 wp_ob_end_flush_all /wp/wp-includes/class-wp-hook.php:324 WP_Hook::apply_filters /wp/wp-includes/class-wp-hook.php:348 WP_Hook::do_action /wp/wp-includes/plugin.php:517 do_action /wp/wp-includes/load.php:1270 shutdown_action_hook [internal] [main]

Apart frrom all the information given in this issue it might me interesting to know that we are using roots/bedrock for the installation with the "Managed WordPress host and Bedrock" solution to set the document root correctly. Thus we are using the .htaccess file to set the document root.

If any more information is needed we are happy to provide.

joejoe04 commented 2 months ago

New related case

viobru commented 1 week ago

Related: https://secure.helpscout.net/conversation/2722420120/515297