Stop a deleted watched path from raising an uncaught EPERM (Windows)

DESIGN.md

@@ -1242,6 +1242,17 @@ New watch sites must go through it. As of this writing the sites are `components
 `server/threads/manageThreads.js`, and `resources/blob.ts`. `fs.watchFile` (`utility/logging/readLog.ts`)
 is stat polling with no fs-event handle and is outside this invariant.
 
+Five of those six sites arm the watch through `guardedWatch()` (`utility/watcherFallback.ts`) rather
+than calling `chokidar.watch`/`fs.watch` directly — it installs a process-level guard for a second,
+unrelated failure (a watched path deleted out from under a non-persistent chokidar watcher raises an
+unhandled async `EPERM`; see that file's header comment) but does no canonicalization of its own.
+Every caller still resolves its own path first and passes the resolved path in, exactly as when they
+called `chokidar.watch` directly, so the invariant holds through the wrapper. `utility/watcherFallback.ts`
+itself is the one file that touches `chokidar` without canonicalizing — the watch-sites source scan
+(`unitTests/utility/watchPath.test.js`) lists it as a native watch site (it does arm one) but exempts
+it from the per-file canonicalization check, since canonicalizing is its callers' job, not its own —
+the same relationship raw `chokidar.watch` has to the other five sites.
+
 Two consequences worth knowing before adding a caller. `EntryHandler` is the one place where the
 canonical path is load-bearing past the `fs.watch` call: chokidar's `ignored` predicate receives
 absolute paths built from `cwd`, so its bases must be derived from the same spelling, while event

components/EntryHandler.ts

@@ -4,14 +4,16 @@ import { createHash } from 'node:crypto';
 import type { Stats } from 'node:fs';
 import { EventEmitter } from 'node:events';
 import { Component, FileAndURLPathConfig } from './Component.ts';
-import chokidar, { FSWatcher, FSWatcherEventMap } from 'chokidar';
+import { FSWatcher, FSWatcherEventMap } from 'chokidar';
 import { isAbsolute, join } from 'node:path';
 import { readFile } from 'node:fs/promises';
 import { FilesOption } from './deriveGlobOptions.ts';
 import { deriveURLPath } from './deriveURLPath.ts';
 import { isMatch } from 'micromatch';
 import {
 	DIRECTORY_POLLING_FALLBACK_OPTIONS,
+	claimLostNativeWatchError,
+	guardedWatch,
 	isWatcherExhaustionError,
 	warnWatcherFallback,
 } from '../utility/watcherFallback.ts';
@@ -392,6 +394,11 @@ export class EntryHandler extends EventEmitter<EntryHandlerEventMap> {
 	}
 
 	#handleWatcherError(error: unknown): void {
+		// A lost native watch handle (the watched tree was deleted or replaced) is
+		// benign and not actionable by a consumer. chokidar's non-persistent branch
+		// never routes it here today — installLostNativeWatchGuard() catches it at
+		// the process instead — but the polling branch and a fixed chokidar would.
+		if (claimLostNativeWatchError(error)) return;
 		if (isWatcherExhaustionError(error)) {
 			// Swallow every exhaustion error — chokidar can emit several before the
 			// failed native watcher closes, and we don't want a flurry of ENOSPC to
@@ -555,40 +562,39 @@ export class EntryHandler extends EventEmitter<EntryHandlerEventMap> {
 		}
 
 		this.#openCount++;
-		const watcher = (this.#watcher = chokidar
-			.watch(watchPattern, {
-				cwd: watchDirectory,
-				persistent: false,
-				followSymlinks: false,
-				...(this.#usingPolling ? DIRECTORY_POLLING_FALLBACK_OPTIONS : {}),
-				ignored: (path) => {
-					const normalizedPath = path.replace(/\\/g, '/');
-
-					// Determine the path relative to the component directory. Leading '/' is preserved
-					// (or empty when the path *is* the component directory) so the regex anchors below
-					// can use `(?:^|/)` to match the first segment without false positives on names
-					// that merely contain the same substring (e.g. `mynode_modules`, `notgit`).
-					const relativePath = normalizedPath.startsWith(normalizedDirectory)
-						? normalizedPath.slice(normalizedDirectory.length)
-						: normalizedPath;
-
-					// Skip node_modules at any depth. This allows plugins loaded from node_modules
-					// to still watch their own component files while ignoring their dependencies.
-					if (/(?:^|\/)node_modules(?:\/|$)/.test(relativePath)) return true;
-
-					// Skip transient package manager and VCS artifacts. Without these, an in-place
-					// `npm install` during a component deploy writes log files and atomic-rename
-					// temp directories that fire change events and drive an auto-reload restart
-					// storm — see harper#488.
-					if (/(?:^|\/)\.git(?:\/|$)/.test(relativePath)) return true;
-					if (/(?:^|\/)\.tmp-/.test(relativePath)) return true;
-					if (/(?:^|\/)(?:npm-debug|yarn-error|yarn-debug|pnpm-debug)\.log(?:\/|$)/.test(relativePath)) return true;
-
-					return (
-						normalizedPath !== normalizedDirectory && normalizedBases.every((base) => !normalizedPath.startsWith(base))
-					);
-				},
-			})
+		const watcher = (this.#watcher = guardedWatch(watchPattern, {
+			cwd: watchDirectory,
+			persistent: false,
+			followSymlinks: false,
+			...(this.#usingPolling ? DIRECTORY_POLLING_FALLBACK_OPTIONS : {}),
+			ignored: (path) => {
+				const normalizedPath = path.replace(/\\/g, '/');
+
+				// Determine the path relative to the component directory. Leading '/' is preserved
+				// (or empty when the path *is* the component directory) so the regex anchors below
+				// can use `(?:^|/)` to match the first segment without false positives on names
+				// that merely contain the same substring (e.g. `mynode_modules`, `notgit`).
+				const relativePath = normalizedPath.startsWith(normalizedDirectory)
+					? normalizedPath.slice(normalizedDirectory.length)
+					: normalizedPath;
+
+				// Skip node_modules at any depth. This allows plugins loaded from node_modules
+				// to still watch their own component files while ignoring their dependencies.
+				if (/(?:^|\/)node_modules(?:\/|$)/.test(relativePath)) return true;
+
+				// Skip transient package manager and VCS artifacts. Without these, an in-place
+				// `npm install` during a component deploy writes log files and atomic-rename
+				// temp directories that fire change events and drive an auto-reload restart
+				// storm — see harper#488.
+				if (/(?:^|\/)\.git(?:\/|$)/.test(relativePath)) return true;
+				if (/(?:^|\/)\.tmp-/.test(relativePath)) return true;
+				if (/(?:^|\/)(?:npm-debug|yarn-error|yarn-debug|pnpm-debug)\.log(?:\/|$)/.test(relativePath)) return true;
+
+				return (
+					normalizedPath !== normalizedDirectory && normalizedBases.every((base) => !normalizedPath.startsWith(base))
+				);
+			},
+		})
 			.on('all', (...args) => this.#handleAll(generation, ...args))
 			.on('error', (error) => {
 				if (generation === this.#watchGeneration) this.#handleWatcherError(error);

components/OptionsWatcher.ts

@@ -2,7 +2,7 @@ import { type Logger } from '../utility/logging/logger.ts';
 import { loggerWithTag } from '../utility/logging/harper_logger.ts';
 import { EventEmitter, once } from 'events';
 import yaml from 'yaml';
-import chokidar, { type FSWatcher } from 'chokidar';
+import { type FSWatcher } from 'chokidar';
 import { readFile } from 'node:fs/promises';
 import { readFileSync } from 'node:fs';
 import { isDeepStrictEqual } from 'util';
@@ -11,6 +11,8 @@ import { cloneDeep } from 'lodash';
 import {
 	POLLING_FALLBACK_OPTIONS,
 	PartialReadRetry,
+	claimLostNativeWatchError,
+	guardedWatch,
 	isPartialReadError,
 	isWatcherExhaustionError,
 	warnWatcherFallback,
@@ -126,11 +128,10 @@ export class OptionsWatcher extends EventEmitter<OptionsWatcherEventMap> {
 
 	#openWatcher() {
 		this.#openCount++;
-		this.#watcher = chokidar
-			.watch(this.#watchPath, {
-				persistent: false,
-				...(this.#usingPolling ? POLLING_FALLBACK_OPTIONS : {}),
-			})
+		this.#watcher = guardedWatch(this.#watchPath, {
+			persistent: false,
+			...(this.#usingPolling ? POLLING_FALLBACK_OPTIONS : {}),
+		})
 			.on('add', this.#handleChange.bind(this))
 			.on('change', this.#handleChange.bind(this))
 			.on('error', this.#handleError.bind(this))
@@ -256,6 +257,9 @@ export class OptionsWatcher extends EventEmitter<OptionsWatcherEventMap> {
 	}
 
 	#handleError(error: unknown) {
+		// See EntryHandler.#handleWatcherError: a lost native watch handle is benign
+		// and must not be surfaced to consumers as a config-watch failure.
+		if (claimLostNativeWatchError(error)) return;
 		if (isWatcherExhaustionError(error)) {
 			// Swallow every exhaustion error — chokidar can emit several before the
 			// failed native watcher closes, and we don't want a flurry of ENOSPC to

config/RootConfigWatcher.ts

@@ -1,11 +1,13 @@
-import chokidar, { FSWatcher } from 'chokidar';
+import { FSWatcher } from 'chokidar';
 import { readFileSync } from 'node:fs';
 import { getConfigFilePath } from './configUtils.ts';
 import { EventEmitter, once } from 'node:events';
 import { parse } from 'yaml';
 import {
 	POLLING_FALLBACK_OPTIONS,
 	PartialReadRetry,
+	claimLostNativeWatchError,
+	guardedWatch,
 	isPartialReadError,
 	isWatcherExhaustionError,
 	warnWatcherFallback,
@@ -38,11 +40,10 @@ export class RootConfigWatcher extends EventEmitter {
 
 	#openWatcher() {
 		this.#openCount++;
-		this.#watcher = chokidar
-			.watch(this.#watchPath, {
-				persistent: false,
-				...(this.#usingPolling ? POLLING_FALLBACK_OPTIONS : {}),
-			})
+		this.#watcher = guardedWatch(this.#watchPath, {
+			persistent: false,
+			...(this.#usingPolling ? POLLING_FALLBACK_OPTIONS : {}),
+		})
 			.on('add', this.handleChange.bind(this))
 			.on('change', this.handleChange.bind(this))
 			.on('error', this.handleError.bind(this));
@@ -66,6 +67,9 @@ export class RootConfigWatcher extends EventEmitter {
 	}
 
 	handleError(error: unknown) {
+		// See EntryHandler.#handleWatcherError: a lost native watch handle is benign
+		// and must not be surfaced to consumers as a config-watch failure.
+		if (claimLostNativeWatchError(error)) return;
 		if (isWatcherExhaustionError(error)) {
 			// Swallow every exhaustion error — chokidar can emit several before the
 			// failed native watcher closes, and we don't want a flurry of ENOSPC to

security/keys.ts

@@ -1,7 +1,6 @@
 'use strict';
 
 import * as path from 'path';
-import { watch } from 'chokidar';
 import * as fs from 'fs-extra';
 import * as forge from 'node-forge';
 import * as net from 'net';
@@ -33,7 +32,13 @@ export const getPrivateKeys = () => privateKeys;
 import { readFileSync, statSync } from 'node:fs';
 import { getTicketKeys, onMessageFromWorkers } from '../server/threads/manageThreads.js';
 import { isMainThread } from 'worker_threads';
-import { POLLING_FALLBACK_OPTIONS, isWatcherExhaustionError, warnWatcherFallback } from '../utility/watcherFallback.ts';
+import {
+	POLLING_FALLBACK_OPTIONS,
+	claimLostNativeWatchError,
+	guardedWatch,
+	isWatcherExhaustionError,
+	warnWatcherFallback,
+} from '../utility/watcherFallback.ts';
 import { resolveWatchTarget } from '../utility/watchPath.ts';
 import { TLSSocket } from 'node:tls';
 
@@ -361,7 +366,7 @@ function loadAndWatch(path, loadCert, type) {
 	let usingPolling = watchTarget.mustPoll;
 	let liveWatcher;
 	const openWatcher = () => {
-		const opened = (liveWatcher = watch(watchTarget.path, {
+		const opened = (liveWatcher = guardedWatch(watchTarget.path, {
 			persistent: false,
 			...(usingPolling ? POLLING_FALLBACK_OPTIONS : {}),
 		}));
@@ -371,6 +376,7 @@ function loadAndWatch(path, loadCert, type) {
 			.on('change', () => loadFile(path))
 			// chokidar emits 'error' unguarded for anything but ENOENT/ENOTDIR.
 			.on('error', (error) => {
+				if (claimLostNativeWatchError(error)) return;
 				if (isWatcherExhaustionError(error)) {
 					if (usingPolling || liveWatcher !== opened) return;
 					warnWatcherFallback(path);

server/threads/manageThreads.js

@@ -26,6 +26,8 @@ const { getConfigPath } = require('../../config/configUtils.ts');
 const { resolveWatchTarget } = require('../../utility/watchPath.ts');
 const {
 	DIRECTORY_POLLING_FALLBACK_OPTIONS,
+	claimLostNativeWatchError,
+	guardedWatch,
 	isWatcherExhaustionError,
 	warnWatcherFallback,
 } = require('../../utility/watcherFallback.ts');
@@ -49,7 +51,6 @@ function getRequireModules() {
 		);
 	return requireModules;
 }
-const chokidar = require('chokidar');
 const isBun = typeof globalThis.Bun !== 'undefined';
 const MB = 1024 * 1024;
 const workers = []; // these are our child workers that we are managing
@@ -1506,7 +1507,7 @@ if (isMainThread) {
 		let usingPolling = watchTarget.mustPoll;
 		let liveWatcher;
 		const openWatcher = () => {
-			const opened = (liveWatcher = chokidar.watch(watchTarget.path, {
+			const opened = (liveWatcher = guardedWatch(watchTarget.path, {
 				persistent: false,
 				...(usingPolling ? DIRECTORY_POLLING_FALLBACK_OPTIONS : {}),
 				ignored: (path) => {
@@ -1517,6 +1518,7 @@ if (isMainThread) {
 				// This runs on the thread that owns every worker, and chokidar emits 'error' unguarded for
 				// anything but ENOENT/ENOTDIR.
 				.on('error', (error) => {
+					if (claimLostNativeWatchError(error)) return;
 					if (isWatcherExhaustionError(error)) {
 						if (usingPolling || liveWatcher !== opened) return;
 						warnWatcherFallback(dir);

server/threads/socketRouter.ts

@@ -18,6 +18,9 @@ let sweptSocketsDirectory = false;
 if (isMainThread) {
 	process.on('uncaughtException', (error) => {
 		// TODO: Maybe we should try to log the first of each type of error
+		// Same `isHandled` contract as threadServer.js: an error another handler has already
+		// classified (a lost native file watch, for instance) must not be logged again here.
+		if ((error as any).isHandled) return;
 		if ((error as any).code === 'ECONNRESET') return; // that's what network connections do
 		if ((error as any).code === 'EIO') {
 			// that means the terminal is closed

unitTests/security/keys.test.js

@@ -1077,7 +1077,7 @@ describe('Test keys module', () => {
 		const watchPollers = keys.__get__('certificateWatchPollers');
 		const localSandbox = sinon.createSandbox();
 		const chokidar = require('chokidar');
-		const realChokidarWatch = chokidar.watch;
+		const realChokidarWatch = chokidar.default.watch;
 		let watchPath;
 
 		// Never open a real watcher here: these tests exercise only the poll/reopen paths, and real
@@ -1094,13 +1094,13 @@ describe('Test keys module', () => {
 		};
 
 		beforeEach(() => {
-			chokidar.watch = () => fakeWatcher();
+			chokidar.default.watch = () => fakeWatcher();
 			watchPath = path.join(test_dir, `watch-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}.pem`);
 			fs.writeFileSync(watchPath, 'PEM-V1');
 		});
 
 		afterEach(() => {
-			chokidar.watch = realChokidarWatch;
+			chokidar.default.watch = realChokidarWatch;
 			localSandbox.restore();
 			// warnWatcherFallback's first-fallback gate is process-global; leaving it set would make a
 			// later suite's warning assertion silently observe nothing.
@@ -1134,7 +1134,7 @@ describe('Test keys module', () => {
 			// chokidar v4 defaults alwaysStat:false, so the 'change' handler is called with undefined
 			// stats; loadFile must stat the file itself rather than throw and silently skip the reload.
 			let changeHandler;
-			chokidar.watch = () =>
+			chokidar.default.watch = () =>
 				fakeWatcher((event, handler) => {
 					if (event === 'change') changeHandler = handler;
 				});
@@ -1159,7 +1159,7 @@ describe('Test keys module', () => {
 			const openedOptions = [];
 			const errorHandlers = [];
 			const exhausted = () => Object.assign(new Error('inotify watch limit reached'), { code: 'ENOSPC' });
-			chokidar.watch = (_watchedPath, options) => {
+			chokidar.default.watch = (_watchedPath, options) => {
 				openedOptions.push(options);
 				return fakeWatcher((event, handler) => {
 					if (event === 'error') errorHandlers.push(handler);
@@ -1189,7 +1189,7 @@ describe('Test keys module', () => {
 			// of reopening on polling.
 			const openedOptions = [];
 			const errorHandlers = [];
-			chokidar.watch = (_watchedPath, options) => {
+			chokidar.default.watch = (_watchedPath, options) => {
 				openedOptions.push(options);
 				const watcher = {
 					on: (event, handler) => {

unitTests/server/threads/watchDirFallback.test.js

@@ -7,10 +7,10 @@ const { watchDir } = require('#src/server/threads/manageThreads');
 const { _resetForTests: resetWatcherFallbackWarning } = require('#src/utility/watcherFallback');
 
 describe('watchDir watcher fallback', () => {
-	const realWatch = chokidar.watch;
+	const realWatch = chokidar.default.watch;
 
 	afterEach(() => {
-		chokidar.watch = realWatch;
+		chokidar.default.watch = realWatch;
 		// warnWatcherFallback's first-fallback gate is process-global; leaving it set would make a
 		// later suite's warning assertion silently observe nothing.
 		resetWatcherFallbackWarning();
@@ -23,7 +23,7 @@ describe('watchDir watcher fallback', () => {
 	const stubChokidar = (close) => {
 		const openedOptions = [];
 		const errorHandlers = [];
-		chokidar.watch = (_watchedPath, options) => {
+		chokidar.default.watch = (_watchedPath, options) => {
 			openedOptions.push(options);
 			const watcher = {
 				on: (event, handler) => {