feature: Add cross-platform filesystem watcher (IOWatch)#475
Conversation
Add IOWatch API for watching directories for file creation, modification, and deletion events. Supports JVM (java.nio.file.WatchService), Node.js (fs.watch), and Scala Native (polling-based). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a cross-platform file system watcher (IOWatch) with implementations for JVM (using java.nio.file.WatchService), Node.js (using fs.watch), and Scala Native (using a polling-based approach). Feedback focuses on improving the robustness of the JVM implementation by recursively registering newly created directories and protecting the background watcher thread from exceptions thrown in user-provided handlers. Additionally, there are suggestions to address platform-specific limitations of Node.js recursive watching on Linux and to improve the API usability by ensuring default arguments are correctly inherited in the IOWatch object.
|
|
||
| // If recursive and a new directory was created, register it | ||
| if options.recursive && kind == ENTRY_CREATE && Files.isDirectory(fullPath) then | ||
| registerDirectory(fullPath) |
There was a problem hiding this comment.
When a new directory is created in recursive mode, simply registering the directory itself is insufficient if that directory already contains subdirectories (e.g., during a cp -r or a move operation). To ensure all nested items are watched, you should perform a recursive walk to register all subdirectories found within the newly created path.
Files.walkFileTree(
fullPath,
new SimpleFileVisitor[Path]:
override def preVisitDirectory(dir: Path, attrs: BasicFileAttributes): FileVisitResult =
registerDirectory(dir)
FileVisitResult.CONTINUE
)| if !NodeFSModule.existsSync(path.path) || !NodeFSModule.statSync(path.path).isDirectory() then | ||
| throw IOOperationException(s"Watch path is not a directory: ${path.path}") | ||
|
|
||
| val watchOptions = js.Dynamic.literal(recursive = options.recursive, persistent = false) |
There was a problem hiding this comment.
Node.js fs.watch throws an ERR_FEATURE_UNAVAILABLE_ON_PLATFORM error on Linux when the recursive option is set to true. Since this is a cross-platform library, you should handle this to avoid a crash. Consider checking the platform and either falling back to non-recursive watching (perhaps with a warning) or implementing manual recursion for Linux environments.
| ) | ||
| _impl | ||
|
|
||
| override def watch(path: IOPath, options: WatchOptions)(handler: WatchEvent => Unit): IOWatcher = |
There was a problem hiding this comment.
The watch method in the IOWatch object should include the default value for the options parameter to match IOWatchBase. In Scala, default arguments are not automatically inherited by overriding methods in a way that makes them available when calling the method on the overriding object directly. Adding it here allows users to call IOWatch.watch(path) { ... } without explicitly providing WatchOptions.default.
override def watch(path: IOPath, options: WatchOptions = WatchOptions.default)(handler: WatchEvent => Unit): IOWatcher =| else | ||
| WatchEventType.Deleted | ||
|
|
||
| handler(WatchEvent(eventType, ioPath)) |
There was a problem hiding this comment.
The handler callback is executed within the watcher's background thread. If the user-provided handler throws an unhandled exception, it will cause the watcher thread to terminate silently, stopping all further event processing. It is safer to wrap the handler call in a try-catch block to ensure the watcher remains resilient to errors in user code.
try
handler(WatchEvent(eventType, ioPath))
catch
case e: Throwable =>
// Prevent user code exceptions from killing the watcher thread
()- JVM: Avoid double-registering root directory in recursive mode by using walkFileTree for all directories including root - JVM: Register entire subtree (not just top-level dir) when a new directory is created during recursive watching - Node.js: Detect platform and throw UnsupportedOperationException for recursive watching on Linux where fs.watch doesn't support it Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a cross-platform file system watcher API, IOWatch, with implementations for JVM, Node.js, and Scala Native. The JVM implementation uses java.nio.file.WatchService, the Node.js version utilizes fs.watch, and the Native version employs a polling-based approach. Feedback highlights the need to address platform-specific limitations of recursive watching in Node.js and to ensure the watcher remains persistent to prevent premature process exit. Additionally, for the JVM implementation, it is recommended to recursively register subdirectories when a new directory tree is created to maintain consistent behavior across all platforms.
| override def watch(path: IOPath, options: WatchOptions)(handler: WatchEvent => Unit): IOWatcher = | ||
| if !isNodeEnv then | ||
| throw UnsupportedOperationException("IOWatch is not supported in browser environments") | ||
|
|
There was a problem hiding this comment.
There are two concerns with the watchOptions configuration:
- Recursive support: Node.js
fs.watchonly supports therecursiveoption on macOS and Windows. On Linux, this implementation will silently fail to watch subdirectories recursively. For a cross-platform library, it is recommended to either implement a manual recursive watcher for Linux or clearly document this platform-specific limitation. - Persistence: Setting
persistent = falsemeans the Node.js event loop will not stay active if this watcher is the only remaining task. Typically, file watchers are expected to keep the process alive (e.g., in build tools or dev servers) until explicitly closed. Settingpersistent = true(the default in Node.js) is generally more appropriate for this API.
val watchOptions = js.Dynamic.literal(recursive = options.recursive, persistent = true)| // If recursive and a new directory was created, register its entire subtree | ||
| if options.recursive && kind == ENTRY_CREATE && Files.isDirectory(fullPath) then |
There was a problem hiding this comment.
When a new directory is created in recursive mode, only the top-level directory is registered for watching. If a directory tree is moved into the watched path or created with mkdir -p, subdirectories of the new directory will not be monitored. Using Files.walkFileTree ensures that all subdirectories within the newly created path are also registered.
if options.recursive && kind == ENTRY_CREATE && Files.isDirectory(fullPath) then
Files.walkFileTree(
fullPath,
new SimpleFileVisitor[Path]:
override def preVisitDirectory(dir: Path, attrs: BasicFileAttributes): FileVisitResult =
registerDirectory(dir)
FileVisitResult.CONTINUE
)- Remove duplicate Scaladoc block in IOWatch.scala - JS: Reuse FileSystem.isNode instead of duplicating isNodeEnv check - JS: Fix TOCTOU in directory validation (single statSync with catch) - JS: Remove unused JSImport import - JVM: Add running check in SimpleFileVisitor to allow prompt shutdown - JVM: Add visitFileFailed handler to tolerate deleted dirs during walk - JVM: Catch IOException around recursive registerTree in event loop - Native: Add @volatile to snapshot var for cross-thread visibility - Remove unnecessary WHAT comments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add default parameter value to IOWatch.watch override so callers can omit the options argument - Wrap handler callbacks in try-catch on JVM and Native to prevent user exceptions from killing the watcher thread - Change Node.js fs.watch persistent option to true (safer default for keeping the event loop alive) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All three platform implementations now protect against user handler exceptions propagating into the watcher's execution context. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
IOWatchAPI towvlet.uni.iofor watching directories for file creation, modification, and deletion eventsjava.nio.file.WatchServicewith daemon thread pollingfs.watchAPI with recursive supportUnsupportedOperationException(not supported)IOWatch.watchthrough theIOfacade objectAPI
Test plan
scalafmtAllpasses🤖 Generated with Claude Code