From 5dda689b4cb0c5056f906ca64fbeef0e41640e8b Mon Sep 17 00:00:00 2001 From: Juergen Kunz Date: Wed, 10 Dec 2025 14:18:40 +0000 Subject: [PATCH] feat(watchers): Improve Node watcher robustness: file-level inode tracking, abortable restarts, restart race guards, and untracked-file handling --- changelog.md | 10 ++ readme.hints.md | 8 ++ ts/00_commitinfo_data.ts | 2 +- ts/watchers/watcher.node.ts | 192 ++++++++++++++++++++++++++++-------- 4 files changed, 168 insertions(+), 44 deletions(-) diff --git a/changelog.md b/changelog.md index 18f34b3..1768613 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,15 @@ # Changelog +## 2025-12-10 - 6.2.0 - feat(watchers) +Improve Node watcher robustness: file-level inode tracking, abortable restarts, restart race guards, and untracked-file handling + +- Add file-level inode tracking to detect delete+recreate (editor atomic saves) and emit correct 'change'/'add' events +- Make restart delays abortable via AbortController so stop() cancels pending restarts and prevents orphan watchers +- Guard against concurrent/dual restarts with restartingPaths to avoid race conditions between health checks and error handlers +- Emit 'unlink' for deletions of previously untracked files (files created after initial scan) and clean up inode state +- Track file inodes during initial scan and update/cleanup inode state on events +- Improve logging for restart/inode/delete+recreate scenarios and update documentation/readme hints to v6.2.0+ + ## 2025-12-08 - 6.1.1 - fix(watchers/watcher.node) Improve Node watcher robustness: inode tracking, ENOSPC detection, enhanced health checks and temp-file handling diff --git a/readme.hints.md b/readme.hints.md index 1fbe913..2de2fbf 100644 --- a/readme.hints.md +++ b/readme.hints.md @@ -79,11 +79,13 @@ The Node.js watcher includes automatic recovery mechanisms based on learnings fr - Watchers automatically restart when errors occur - Exponential backoff (1s → 30s max) - Maximum 3 retry attempts before giving up +- **v6.2.0+**: Race condition guards prevent orphan watchers when `stop()` is called during restart **Inode tracking (critical for long-running watchers):** - `fs.watch()` watches the **inode**, not the path! - When directories are replaced (git checkout, atomic saves), the inode changes - Health check detects inode changes and restarts the watcher +- **v6.2.0+**: File-level inode tracking detects delete+recreate (common editor save pattern) - This is the most common cause of "watcher stops working after some time" **Health check monitoring:** @@ -91,6 +93,7 @@ The Node.js watcher includes automatic recovery mechanisms based on learnings fr - Detects when watched paths disappear - Detects inode changes (directory replacement) - Detects ENOSPC errors (inotify limit exceeded) +- **v6.2.0+**: Protected against dual-restart race conditions (health check + error handler) **ENOSPC detection (Linux inotify limit):** - Detects when `/proc/sys/fs/inotify/max_user_watches` is exceeded @@ -100,6 +103,10 @@ The Node.js watcher includes automatic recovery mechanisms based on learnings fr - Subscriber errors don't crash the watcher - All events emitted via `safeEmit()` with try-catch +**Untracked file handling (v6.2.0+):** +- Files created after initial scan are properly detected +- Untracked file deletions emit `unlink` events instead of being silently dropped + **Verbose logging:** - All lifecycle events logged with `[smartwatch]` prefix - Helps debug watcher issues in production @@ -113,6 +120,7 @@ Example log output: [smartwatch] Health check: 1 watchers active [smartwatch] Inode changed for ./src: 12345 -> 67890 [smartwatch] fs.watch watches inode, not path - restarting watcher +[smartwatch] File inode changed (delete+recreate): ./src/file.ts ``` ### Known fs.watch Limitations diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index 8f0a003..efda168 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.1.1', + version: '6.2.0', 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 3dba60f..8bf5e48 100644 --- a/ts/watchers/watcher.node.ts +++ b/ts/watchers/watcher.node.ts @@ -23,6 +23,17 @@ export class NodeWatcher implements IWatcher { // fs.watch watches the inode, not the path. If inode changes, we need to restart. private watchedInodes: Map = new Map(); + // File inode tracking - detect when individual files are deleted and recreated + // This is critical: editors delete+recreate files, fs.watch watches OLD inode! + // See: https://github.com/paulmillr/chokidar/issues/972 + private fileInodes: Map = new Map(); + + // Abort controllers for pending restart delays - prevents orphan watchers on stop() + private restartAbortControllers: Map = new Map(); + + // Prevent concurrent restarts for the same path (health check + error can race) + private restartingPaths: Set = new Set(); + // Configuration constants private static readonly MAX_RETRIES = 3; private static readonly INITIAL_RESTART_DELAY = 1000; @@ -47,49 +58,90 @@ export class NodeWatcher implements IWatcher { /** * Restart a watcher after an error with exponential backoff + * Includes guards against: + * - Dual restart race condition (health check + error handler calling simultaneously) + * - Orphan watchers when stop() is called during restart delay */ private async restartWatcher(basePath: string, error: Error): Promise { - const attempts = (this.restartAttempts.get(basePath) || 0) + 1; - this.restartAttempts.set(basePath, attempts); - - console.log(`[smartwatch] Watcher error for ${basePath}: ${error.message}`); - console.log(`[smartwatch] Restart attempt ${attempts}/${NodeWatcher.MAX_RETRIES}`); - - if (attempts > NodeWatcher.MAX_RETRIES) { - console.error(`[smartwatch] Max retries exceeded for ${basePath}, giving up`); - this.safeEmit({ - type: 'error', - path: basePath, - error: new Error(`Max restart retries (${NodeWatcher.MAX_RETRIES}) exceeded`) - }); + // Guard: Prevent concurrent restarts for the same path + if (this.restartingPaths.has(basePath)) { + console.log(`[smartwatch] Restart already in progress for ${basePath}, skipping`); return; } - - // Close failed watcher - const oldWatcher = this.watchers.get(basePath); - if (oldWatcher) { - try { - oldWatcher.close(); - } catch { - // Ignore close errors - } - this.watchers.delete(basePath); - } - - // Exponential backoff - const delay = this.restartDelays.get(basePath) || NodeWatcher.INITIAL_RESTART_DELAY; - console.log(`[smartwatch] Waiting ${delay}ms before restart...`); - await new Promise((resolve) => setTimeout(resolve, delay)); - this.restartDelays.set(basePath, Math.min(delay * 2, NodeWatcher.MAX_RESTART_DELAY)); + this.restartingPaths.add(basePath); try { - await this.watchPath(basePath, 0); - console.log(`[smartwatch] Successfully restarted watcher for ${basePath}`); - this.restartDelays.set(basePath, NodeWatcher.INITIAL_RESTART_DELAY); - this.restartAttempts.set(basePath, 0); - } catch (restartError) { - console.error(`[smartwatch] Restart failed for ${basePath}:`, restartError); - this.restartWatcher(basePath, restartError as Error); // Recursive retry + const attempts = (this.restartAttempts.get(basePath) || 0) + 1; + this.restartAttempts.set(basePath, attempts); + + console.log(`[smartwatch] Watcher error for ${basePath}: ${error.message}`); + console.log(`[smartwatch] Restart attempt ${attempts}/${NodeWatcher.MAX_RETRIES}`); + + if (attempts > NodeWatcher.MAX_RETRIES) { + console.error(`[smartwatch] Max retries exceeded for ${basePath}, giving up`); + this.safeEmit({ + type: 'error', + path: basePath, + error: new Error(`Max restart retries (${NodeWatcher.MAX_RETRIES}) exceeded`) + }); + return; + } + + // Close failed watcher + const oldWatcher = this.watchers.get(basePath); + if (oldWatcher) { + try { + oldWatcher.close(); + } catch { + // Ignore close errors + } + this.watchers.delete(basePath); + } + + // Exponential backoff with AbortController (so stop() can cancel) + const delay = this.restartDelays.get(basePath) || NodeWatcher.INITIAL_RESTART_DELAY; + console.log(`[smartwatch] Waiting ${delay}ms before restart...`); + + const abortController = new AbortController(); + this.restartAbortControllers.set(basePath, abortController); + + try { + await new Promise((resolve, reject) => { + const timeout = setTimeout(resolve, delay); + abortController.signal.addEventListener('abort', () => { + clearTimeout(timeout); + reject(new Error('Restart aborted by stop()')); + }); + }); + } catch (abortError) { + console.log(`[smartwatch] Restart aborted for ${basePath}`); + return; // stop() was called, don't continue + } finally { + this.restartAbortControllers.delete(basePath); + } + + // Double-check we're still watching after the delay + if (!this._isWatching) { + console.log(`[smartwatch] Watcher stopped during restart delay, aborting`); + return; + } + + this.restartDelays.set(basePath, Math.min(delay * 2, NodeWatcher.MAX_RESTART_DELAY)); + + try { + await this.watchPath(basePath, 0); + console.log(`[smartwatch] Successfully restarted watcher for ${basePath}`); + this.restartDelays.set(basePath, NodeWatcher.INITIAL_RESTART_DELAY); + this.restartAttempts.set(basePath, 0); + } catch (restartError) { + console.error(`[smartwatch] Restart failed for ${basePath}:`, restartError); + // Clear restartingPaths before recursive call + this.restartingPaths.delete(basePath); + this.restartWatcher(basePath, restartError as Error); // Recursive retry + return; // Don't delete from restartingPaths again in finally + } + } finally { + this.restartingPaths.delete(basePath); } } @@ -209,6 +261,13 @@ export class NodeWatcher implements IWatcher { // Stop health check monitoring this.stopHealthCheck(); + // Abort all pending restart delays (prevents orphan watchers) + for (const [path, controller] of this.restartAbortControllers) { + console.log(`[smartwatch] Aborting pending restart for: ${path}`); + controller.abort(); + } + this.restartAbortControllers.clear(); + // Cancel all pending debounced emits for (const timeout of this.pendingEmits.values()) { clearTimeout(timeout); @@ -223,10 +282,12 @@ export class NodeWatcher implements IWatcher { this.watchers.clear(); this.watchedFiles.clear(); - // Clear restart tracking state + // Clear all tracking state this.restartDelays.clear(); this.restartAttempts.clear(); this.watchedInodes.clear(); + this.fileInodes.clear(); + this.restartingPaths.clear(); console.log('[smartwatch] Watcher stopped'); } @@ -331,6 +392,12 @@ export class NodeWatcher implements IWatcher { /** * Emit the actual file event after debounce + * + * Handles file inode tracking to detect delete+recreate scenarios: + * - fs.watch watches the inode, not the path + * - When editors delete+recreate files, the inode changes + * - Without inode tracking, events for the new file would be missed + * - See: https://github.com/paulmillr/chokidar/issues/972 */ private async emitFileEvent( fullPath: string, @@ -350,38 +417,75 @@ export class NodeWatcher implements IWatcher { } } else { const wasWatched = this.watchedFiles.has(fullPath); + const currentInode = BigInt(stats.ino); + const previousInode = this.fileInodes.get(fullPath); + + // Track file inode for delete+recreate detection + this.fileInodes.set(fullPath, currentInode); this.watchedFiles.add(fullPath); - this.safeEmit({ - type: wasWatched ? 'change' : 'add', - path: fullPath, - stats - }); + + // Check if file was recreated with different inode (delete+recreate scenario) + if (wasWatched && previousInode !== undefined && previousInode !== currentInode) { + console.log(`[smartwatch] File inode changed (delete+recreate): ${fullPath}`); + console.log(`[smartwatch] Previous inode: ${previousInode}, current: ${currentInode}`); + // Emit as 'change' since the file content likely changed + this.safeEmit({ type: 'change', path: fullPath, stats }); + } else { + this.safeEmit({ + type: wasWatched ? 'change' : 'add', + path: fullPath, + stats + }); + } } } else { // File doesn't exist - it was deleted if (this.watchedFiles.has(fullPath)) { const wasDir = this.isKnownDirectory(fullPath); this.watchedFiles.delete(fullPath); + this.fileInodes.delete(fullPath); // Clean up inode tracking this.safeEmit({ type: wasDir ? 'unlinkDir' : 'unlink', path: fullPath }); + } else { + // Fix #4: File wasn't tracked but was deleted - still emit event + // This handles files created after initial scan that we may have missed + console.log(`[smartwatch] Untracked file deleted: ${fullPath}`); + this.fileInodes.delete(fullPath); + this.safeEmit({ type: 'unlink', path: fullPath }); } } } else if (eventType === 'change') { // File was modified if (stats && !stats.isDirectory()) { const wasWatched = this.watchedFiles.has(fullPath); + const currentInode = BigInt(stats.ino); + const previousInode = this.fileInodes.get(fullPath); + + // Track file inode + this.fileInodes.set(fullPath, currentInode); + if (!wasWatched) { // This is actually an 'add' - file wasn't being watched before this.watchedFiles.add(fullPath); this.safeEmit({ type: 'add', path: fullPath, stats }); + } else if (previousInode !== undefined && previousInode !== currentInode) { + // Inode changed during 'change' event - file was replaced + console.log(`[smartwatch] File replaced (inode change on modify): ${fullPath}`); + this.safeEmit({ type: 'change', path: fullPath, stats }); } else { this.safeEmit({ type: 'change', path: fullPath, stats }); } } else if (!stats && this.watchedFiles.has(fullPath)) { // File was deleted this.watchedFiles.delete(fullPath); + this.fileInodes.delete(fullPath); + this.safeEmit({ type: 'unlink', path: fullPath }); + } else if (!stats && !this.watchedFiles.has(fullPath)) { + // Fix #4: Untracked file deleted during 'change' event + console.log(`[smartwatch] Untracked file deleted (change event): ${fullPath}`); + this.fileInodes.delete(fullPath); this.safeEmit({ type: 'unlink', path: fullPath }); } } @@ -421,6 +525,8 @@ export class NodeWatcher implements IWatcher { await this.scanDirectory(fullPath, depth + 1); } else if (entry.isFile()) { this.watchedFiles.add(fullPath); + // Track file inode for delete+recreate detection + this.fileInodes.set(fullPath, BigInt(stats.ino)); this.safeEmit({ type: 'add', path: fullPath, stats }); } }