Studio: Fix no directory or file error on Windows

[katinthehatsite]

Related issues

Fixes STU-442

Proposed Changes

I could not reproduce this issue on Windows on my end but I noticed that several issues in Sentry point to downloadWordPress function in vendor/wp-now/src/download.ts. Specifically, the error seems to happen when there is a creation or interaction with the temporary directory that is used to store downloaded WordPress before moving it to final folder.

Running through Sentry errors, I noticed that we always place wordpress like this:

• C:\Users\USERNAME\AppData\Local\Temp\wordpress on Windows
• /var/folders/bj/{userSessionToken}/T/wordpress on macOS

My suspicion is that sometimes there might be multiple processes that are downloading WordPress at the same time which could create some conflicts with the temporary directory as it is now. In this PR, I suggest that we use unique parent directories for each download process so that it does not create those possible conflicts. After modification, each process would get its own unique path such as (example below is for Windows):

  C:\Users\USERNAME\AppData\Local\Temp\wordpress-download-xwfmXp\wordpress - Process 1
  C:\Users\USERNAME\AppData\Local\Temp\wordpress-download-ZhuU1E\wordpress - Process 2

We will also clean up these temporary directories once WordPress is moved from them to the final destination.

Testing Instructions

I think it would be sufficient to test adding a site with different WordPress versions that you don't have installed under /Library/Application Support/Studio/server-files/wordpress-version and confirm that the site is created successfully. But if you want to see some behind the scenes, you can use these steps:

• Pull the changes from this branch
• Apply the following diff:

index d82c9c40..b0ac8652 100644
--- a/vendor/wp-now/src/download.ts
+++ b/vendor/wp-now/src/download.ts
@@ -157,6 +157,7 @@ export async function downloadWordPress(
 ) {
        const finalFolder = getWordPressVersionPath( wordPressVersion );
        const tempDir = await fs.mkdtemp( path.join( os.tmpdir(), 'wordpress-download-' ) );
+       console.log( 'Temporary directory path:', tempDir );
 
        try {
                const { downloaded, statusCode } = await downloadFileAndUnzip( {

• Start the app with npm start (if switching directly from trunk, make sure to restart the app as we are modifying the main process)
• Observe the location of the temporary directory in the logs in your terminal where you started Studio. It will look something like: /var/folders/bj/1qgxs1qd6_z20tw9llkw842h0000gn/T
• Run in a Terminal on macOS open /var/folders/bj/1qgxs1qd6_z20tw9llkw842h0000gn/T/ (replace with your path that you got in the previous step)
• Back in Studio, click on Add site and select a WP version that you don't have added under /Library/Application Support/Studio/server-files/wordpress-version
• Observe that once the download process starts, you can see a sub-directory created under /var/folders/bj/1qgxs1qd6_z20tw9llkw842h0000gn/T/. It should have a structure similar to /wordpress-download-{randomLetters}.
• Confirm that once the download is completed, the sub-directory is removed
• Confirm that the site gets added and starts successfully

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[gavande1]

I think your observation is spot-on. The errors indicate a problem with cleaning up the temporary folder. The changes look good to me! 👍

I would wait a few days before closing the Linear issue since we are not sure if this PR will actually fix the problem.

[fredrikekelund]

So, the theory is that if users create multiple sites in quick succession with a WordPress version that wasn't downloaded before they created the first site, multiple downloadWordPress calls will get kicked off simultaneously, causing them to modify the same files, right? This seems like a reasonable theory to me, and the use of fs.mkdtemp is a good way to mitigate that.

I came to think of the sequential helper in src/lib/proxy-server.ts, but I don't think it would add much here. This solution covers the needs here already. I've also tested it, and it works well 👍

One question that came up for me is whether we really need to call downloadWordPress in the startWPNow function..? Haven't we already guaranteed that WordPress is installed in the project directory at that point?

[katinthehatsite]

I would wait a few days before closing the Linear issue since we are not sure if this PR will actually fix the problem.

Sounds good, I was going to double-check on Slack at first to see what is the process for this and what steps we need to take in Sentry.

[fredrikekelund]

Looking up all the related Sentry issues might be a hassle, because I know there were multiple ones, but Sentry has a feature to mark issues as resolved in the next release. That would be a very good fit for this scenario, and it gives us an alert if it turns out the same errors continue appearing in the next release, so it increases visibility.

[katinthehatsite]

So, the theory is that if users create multiple sites in quick succession with a WordPress version that wasn't downloaded before they created the first site, multiple downloadWordPress calls will get kicked off simultaneously, causing them to modify the same files, right?

Yeah, I inferred this from the fact that the error was always similar and was always related to some file missing but it was never exactly the same file. I also saw some logs related to auto-updates on one of the error instances in Sentry which made me think that two processes could be using the downloadWordPress function at once.

One question that came up for me is whether we really need to call downloadWordPress in the startWPNow function..? Haven't we already guaranteed that WordPress is installed in the project directory at that point?

I did not look too closely into that bit. From quick look, it might be but we if we want to clean it up, we should double-check the whole flow to make sure nothing breaks and can clean it up after. We can open an issue to look through this. Regardless, I think the changes from this PR would be useful.

[katinthehatsite]

Looking up all the related Sentry issues might be a hassle, because I know there were multiple ones, but Sentry has a feature to mark issues as resolved in the next release. That would be a very good fit for this scenario, and it gives us an alert if it turns out the same errors continue appearing in the next release, so it increases visibility.

I will take a look, thanks 👀

[katinthehatsite]

Added STU-509 to check the data flow for downloadWordPress in startWPNow.