jprichardson / node-fs-extra

Node.js: extra methods for the fs object like copy(), remove(), mkdirs()
MIT License
9.47k stars 772 forks source link

Better error message for ensureSymlink if the current existing link is broken #925

Open aviladev opened 3 years ago

aviladev commented 3 years ago

I'm not sure if this is the expected behavior.

Here's what I'm trying to do... basically I want to replicate what ln -sfn does. I would like to create a link to a file in my home bin folder. If I move the target file to another directory, then as expected, that link will now be broken. That's why I want to recreate that link, updating the target path. The problem is when I do this (calling createSymlink) node throws an error (because the link is broken):

node:internal/process/promises:246
          triggerUncaughtException(err, true /* fromPromise */);
          ^

[Error: ENOENT: no such file or directory, stat '/home/myusername/bin/rgb-toggle'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'stat',
  path: '/home/myusername/bin/rgb-toggle'
}

The error is thrown by Node when calling fs.stat or fs.statSync, here's what happens when I call it in Node REPL:

> fs.statSync(`${os.homedir()}/bin/rgb-toggle`)
Uncaught Error: ENOENT: no such file or directory, stat '/home/myusername/bin/rgb-toggle'
    at Object.statSync (node:fs:1536:3) {
  errno: -2,
  syscall: 'stat',
  code: 'ENOENT',
  path: '/home/myusername/bin/rgb-toggle'
}

The link exists, only it's broken. Looks like that is expected behavior for Node because it will call POSIX stat, and as the description:

stat() retrieve information about the file pointed to by pathname.

However for that purpose we can use fs.lstat or fs.lstatSync which will call POSIX lstat, which as described:

lstat() is identical to stat(), except that if pathname is a symbolic link, then it returns information about the link itself, not the file that the link refers to.

Calling fs.lstatSync succeeds:

> fs.lstatSync(`${os.homedir()}/bin/rgb-toggle`)
Stats {
  dev: 66307,
  mode: 41471,
  nlink: 1,
  uid: 1000,
  gid: 1000,
  rdev: 0,
  blksize: 4096,
  ino: 5396104,
  size: 60,
  blocks: 8,
  atimeMs: 1634260915813.8984,
  mtimeMs: 1634260915813.8984,
  ctimeMs: 1634260915813.8984,
  birthtimeMs: 1634260915813.8984,
  atime: 2021-10-15T01:21:55.814Z,
  mtime: 2021-10-15T01:21:55.814Z,
  ctime: 2021-10-15T01:21:55.814Z,
  birthtime: 2021-10-15T01:21:55.814Z
}

Shouldn't fs.ensureSymlink check for broken symlink and update it (given the new target path exists)?

RyanZim commented 3 years ago

ping @manidlou

RyanZim commented 2 years ago

If you call ensureSymlink on an existing symlink, with a different target, it fails with Error: EEXIST: file already exists. It'd be weird for this to succeed if the symlink was broken. I'll admit, ENOENT is an odd error to throw here. If possible, we should try to change this to throw EEXIST even when the symlink is broken.