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

Misleading error message when recursive fs.mkdir fails with EACCES #31481

Closed
isaacs opened this issue Jan 23, 2020 · 1 comment
Closed

Misleading error message when recursive fs.mkdir fails with EACCES #31481

isaacs opened this issue Jan 23, 2020 · 1 comment
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@isaacs
Copy link
Contributor

isaacs commented Jan 23, 2020

  • Version: v13.4.0
  • Platform: darwin
  • Subsystem: fs
> fs.mkdirSync('/var/this/fails', { recursive: true })
    at Object.mkdirSync (fs.js:854:3) {
  errno: -2,
  syscall: 'mkdir',
  code: 'ENOENT',
  path: '/var/this/fails'
}

> fs.mkdir('/var/this/fails', { recursive: true }, console.error)
undefined
> [Error: ENOENT: no such file or directory, mkdir] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'mkdir'
}

Problems with this error:

  1. No path is provided in the async version. (Reported in missing error.path at fs.mkdir callback on last call #28015)
  2. Error code is ENOENT when it should be EACCES.
  3. Error errno is -2 when it should be -13.

A manual recursive implementation (like make-dir or mkdirp) will fail when it hits this:

> fs.mkdirSync('/var/this', { recursive: false })
Uncaught Error: EACCES: permission denied, mkdir '/var/this'
    at Object.mkdirSync (fs.js:854:3) {
  errno: -13,
  syscall: 'mkdir',
  code: 'EACCES',
  path: '/var/this'
}
> fs.mkdir('/var/this', { recursive: false }, console.error)
undefined
> [Error: EACCES: permission denied, mkdir '/var/this'] {
  errno: -13,
  code: 'EACCES',
  syscall: 'mkdir',
  path: '/var/this'
}

Note that the recursive: true implementation will fail in the same way with ENOENT even if the parent path does in fact exist and the syscall is failing with EACCES:

> fs.mkdirSync('/var/this', { recursive: true })
Uncaught Error: ENOENT: no such file or directory, mkdir '/var/this'
    at Object.mkdirSync (fs.js:854:3) {
  errno: -2,
  syscall: 'mkdir',
  code: 'ENOENT',
  path: '/var/this'
> fs.mkdir('/var/this', { recursive: true }, console.error)
undefined
> [Error: ENOENT: no such file or directory, mkdir] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'mkdir'
}
@richardlau richardlau added the fs Issues and PRs related to the fs subsystem / file system. label Jan 23, 2020
@isaacs
Copy link
Contributor Author

isaacs commented Jan 23, 2020

cc @bcoe

bcoe added a commit to bcoe/node-1 that referenced this issue Jan 25, 2020
When creating directories recursively, the logic should bail
immediately on UV_EACCES and bubble the error to the user.

Fixes: nodejs#31481
@Trott Trott closed this as completed in f284290 Jan 27, 2020
bcoe added a commit to bcoe/node-1 that referenced this issue Jan 29, 2020
mkdir('/foo/bar', { recursive: true }) and mkdirSync will now return
the path of the first folder created. This matches more closely
mkdirp's behavior, and is useful for setting folder permissions.

Refs: nodejs#31481
bcoe added a commit that referenced this issue Jan 31, 2020
mkdir('/foo/bar', { recursive: true }) and mkdirSync will now return
the path of the first folder created. This matches more closely
mkdirp's behavior, and is useful for setting folder permissions.

PR-URL: #31530
Refs: #31481
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this issue Feb 17, 2020
When creating directories recursively, the logic should bail
immediately on UV_EACCES and bubble the error to the user.

PR-URL: #31505
Fixes: #31481
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 10, 2020
mkdir('/foo/bar', { recursive: true }) and mkdirSync will now return
the path of the first folder created. This matches more closely
mkdirp's behavior, and is useful for setting folder permissions.

PR-URL: #31530
Refs: #31481
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this issue Mar 15, 2020
When creating directories recursively, the logic should bail
immediately on UV_EACCES and bubble the error to the user.

PR-URL: #31505
Fixes: #31481
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this issue Mar 17, 2020
When creating directories recursively, the logic should bail
immediately on UV_EACCES and bubble the error to the user.

PR-URL: #31505
Fixes: #31481
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this issue Mar 30, 2020
When creating directories recursively, the logic should bail
immediately on UV_EACCES and bubble the error to the user.

PR-URL: #31505
Fixes: #31481
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit to targos/node that referenced this issue Apr 25, 2020
mkdir('/foo/bar', { recursive: true }) and mkdirSync will now return
the path of the first folder created. This matches more closely
mkdirp's behavior, and is useful for setting folder permissions.

PR-URL: nodejs#31530
Refs: nodejs#31481
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
targos pushed a commit that referenced this issue Apr 28, 2020
mkdir('/foo/bar', { recursive: true }) and mkdirSync will now return
the path of the first folder created. This matches more closely
mkdirp's behavior, and is useful for setting folder permissions.

PR-URL: #31530
Refs: #31481
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
facebook-github-bot pushed a commit to facebook/react-native that referenced this issue Mar 17, 2023
…36448)

Summary:
```javascript
      // workaround nodejs/node#31481
      // todo: remove the ENOENT error check when we drop node.js 13 support
      case 'ENOENT':
      case 'EACCES':
```

Well, there was a TODO comment asking to remove `ENOENT` error check when react native drop nodejs 13 support since nodejs 14 has fixed the bug that motivated this check. Now react native minimum nodejs version supported is nodejs 14, so this check is not needed anymore.

## Changelog

[Internal] [Fixed] - Removes `ENOENT` error check

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Pull Request resolved: #36448

Reviewed By: NickGerleman

Differential Revision: D44025087

Pulled By: cortinico

fbshipit-source-id: f42cb7105a088e2eea207da12d4e63c6d26f3c77
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this issue May 22, 2023
…acebook#36448)

Summary:
```javascript
      // workaround nodejs/node#31481
      // todo: remove the ENOENT error check when we drop node.js 13 support
      case 'ENOENT':
      case 'EACCES':
```

Well, there was a TODO comment asking to remove `ENOENT` error check when react native drop nodejs 13 support since nodejs 14 has fixed the bug that motivated this check. Now react native minimum nodejs version supported is nodejs 14, so this check is not needed anymore.

## Changelog

[Internal] [Fixed] - Removes `ENOENT` error check

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Pull Request resolved: facebook#36448

Reviewed By: NickGerleman

Differential Revision: D44025087

Pulled By: cortinico

fbshipit-source-id: f42cb7105a088e2eea207da12d4e63c6d26f3c77
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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants