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.

Sign up for GitHub

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

Jump to bottom

fs,macos: fs.watch() behavior change in node >= 10.16.0 #29460

Closed
bnoordhuis opened this issue Sep 5, 2019 · 8 comments
Closed

fs,macos: fs.watch() behavior change in node >= 10.16.0 #29460

bnoordhuis opened this issue Sep 5, 2019 · 8 comments
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. macos Issues and PRs related to the macOS platform / OSX.

Comments

@bnoordhuis
Copy link
Member

This is a container bug for issues caused by PR libuv/libuv#2082. Disparate reports but the root cause is the linked PR.

#25856
#27869
#28512
#28882
#28917

Fixing it turned out to be not so easy (libuv/libuv#2313) and simply reverting trades one regression for another.
@bnoordhuis bnoordhuis added confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. macos Issues and PRs related to the macOS platform / OSX. labels Sep 5, 2019
This was referenced Sep 5, 2019
Closed
Closed
Closed
bnoordhuis added a commit to bnoordhuis/libuv that referenced this issue Sep 5, 2019
@bnoordhuis
Revert "fsevents: really watch files with fsevents on macos 10.7+"
76baf24 
This commit introduces numerous regressions, see the Node.js container
bug and the issues linked therein.

I did not revert the changes to the test suite since those work okay
with and without the reverted commit.

I also had to roll back (parts of) the following commits:

* 6602fca unix: fall back to kqueue on older macOS systems
* 09ba477 bsd: plug uv_fs_event_start() error path fd leak

This reverts commit 2d2af38.

Fixes: nodejs/node#29460
Refs: libuv#2082
@bnoordhuis bnoordhuis mentioned this issue Sep 5, 2019
Closed
@bnoordhuis
Copy link
Member Author

libuv/libuv#2452 - let's see how a revert is received.

vtjnash added a commit to vtjnash/libuv that referenced this issue Sep 7, 2019
@vtjnash
fsevents: stop using fsevents to watch files
7a3ff31 
Goes back to just using it to watch folders,
but keeps the other logic changes around.

Refs: libuv#387
Refs: libuv#2082
Refs: libuv#1572
Refs: libuv#2452
Refs: nodejs/node#29460
@vtjnash vtjnash mentioned this issue Sep 7, 2019
Closed
@tvvignesh
Copy link

@bnoordhuis @vtjnash Faced the same problem on our end when running with nodemon with Node.js 11.11 in production - Will be removing nodemon and running it with Node.js directly - Anyways, would be great to get an idea on the version of Node.js where this fix will be rolled out.

Running it inside Docker in Amazon Linux 2 - Thus, would like to point out that this is not a MacOS specific issue as the label suggests.

Thanks in advance.

@bnoordhuis
Copy link
Member Author

@tvvignesh Then your issue is something else. This issue is specifically about a macos-only change.

vtjnash added a commit to vtjnash/libuv that referenced this issue Sep 18, 2019
@vtjnash
fsevents: stop using fsevents to watch files
e23b9b7 
Goes back to just using it to watch folders,
but keeps the other logic changes around.

Refs: libuv#387
Refs: libuv#2082
Refs: libuv#1572
Refs: libuv#2452
Refs: nodejs/node#29460
This was referenced Sep 19, 2019
Closed
Open
wearhere added a commit to mixmaxhq/custody that referenced this issue Sep 19, 2019
@wearhere
Fix logfile tailing under Node 10.16.0
5caea0c 
`fs.watch` does not consistently emit `'change'` events on Node 10.16.0,
possibly due to nodejs/node#29460. `fs.watchFile`
still works, but is less efficient, per the docs, so fall back to that only if necessary and also suggest that users run custody under an older version of Node.

I explored several Node file-watching libraries (e.g. chokidar, sane) to try to
find a more performant solution for Node 10.16.0 and it seems that they pretty
much all fall back to `fs.watch` / `fs.watchFile`, unfortunately. sane offers
the option to use watchman, but that requires installing a daemon so is a
non-starter.

Fixes #82.

#83 tracks reverting this when
the underlying Node issue has been fixed.

Tested that log files update both when running custody under v8.9.3 and
v10.16.0, and that `node-tail` only uses `fs.watchFile` in the latter.
@wearhere wearhere mentioned this issue Sep 19, 2019
Merged
@mjbvz mjbvz mentioned this issue Sep 26, 2019
Closed
saghul pushed a commit to libuv/libuv that referenced this issue Sep 26, 2019
@vtjnash @saghul
fsevents: stop using fsevents to watch files
97b85e8 
Goes back to just using it to watch folders,
but keeps the other logic changes around.

Refs: #387
Refs: #2082
Refs: #1572
Refs: nodejs/node#29460
Fixes: #2488
Closes: #2452
PR-URL: #2459
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
deepak1556 added a commit to electron/electron that referenced this issue Oct 3, 2019
@deepak1556
fix: fs.watch() behavior change in node >= 10.16.0
6003f0c 
This reverts the patch from electron/node#100
which never got merged due to reasons outlined in libuv/libuv#2313

* Adds new patches that backports libuv/libuv#2459
  and libuv/libuv#2460

Based on nodejs/node#29460
@deepak1556 deepak1556 mentioned this issue Oct 3, 2019
Merged
6 tasks
deepak1556 added a commit to electron/electron that referenced this issue Oct 4, 2019
@deepak1556
fix: fs.watch() behavior change in node >= 10.16.0
b7ef041 
This reverts the patch from electron/node#100
which never got merged due to reasons outlined in libuv/libuv#2313

* Adds new patches that backports libuv/libuv#2459
  and libuv/libuv#2460

Based on nodejs/node#29460
deepak1556 added a commit to electron/electron that referenced this issue Oct 4, 2019
@deepak1556
fix: fs.watch() behavior change in node >= 10.16.0
6b30a71 
This reverts the patch from electron/node#100
which never got merged due to reasons outlined in libuv/libuv#2313

* Adds new patches that backports libuv/libuv#2459
  and libuv/libuv#2460

Based on nodejs/node#29460
This was referenced Oct 4, 2019
Merged
Merged
deepak1556 added a commit to electron/electron that referenced this issue Oct 4, 2019
@deepak1556
fix: fs.watch() behavior change in node >= 10.16.0
ca4571e 
This reverts the patch from electron/node#100
which never got merged due to reasons outlined in libuv/libuv#2313

* Adds new patches that backports libuv/libuv#2459
  and libuv/libuv#2460

Based on nodejs/node#29460
deepak1556 added a commit to electron/electron that referenced this issue Oct 7, 2019
@deepak1556
fix: fs.watch() behavior change in node >= 10.16.0 (#20408)
c85648a 
This reverts the patch from electron/node#100
which never got merged due to reasons outlined in libuv/libuv#2313

* Adds new patches that backports libuv/libuv#2459
  and libuv/libuv#2460

Based on nodejs/node#29460
deepak1556 added a commit to electron/electron that referenced this issue Oct 7, 2019
@deepak1556
fix: fs.watch() behavior change in node >= 10.16.0 (#20429)
e501930 
This reverts the patch from electron/node#100
which never got merged due to reasons outlined in libuv/libuv#2313

* Adds new patches that backports libuv/libuv#2459
  and libuv/libuv#2460

Based on nodejs/node#29460
deepak1556 added a commit to electron/electron that referenced this issue Oct 7, 2019
@deepak1556
fix: fs.watch() behavior change in node >= 10.16.0 (#20430)
77af675 
This reverts the patch from electron/node#100
which never got merged due to reasons outlined in libuv/libuv#2313

* Adds new patches that backports libuv/libuv#2459
  and libuv/libuv#2460

Based on nodejs/node#29460
deepak1556 added a commit to electron/electron that referenced this issue Oct 7, 2019
@deepak1556
fix: fs.watch() behavior change in node >= 10.16.0 (#20433)
486a8ea 
This reverts the patch from electron/node#100
which never got merged due to reasons outlined in libuv/libuv#2313

* Adds new patches that backports libuv/libuv#2459
  and libuv/libuv#2460

Based on nodejs/node#29460
@bnoordhuis bnoordhuis mentioned this issue Oct 9, 2019
Closed
@gap777
Copy link

gap777 commented Dec 6, 2019

Based on your investigations, any suggested workarounds until a fix is released?

@bnoordhuis
Copy link
Member Author

The issue was fixed in libuv v1.33.0 and that was released as part of node v13.0.1 and v12.13.1. If you're on v10.x, you'll need to wait a little longer - no ETA.

@dosentmatter
Copy link

I have a project using webpack@4 watchers that uses watchpack -> chokidar -> fsevents.

I recently upgraded from node v11.9.0 to v13.9.0. I am watching a lot of directories, because I have server-side and client-side HMR. Also, some packages in node_modules are being watched because some of them are custom packages that developers may want to chain watchers to output to node_modules and then be watched by webpack.

I get the error:

2020-03-09 08:30 node[39625] (FSEvents.framework) FSEventStreamStart: register_with_server: ERROR: f2d_register_rpc() => (null) (-21)

for node v13.9.0, but not v11.9.0.

There are a few workarounds I've found.

  1. Change server-side HMR config to poll for watch.
  2. Change server-side HMR config to ignore /node_modules/ for watch.
    Not sure why, but changing the client-side config doesn't seem to help.

@vtjnash
Copy link
Contributor

vtjnash commented Mar 9, 2020

For future readers of this thread, I think the message there is from interpreting an EMFILE response (ulimit -n). As far as I know though, nothing significant should have changed about how a directory is watched recursively.

@sindresorhus sindresorhus mentioned this issue Jul 3, 2020
Merged
@marcins marcins mentioned this issue Jul 27, 2020
Closed
@jasnell
Copy link
Member

jasnell commented May 12, 2021

Since Node.js 10.x is no longer supported, and since this appears to be fixed in more recent versions, I'm closing this issue. We can revisit if it turns out that it is still an issue

@jasnell jasnell closed this as completed May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. macos Issues and PRs related to the macOS platform / OSX.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Revert "fsevents: really watch files with fsevents on macos 10.7+" bnoordhuis/libuv
6 participants
@bnoordhuis @vtjnash @jasnell @tvvignesh @dosentmatter @gap777