From da77d8a60801a40bcc320afefa643ebf98c0a210 Mon Sep 17 00:00:00 2001 From: Juergen Kunz Date: Thu, 11 Dec 2025 19:13:35 +0000 Subject: [PATCH] fix(watcher.node): Normalize paths and improve Node watcher robustness: restart/rescan on errors (including ENOSPC), clear stale state, and remove legacy throttler --- changelog.md | 9 ++++ readme.hints.md | 19 ++++++-- ts/00_commitinfo_data.ts | 2 +- ts/watchers/watcher.node.ts | 97 ++++++++++++------------------------- 4 files changed, 54 insertions(+), 73 deletions(-) diff --git a/changelog.md b/changelog.md index e750318..08437b5 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,14 @@ # Changelog +## 2025-12-11 - 6.2.5 - fix(watcher.node) +Normalize paths and improve Node watcher robustness: restart/rescan on errors (including ENOSPC), clear stale state, and remove legacy throttler + +- Normalize all paths to absolute at watcher entry points (watchPath, handleFsEvent, scanDirectory) to avoid relative/absolute mismatch bugs +- On watcher restart: clear pending unlink timeouts, dispose stale DirEntry data, and perform a rescan to catch files created during the restart window +- Trigger watcher restart on ENOSPC (inotify limit) errors instead of only logging the error +- Remove the previous Throttler implementation and rely on the existing debounce + event-sequence tracking to handle rapid events +- Atomic write handling and queued unlink behavior preserved; pending unlinks are cleared for restarted base paths to avoid stale events + ## 2025-12-11 - 6.2.4 - fix(tests) Stabilize tests and document chokidar-inspired Node watcher architecture diff --git a/readme.hints.md b/readme.hints.md index 79e9bb1..7268ac3 100644 --- a/readme.hints.md +++ b/readme.hints.md @@ -80,11 +80,20 @@ The Node.js watcher has been refactored with elegant patterns inspired by [choki - Encapsulates file tracking and inode management - `dispose()` method freezes object to catch use-after-cleanup bugs -**Throttler Pattern:** -- More sophisticated than simple debounce -- Tracks count of suppressed events -- Returns `false` if already throttled, `Throttler` object otherwise -- Used for change events to prevent duplicate emissions +**Path Normalization (v6.3.1+):** +- ALL paths are normalized to absolute at entry points +- Prevents relative/absolute path mismatch bugs +- `watchPath()`, `handleFsEvent()`, `scanDirectory()` all resolve paths + +**Restart Rescan (v6.3.1+):** +- When watcher restarts, it rescans the directory +- Catches files created during the restart window +- Clears stale DirEntry data before rescan +- Clears pending unlink timeouts to prevent stale events + +**ENOSPC Handling (v6.3.1+):** +- inotify limit errors now trigger watcher restart +- Previously only logged error without recovery **Atomic Write Handling:** - Unlink events are queued with 100ms delay diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index e7bab4f..c2cc14e 100644 --- a/ts/00_commitinfo_data.ts +++ b/ts/00_commitinfo_data.ts @@ -3,6 +3,6 @@ */ export const commitinfo = { name: '@push.rocks/smartwatch', - version: '6.2.4', + version: '6.2.5', description: 'A cross-runtime file watcher with glob pattern support for Node.js, Deno, and Bun.' } diff --git a/ts/watchers/watcher.node.ts b/ts/watchers/watcher.node.ts index be68427..7dd220a 100644 --- a/ts/watchers/watcher.node.ts +++ b/ts/watchers/watcher.node.ts @@ -18,16 +18,6 @@ const EV = { ERROR: 'error', } as const; -/** Throttle action types */ -type ThrottleType = 'change' | 'unlink' | 'add'; - -/** Throttler state for an action */ -interface Throttler { - timeout: NodeJS.Timeout; - count: number; - clear: () => number; -} - /** Configuration constants */ const CONFIG = { MAX_RETRIES: 3, @@ -127,9 +117,6 @@ export class NodeWatcher implements IWatcher { // Event stream public readonly events$ = new smartrx.rxjs.Subject(); - // Throttling - inspired by chokidar's _throttle pattern - private throttled: Map> = new Map(); - // Atomic write handling - pending unlinks that may become changes private pendingUnlinks: Map = new Map(); @@ -156,12 +143,7 @@ export class NodeWatcher implements IWatcher { // Closer registry - inspired by chokidar for clean resource management private closers: Map void>> = new Map(); - constructor(private options: IWatcherOptions) { - // Initialize throttle maps - this.throttled.set('change', new Map()); - this.throttled.set('unlink', new Map()); - this.throttled.set('add', new Map()); - } + constructor(private options: IWatcherOptions) {} get isWatching(): boolean { return this._isWatching; @@ -228,14 +210,6 @@ export class NodeWatcher implements IWatcher { } this.pendingUnlinks.clear(); - // Clear throttles - for (const actionMap of this.throttled.values()) { - for (const throttler of actionMap.values()) { - clearTimeout(throttler.timeout); - } - actionMap.clear(); - } - // Now set flag this._isWatching = false; @@ -285,41 +259,6 @@ export class NodeWatcher implements IWatcher { } } - // =========================================================================== - // Throttling - Inspired by chokidar's elegant _throttle pattern - // =========================================================================== - - /** - * Throttle an action for a path. Returns false if already throttled. - * Unlike simple debounce, this tracks how many events were suppressed. - */ - private throttle(actionType: ThrottleType, filePath: string, timeout: number): Throttler | false { - const actionMap = this.throttled.get(actionType); - if (!actionMap) return false; - - const existing = actionMap.get(filePath); - if (existing) { - existing.count++; - return false; - } - - const clear = (): number => { - const item = actionMap.get(filePath); - const count = item?.count ?? 0; - actionMap.delete(filePath); - return count; - }; - - const throttler: Throttler = { - timeout: setTimeout(clear, timeout), - count: 0, - clear, - }; - - actionMap.set(filePath, throttler); - return throttler; - } - // =========================================================================== // Closer Registry - Clean resource management // =========================================================================== @@ -424,6 +363,9 @@ export class NodeWatcher implements IWatcher { // =========================================================================== private async watchPath(watchPath: string): Promise { + // Normalize path to absolute - critical for consistent lookups + watchPath = path.resolve(watchPath); + try { const stats = await this.statSafe(watchPath); if (!stats?.isDirectory()) return; @@ -494,7 +436,8 @@ export class NodeWatcher implements IWatcher { return; } - const fullPath = path.join(basePath, filename); + // Normalize to absolute path - critical for consistent lookups + const fullPath = path.resolve(path.join(basePath, filename)); // Handle temp files from atomic writes if (this.isTemporaryFile(fullPath)) { @@ -602,10 +545,7 @@ export class NodeWatcher implements IWatcher { this.safeEmit({ type: EV.CHANGE, path: fullPath, stats }); } } else { - // Apply throttle for change events - if (!this.throttle('change', fullPath, 50)) { - return; // Throttled - } + // Debounce already handles rapid events - no extra throttle needed this.safeEmit({ type: EV.CHANGE, path: fullPath, stats }); } } @@ -667,6 +607,9 @@ export class NodeWatcher implements IWatcher { // =========================================================================== private async scanDirectory(dirPath: string, depth: number): Promise { + // Normalize path to absolute - critical for consistent lookups + dirPath = path.resolve(dirPath); + if (depth > this.options.depth) return; try { @@ -724,6 +667,8 @@ export class NodeWatcher implements IWatcher { console.error('[smartwatch] ENOSPC: inotify watch limit exceeded!'); console.error('[smartwatch] Fix: echo fs.inotify.max_user_watches=524288 | sudo tee -a /etc/sysctl.conf && sudo sysctl -p'); this.safeEmit({ type: EV.ERROR, path: basePath, error }); + // Trigger restart - watcher may be broken after ENOSPC + this.restartWatcher(basePath, error); } else { console.error(`[smartwatch] Health check error for ${basePath}:`, error); } @@ -772,6 +717,22 @@ export class NodeWatcher implements IWatcher { this.watchers.delete(basePath); } + // Clear pending unlinks for this base path (prevent stale events) + for (const [unlinkedPath, pending] of this.pendingUnlinks) { + if (unlinkedPath.startsWith(basePath)) { + clearTimeout(pending.timeout); + this.pendingUnlinks.delete(unlinkedPath); + } + } + + // Clear stale DirEntry data (will be repopulated by rescan) + for (const [dirPath, entry] of this.watched) { + if (dirPath === basePath || dirPath.startsWith(basePath + path.sep)) { + entry.dispose(); + this.watched.delete(dirPath); + } + } + // Exponential backoff with abort support const delay = this.restartDelays.get(basePath) || CONFIG.INITIAL_RESTART_DELAY; console.log(`[smartwatch] Waiting ${delay}ms before restart...`); @@ -803,6 +764,8 @@ export class NodeWatcher implements IWatcher { try { await this.watchPath(basePath); + // Rescan to catch files created during restart window + await this.scanDirectory(basePath, 0); console.log(`[smartwatch] Successfully restarted watcher for ${basePath}`); this.restartDelays.set(basePath, CONFIG.INITIAL_RESTART_DELAY); this.restartAttempts.set(basePath, 0);