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

Renaming a folder which has subfolders throws exception on Windows #1380

Open
eyza-cod2 opened this issue Oct 26, 2024 · 11 comments
Open

Renaming a folder which has subfolders throws exception on Windows #1380

eyza-cod2 opened this issue Oct 26, 2024 · 11 comments

Comments

@eyza-cod2
Copy link

Renaming a folder which has subfolders throws exception on Windows.

  • Chokidar version [4.0.1]
  • Node version [18, 20, 22]
  • OS version: [Windows 11]

To Reproduce:

Create following folder structure:
TEST_FOLDER\folder1
TEST_FOLDER\folder2
TEST_FOLDER\folder2\subfolder

Setup chokidar to watch folder 'TEST_FOLDER'

Rename folder "folder2" to "folder2_renamed"

Exception is thrown:

Error: EPERM: operation not permitted, rename 'C:\Users\tomas\chokidar-bug\TEST_FOLDER\folder2' -> 'C:\Users\tomas\chokidar-bug\TEST_FOLDER\folder2_renamed'
    at Object.renameSync (node:fs:1030:11)
    at renameFolders (C:\Users\tomas\chokidar-bug\index.js:50:8)
    at async main (C:\Users\tomas\chokidar-bug\index.js:18:9) {
  errno: -4048,
  code: 'EPERM',
  syscall: 'rename',
  path: 'C:\\Users\\tomas\\chokidar-bug\\TEST_FOLDER\\folder2',
  dest: 'C:\\Users\\tomas\\chokidar-bug\\TEST_FOLDER\\folder2_renamed'
}

Test code:

const chokidar = require('chokidar');
const fs = require('fs');
const path = require('path');

const rootFolder = path.join(__dirname, 'TEST_FOLDER');

async function main() {
    let watcher;
    try {
        prepareFolderStructure();

        watcher = chokidar.watch(rootFolder, { persistent: true });

        watcher.on('all', (event, path) => {
            console.log(event, path);
        });

        await renameFolders();

    } catch (error) {
        console.error(`Error: ${error.message}`);
        throw error;

    } finally {
        if (watcher) {
            await watcher.close();
            console.log('Watcher closed');
        }
    }
}

function prepareFolderStructure() {    
    if (fs.existsSync(rootFolder)) {
        fs.rmSync(rootFolder, { recursive: true });
    }
    // Create the directory structure
    fs.mkdirSync(rootFolder);
    fs.mkdirSync(path.join(rootFolder, 'folder1'));
    fs.mkdirSync(path.join(rootFolder, 'folder2'));
    fs.mkdirSync(path.join(rootFolder, 'folder2', 'subfolder'));
}

async function renameFolders() {
    await new Promise(resolve => setTimeout(resolve, 500));

    fs.renameSync(path.join(rootFolder, 'folder1'), path.join(rootFolder, 'folder1_renamed')); // Rename folder1 - OK

    await new Promise(resolve => setTimeout(resolve, 500));

    fs.renameSync(path.join(rootFolder, 'folder2'), path.join(rootFolder, 'folder2_renamed')); // Rename folder2 with subfolder - IT THROWS AN ERROR!

    await new Promise(resolve => setTimeout(resolve, 500));
}

main();

Expected behavior
No exception, like it is on linux / macos

Additional context
CI:
image

Repo:
https://github.com/eyza-cod2/chokidar-bug

@ketansahugit
Copy link

Hi, I've attempted to resolve this issue, but it continues to throw an error unless we temporarily stop Chokidar while renaming the folder that contains a subfolder.

@eyza-cod2
Copy link
Author

Ye, that might help when you control the renaming action, but when some file is renamed outside the scope of your app, there is propably no way how to fix that.

I wanted to use this package but after this error I was forced to use something else.

@MikeJerred
Copy link

I'm running into this issue as well. Only workaround seems to be to set usePolling: true in the chokidar config, but the performance impact of this can be high.

@MikeJerred
Copy link

MikeJerred commented Jan 9, 2025

Looks like this issue is long-standing:

#1031
#1281
#1346
#1398

@43081j
Copy link
Collaborator

43081j commented Jan 10, 2025

ok sorry this took me a while to get to

this isn't a chokidar problem it seems. see the reproduction below:

import * as fs from 'node:fs/promises';
import {watch} from 'node:fs';
import * as path from 'node:path';
import {fileURLToPath} from 'node:url';

const __dirname = fileURLToPath(new URL('.', import.meta.url));
const rootFolder = path.join(__dirname, 'TEST_FOLDER');

async function main() {
  await prepareFolderStructure();

  // Watch a directory _under_ the one we're going to rename
  watch(path.join(rootFolder, 'folder1'), console.log);

  await renameFolders();
}

async function prepareFolderStructure() {
  await fs.rm(rootFolder, {recursive: true, force: true});
  await fs.rm(path.resolve(rootFolder, '../TEST_FOLDER2'), {force: true});

  await fs.mkdir(rootFolder);
  await fs.mkdir(path.join(rootFolder, 'folder1'));
}

async function renameFolders() {
  await new Promise(resolve => setTimeout(resolve, 500));
  await fs.rename(rootFolder, path.resolve(rootFolder, '../TEST_FOLDER2'));
}

main();

basically, if you try to rename the parent directory of a directory you're watching with node's watch, you'll get an EPERM error.

i'm not sure this is fixable on our end (or by anyone). we'd have to remove our listeners just before a parent is removed (i.e. when we get another FS event telling us its happening, but it already happened by then i imagine)

@MikeJerred
Copy link

Thanks for looking into this! There is an issue on the nodejs github: nodejs/node#31702 but getting an error when the root folder that is being watched is deleted seems like reasonable behavior.

This code works fine:

import * as fs from 'node:fs/promises';
import {watch} from 'node:fs';
import * as path from 'node:path';
import {fileURLToPath} from 'node:url';
import { watch as chokidarWatch } from 'chokidar';

const __dirname = fileURLToPath(new URL('.', import.meta.url));
const rootFolder = path.join(__dirname, 'TEST_FOLDER');

async function main() {
  await prepareFolderStructure();

  // Watch a directory _under_ the one we're going to rename
  watch(rootFolder, { recursive: true }, console.log);
  // chokidarWatch(rootFolder).on('all', console.log);

  await renameFolders();
}

async function prepareFolderStructure() {
  await fs.rm(rootFolder, {recursive: true, force: true});

  await fs.mkdir(rootFolder);
  await fs.mkdir(path.join(rootFolder, 'folder1'));
  await fs.mkdir(path.join(rootFolder, 'folder1', 'subfolder'));
}

async function renameFolders() {
  await new Promise(resolve => setTimeout(resolve, 500));
  await fs.rename(path.join(rootFolder, 'folder1'), path.resolve(rootFolder, 'folder1_renamed'));
}

main();

Switching line 14 to watch using chokidar gives the error.

I take it that chokidar creates a separate watcher for each subdirectory rather than using the recursive option?

@43081j
Copy link
Collaborator

43081j commented Jan 10, 2025

Your example works because you are watching the directory you are renaming

My example specifically watches a directory below the one I'm renaming

So yes indeed we don't use recursive because it has some problems still across platforms. But also even if we did use it, this problem still exists unrelated to chokidar (see my repro)

@MikeJerred
Copy link

Getting an error when renaming a parent folder of the one we are watching is reasonable behavior - that is not the problem we are having on this issue.
The problem we have, is that renaming folders inside of the watched folder is giving errors, see the first post on the issue:

Create following folder structure:
TEST_FOLDER\folder1
TEST_FOLDER\folder2
TEST_FOLDER\folder2\subfolder

Setup chokidar to watch folder 'TEST_FOLDER'

Rename folder "folder2" to "folder2_renamed"

If I am watching TEST_FOLDER, I would except to be able to add/rename/delete files inside of it and not get errors.

This is not a problem with node, because as highlighted in the code I put above, we can simply switch one line to watch using node instead of chokidar and it no longer crashes.

@43081j
Copy link
Collaborator

43081j commented Jan 10, 2025

You are renamingfolder2, a directory which contains subfolder, a directory we are currently watching

That is why you get the error (same as in my repro)

One day, when we move to recursive, it'll probably go away. But ultimately it's probably not a bug in node, it's expected behaviour when trying to move something above a watcher

And no, you didn't just change it to use chokidar. You changed the path being watched. My repro specifically watches a directory below the one I'm renaming

Your code changed that (look again, my code watches folder1 to repro the exact problem the same way it is happening in the original problem under the hood)

@MikeJerred
Copy link

What I am saying is that if you take the code I wrote above, and apply this diff which is a 1 line change:

14,15c14,15
<   watch(rootFolder, { recursive: true }, console.log);   
<   // chokidarWatch(rootFolder).on('all', console.log);   
---
>   // watch(rootFolder, { recursive: true }, console.log);
>   chokidarWatch(rootFolder).on('all', console.log);

The error is reproduced, whereas originally when just using node there was no error.

The problem is that chokidar is spinning up a node watcher for each subdirectory, blocking renames of those subdirectories. Agreed that switching to use recursive internally will very likely resolve the issue, do you have any info about the problems that exist with using that?

@43081j
Copy link
Collaborator

43081j commented Jan 10, 2025

Yes that code produces an error because of the exact repro code I posted

Moving the parent directory of a directory being watched results in that error, as the repro I posted discovered

The code you posted immediately after mine changed the directory being watched and is why it then began to work with node.

Chokidar using a watcher per directory isn't a problem but is the reason you get this error. One day when we use recursive , it'll go away (in that the behaviour will still exist in node but we will avoid it)

I don't remember the problems with recursive. It'll all be explained in an issue somewhere but will need to do some searching

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants