Skip to content

Throw EEXIST when ensureSymlink dest is a broken symlink - #1076

Merged
RyanZim merged 2 commits into
jprichardson:masterfrom
sarathfrancis90:fix-ensuresymlink-broken-dest
Jul 23, 2026
Merged

RyanZim merged 2 commits into
jprichardson:masterfrom
sarathfrancis90:fix-ensuresymlink-broken-dest

Conversation

@sarathfrancis90

Copy link
Copy Markdown
Contributor

Fixes #925.

ensureSymlink/ensureSymlinkSync stat the destination to check whether an existing symlink already points at srcpath. When that symlink is broken, following it fails and the raw ENOENT (stat '<dest>') bubbles up to the caller. That's surprising, since a symlink pointing at a different existing target already throws EEXIST — only the broken case leaked the stat error. @RyanZim noted the same direction in the issue.

I skip the idempotency check when the destination can't be followed, so the existing creation path reports EEXIST consistently whether the existing link is broken or just points elsewhere. Added async and sync regression tests; npm test passes locally.

ensureSymlink/ensureSymlinkSync stat() the destination to check whether
an existing symlink already points at srcpath. When that symlink is
broken, following it fails and the raw ENOENT (stat '<dest>') surfaced to
the caller, which is confusing: a symlink that points at a *different*
existing target already throws EEXIST. Skip the idempotency check when the
destination can't be followed so the existing creation path reports EEXIST
consistently, broken or not.

Closes jprichardson#925
Comment thread lib/ensure/symlink.js Outdated
When an existing dest is a symlink, the stat that checks whether it already
points at src is expected to fail with ENOENT for a broken link. Narrow the
catch so that only ENOENT is swallowed (falling through to the existing
EEXIST behavior) and any other error propagates instead of being hidden.
@sarathfrancis90

Copy link
Copy Markdown
Contributor Author

Good call. I've narrowed that catch so it only swallows ENOENT (the broken-link case we're actually expecting) and rethrows anything else, following the err.code pattern used elsewhere in the lib. Added tests for both the async and sync paths to make sure a non-ENOENT error now propagates.

@RyanZim
RyanZim merged commit 9c2d3c9 into jprichardson:master Jul 23, 2026
21 checks passed
@RyanZim

RyanZim commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Published in fs-extra@11.4.0 🎉

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

Successfully merging this pull request may close these issues.

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

2 participants