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

fs: calling mkdir in fs.cp function can ignore EEXIST error #53534

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ShenHongFei
Copy link

This PR mainly aims to solve the following problem. During the execution of the fsp.cp function, due to other file operations, the target directory of fsp.cp is created in parallel. The actual situation may be more complicated. Multiple file operations are executed concurrently, resulting in an error in the internal fsp.cp when calling mkdir, because the folder has been created by other file operations. In this case, you can actually ignore this error and continue with the subsequent file copying operation.

import { promises as fsp } from 'fs'

const src = 'T:/src/'
const dst = 'T:/out/'

try {
    await fsp.rm(dst, { recursive: true })
} catch { }

await Promise.all([
    fsp.cp(src, dst, {
        recursive: true,
        force: true,
        errorOnExist: false,
        mode: 0
    }),
    
    // example other file operations in parallel
    fsp.mkdir(dst, { recursive: true })
])

// throws EEXIST: file already exists, mkdir 'T:/out/'
// at mkdir()
// at mkDirAndCopy() lib/internal/fs/cp/cp.js
// at onDir() lib/internal/fs/cp/cp.js

The relevant code is in lib/internal/fs/cp/cp.js.
destStat is empty here, it did not exist when checking the folder before.

function onDir(srcStat, destStat, src, dest, opts) {
  if (!destStat) return mkDirAndCopy(srcStat.mode, src, dest, opts);
  return copyDir(src, dest, opts);
}

async function mkDirAndCopy(srcMode, src, dest, opts) {
  await mkdir(dest);
  await copyDir(src, dest, opts);
  return setDestMode(dest, srcMode);
}

I want to ignore the EEXIST error thrown by mkdir

async function mkDirAndCopy(srcMode, src, dest, opts) {
  try {
    await mkdir(dest);
  } catch (error) {
    // If the folder already exists, skip it.
    if (error.code !== 'EEXIST')
      throw error;
  }

  await copyDir(src, dest, opts);
  return setDestMode(dest, srcMode);
}
@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. labels Jun 21, 2024
await mkdir(dest);
} catch (error) {
// If the folder already exists, skip it.
if (error.code !== 'EEXIST')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull-request needs a test.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test and adjusted it to ignore this error only when the errorOnExist parameter is false

@ShenHongFei ShenHongFei requested a review from anonrig June 24, 2024 04:31
@ShenHongFei
Copy link
Author

@anonrig I added a test and changed the logic to ignore the error only when the errorOnExist parameter is false. What else do I need to do? Can you take another look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run.
3 participants