Skip to content

Report watcher backend errors without terminating Metro #2147

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

Open
sorinc03 wants to merge 3 commits into
base: main
Choose a base branch
from
+865 −30
Open

Report watcher backend errors without terminating Metro #2147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ac137a5
Report watcher backend errors without terminating Metro
sorinc03 Nov 2, 2026
e051d28
Recover watcher connections and bound backend error reporting
sorinc03 Nov 2, 2026
0faadf1
Handle remaining watcher startup and error diagnostics
sorinc03 Nov 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions packages/metro-file-map/src/Watcher.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,34 @@ export class Watcher extends EventEmitter {
clocks: rootClocks,
};
const watcher: WatcherBackend = new WatcherImpl(root, watcherOptions);
const reportedResourceLimits = new Set<string>();

return new Promise(async (resolve, reject) => {
return new Promise((resolve, reject) => {
const rejectTimeout = setTimeout(
() => reject(new Error('Failed to start watch mode.')),
MAX_WAIT_TIME,
);

watcher.onError(error => {

@github-actions github-actions Bot Nov 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should fix · A failed Watchman reconnect now leaves a permanently dead watcher with one log line

Logging is the right result for a one-off lstat failure, but for a Watchman client error it means watching has stopped for good, and nothing recovers or says so.

Failure case: Watchman goes away, so the client gets end and WatchmanWatcher.#init reconnects once. If that attempt fails (socket connect error, or watchman get-sockname can't spawn), fb-watchman emits error but not end, so no further retry happens. Before this PR the process exited and the user restarted it. Now Metro keeps running and serves stale bundles, because no file event ever arrives again. The only signal is one watchman watcher error line that doesn't say changes are no longer seen. The health check that could expose this defaults to enabled: false.

From reading fb-watchman, a socket error without end on an established connection (e.g. ECONNRESET) reaches the same state without even one reconnect attempt. I did not exercise that path.

Suggested fix: handle this in WatchmanWatcher: on a client error or failed #init, retry with backoff until a new subscription is live, since a re-subscribe can recover. If retrying is out of scope, keep Watchman connection failures fatal, or at least log clearly that file watching has stopped and Metro must be restarted. A test should cover connection loss followed by a failed reconnect.

Reproduced: repro/watchman-repro.js dead, using the real Watcher and fb-watchman against a fake Watchman socket. The server closes, the reconnect fails, then the server returns. Head logs watchman watcher error ... connect ENOENT, then prints RESULT connections after Watchman came back: 0 after 5s. Base exits with RESULT UNCAUGHT: connect ENOENT. · id 79065045

@sorinc03 sorinc03 Nov 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e051d28. Failed Watchman reconnects now retry with exponential backoff from 1 second up to 30 seconds, resetting after a successful subscription. Coverage includes transport, watch-project, clock and subscribe failures, a later successful subscription, continued file events and cancellation on stop. The existing fresh-clock semantics are unchanged; this does not claim replay of changes during an outage. Full validation passes: 165 suites, 3,076 tests, 495 snapshots (14 skipped), plus Flow/lint/format/build.

@github-actions github-actions Bot Nov 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should fix · An error on NativeWatcher's FSWatcher still terminates Metro

This listener only sees errors that go through emitError. NativeWatcher.startWatching (NativeWatcher.js:84) creates its FSWatcher without an error listener, unlike FallbackWatcher.js:216, so an error event on that object is still uncaught.

Failure case: on macOS without Watchman, Metro is running on NativeWatcher and its recursive FSWatcher emits error. The event throws instead of reaching the new reporting, and the process exits. The PR's changelog and summary cover filesystem errors from watcher backends generally, so this path is in scope. I triggered it by emitting the event on the real FSWatcher, and didn't establish which OS condition produces it.

Suggested fix: in NativeWatcher.startWatching, add this.#fsWatcher.on('error', error => this.emitError(error)), plus a test.

Reproduced: repro.js native-fswatcher-error on head and base, on macOS with the real Watcher and NativeWatcher: after watch() resolved, emitting error (code EIO) on the backend's FSWatcher threw Error: watch failed out of emit, so the survival log line never printed. · id dd9e1d91

@sorinc03 sorinc03 Nov 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0faadf1. NativeWatcher forwards the filesystem handle's error event through the backend error channel. Tests cover original-error identity, continued file delivery, and safe late errors after stopping.

const code = (error as interface {readonly code?: unknown}).code;
let guidance = '';
if (code === 'ENOSPC' || code === 'EMFILE') {
// A resource limit can fail once per directory in a large crawl.
if (reportedResourceLimits.has(code)) {
return;
}
reportedResourceLimits.add(code);
guidance =
' Changes in unwatched directories will not be detected. ' +
(code === 'ENOSPC'
? 'Increase fs.inotify.max_user_watches on Linux.'
: 'Increase the open file limit (ulimit -n).');
}
this.#options.console.error(

@github-actions github-actions Bot Nov 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should fix · Hitting the watch limit logs a separate error and stack for every remaining directory

FallbackWatcher.#watchdir reports each fs.watch failure and carries on, so with this listener ENOSPC or EMFILE produces one console.error, with its stack, per unwatched directory.

Failure case: on Linux, a project with more directories than fs.inotify.max_user_watches used to stop on the first error, with a clear message. It now starts normally after printing an error for every directory past the limit. That can be many thousands of stack traces, burying the Metro UI and the one useful line, and the directories past the limit go unwatched.

Suggested fix: report resource-limit errors once per backend, or as one summary after startWatching, saying that changes in unwatched directories won't be detected and how to raise the limit. Add a test that fails fs.watch for many directories through the real Watcher and FallbackWatcher.

Reproduced: repro/flood-repro.js <checkout> 3000 100, using the real Watcher and FallbackWatcher with fs.watch throwing ENOSPC after 100 watches (a simulated limit, not real inotify exhaustion). Head prints RESULT started normally; console.error calls: 2901 of 3000 dirs (limit 100). Base prints RESULT process would exit, unhandled rejection: ENOSPC.... · id 488248c3

@sorinc03 sorinc03 Nov 2, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in e051d28. ENOSPC and EMFILE are reported once per code per backend instead of once per remaining directory, with explicit guidance that unwatched directories will not deliver changes. The regression uses the real FallbackWatcher over 128 temporary directories with simulated fs.watch limit failures, verifies the crawl still visits them, and verifies a separate root gets its own diagnostic. No actual OS-wide watch limit was exhausted.

@github-actions github-actions Bot Nov 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should fix · A lost FallbackWatcher directory watch is logged without saying changes will be missed

When a directory's FSWatcher errors, FallbackWatcher.js:216-224 forgets the handle and nothing watches that directory again. On base the unhandled error ended the process. Now the error is logged and Metro keeps running, with that directory silently out of date for the rest of the session.

Failure case: on Linux or Windows, the FSWatcher for src/feature errors with a code that isn't ignorable, such as EIO on a flaky mount. Metro logs fallback watcher error in <root>: watch failed, and later edits under src/feature produce no events. The message names neither the directory nor the consequence. The new ENOSPC/EMFILE guidance does state the consequence, but other codes get none. I triggered this by closing the real handle and emitting error, as Node does when it closes a failed watcher; I didn't produce a real OS failure.

Suggested fix: in FallbackWatcher, try to re-watch the directory and emit a recrawl for it. At minimum, report that changes under that directory won't be detected until a restart, and include the path.

Reproduced: repro.js fallback-lost-watch on head (macOS, real FallbackWatcher forced on): events before the error were ["sub/before.js","sub"]. After close() and emit('error') on the handle for sub, writing sub/after.js gave events after error [], with one generic console.error. On base the same emit threw. · id bb4c86e5

@sorinc03 sorinc03 Nov 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0faadf1. A lost directory watch reports its path, the missed-change consequence, and restart guidance. The wrapper preserves the original cause and error code, retaining ENOSPC/EMFILE deduplication and existing ignorable-error behavior. Tests exercise real fallback watcher startup with controlled failing handles.

`metro-file-map: ${this.#activeWatcher ?? 'unknown'} watcher error in ${root}:${guidance}`,
error,
);
});
watcher.onFileEvent(change => {
const basename = path.basename(change.relativePath);
if (basename.startsWith(this.#options.healthCheckFilePrefix)) {
Expand Down Expand Up @@ -358,9 +379,18 @@ export class Watcher extends EventEmitter {
this.#bufferedEvents.push(change);
}
});
await watcher.startWatching();
clearTimeout(rejectTimeout);
resolve(watcher);
void Promise.resolve()
.then(() => watcher.startWatching())
.then(
() => {
clearTimeout(rejectTimeout);
resolve(watcher);
},
error => {
clearTimeout(rejectTimeout);
reject(error);
},
);
});
};

Expand Down
135 changes: 134 additions & 1 deletion packages/metro-file-map/src/__tests__/Watcher-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import type {

import TreeFS from '../lib/TreeFS';
import {Watcher} from '../Watcher';
import EventEmitter from 'node:events';
import * as fs from 'node:fs';
import * as path from 'node:path';

Expand All @@ -39,14 +40,17 @@ type MockBackend = {
root: string,
clocks: ?Map<string, string>,
emit: (event: WatcherBackendChangeEvent) => void,
emitError: (error: Error) => void,
};
const mockStartedBackends: Array<MockBackend> = [];
const mockStartWatching = jest.fn<[string], Promise<void>>();
let mockNativeLiveOnStart = true;
function mockBackendClass(kind: MockBackend['kind']) {
return class {
#root: string;
#clocks: ?Map<string, string>;
#listener: ?(event: WatcherBackendChangeEvent) => void;
#emitter: EventEmitter = new EventEmitter();
static isSupported(): boolean {
return true;
}
Expand All @@ -60,13 +64,23 @@ function mockBackendClass(kind: MockBackend['kind']) {
onFileEvent(listener: (event: WatcherBackendChangeEvent) => void) {
this.#listener = listener;
}
async startWatching() {
onError(listener: (error: Error) => void): () => void {
this.#emitter.on('error', listener);
return () => {
this.#emitter.removeListener('error', listener);
};
}
startWatching(): Promise<void> {
mockStartedBackends.push({
kind,
root: this.#root,
clocks: this.#clocks,
emit: event => this.#listener?.(event),
emitError: error => {
this.#emitter.emit('error', error);
},
});
return mockStartWatching(this.#root);
}
async stopWatching() {}
getPauseReason(): ?string {
Expand Down Expand Up @@ -144,13 +158,106 @@ beforeEach(() => {
mockNativeLiveOnStart = true;
mockWatchmanCrawl.mockReset();
mockNodeCrawl.mockReset();
mockStartWatching.mockReset();
mockStartWatching.mockImplementation(async () => {});
jest.spyOn(console, 'warn').mockImplementation(() => {});
});

afterEach(() => {
jest.clearAllTimers();
jest.restoreAllMocks();
});

describe('Watcher startup rejection', () => {
test.each([
[false, 'crawl'],
[false, 'watch'],
[true, 'watch'],
])(
'propagates a backend rejection from %s/%s before the timeout',
async (useWatchman, operation) => {
const error = new Error('Backend could not start');
mockStartWatching.mockImplementationOnce(async () => {
throw error;
});
const watcher = createWatcher(useWatchman, null, true);
const outcomes: Array<?Error> = [];
const starting = (
operation === 'crawl' ? watcher.crawl() : watcher.watch(() => {})
).then(
() => outcomes.push(null),
(error: Error) => outcomes.push(error),
);

// $FlowFixMe[prop-missing] Available in Jest 29, missing from its libdef.
await jest.advanceTimersByTimeAsync(0);

expect(outcomes).toEqual([error]);
expect(jest.getTimerCount()).toBe(0);
await starting;
await watcher.close();
},
);

test('propagates a failure from watchRoot without leaving a timer', async () => {
const watcher = createWatcher(true, null, true);
await watcher.watch(() => {});
const error = new Error('Added root could not start');
mockStartWatching.mockImplementationOnce(async () => {
throw error;
});
const outcomes: Array<?Error> = [];
const starting = watcher.watchRoot(path.join(rootDir, 'added')).then(
() => outcomes.push(null),
(error: Error) => outcomes.push(error),
);

// $FlowFixMe[prop-missing] Available in Jest 29, missing from its libdef.
await jest.advanceTimersByTimeAsync(0);

expect(outcomes).toEqual([error]);
expect(jest.getTimerCount()).toBe(0);
await starting;
await watcher.close();
});

test('propagates a synchronous startup error without leaving a timer', async () => {
const error = new Error('Synchronous startup failure');
mockStartWatching.mockImplementationOnce(() => {
throw error;
});
const watcher = createWatcher(true, null, true);

await expect(watcher.watch(() => {})).rejects.toBe(error);
expect(jest.getTimerCount()).toBe(0);
await watcher.close();
});

test('retains the existing deadline for a startup that never settles', async () => {
mockStartWatching.mockImplementationOnce(() => new Promise(() => {}));
const watcher = createWatcher(true, null, true);
const outcomes: Array<?Error> = [];
const starting = watcher
.watch(() => {})
.then(
() => outcomes.push(null),
(error: Error) => outcomes.push(error),
);

// $FlowFixMe[prop-missing] Available in Jest 29, missing from its libdef.
await jest.advanceTimersByTimeAsync(239999);
expect(outcomes).toEqual([]);
// $FlowFixMe[prop-missing] Available in Jest 29, missing from its libdef.
await jest.advanceTimersByTimeAsync(1);

expect(outcomes).toHaveLength(1);
expect(outcomes[0]?.message).toBe('Failed to start watch mode.');
expect(jest.getTimerCount()).toBe(0);
await starting;
await watcher.close();
});
});

describe('Watcher crawl fallback', () => {
test('the node crawler runs directly when Watchman is not preferred', async () => {
const canonicalPath = path.join('src', 'index.js');
Expand Down Expand Up @@ -234,6 +341,32 @@ describe('Watcher backend choice in watch mode', () => {
metadata: {modifiedTime: 1, size: 1, type: 'f'},
};

test.each([false, true])(
'reports backend errors without losing later changes (useWatchman=%s)',
async useWatchman => {
mockNodeCrawl.mockImplementation(async () => crawlResult([]));
mockWatchmanCrawl.mockImplementation(async () => crawlResult([]));
const reportError = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
const watcher = createWatcher(useWatchman, null, true);
const onChange = jest.fn<[WatcherBackendChangeEvent], void>();
await watcher.crawl();
await watcher.watch(onChange);

const backend = mockStartedBackends[0];
const error = new Error('Cannot read watched directory');
expect(() => backend.emitError(error)).not.toThrow();
expect(reportError).toHaveBeenCalledWith(
`metro-file-map: ${backend.kind} watcher error in ${rootDir}:`,
error,
);
backend.emit(event);
expect(onChange).toHaveBeenCalledWith(event);
await watcher.close();
},
);

test('a successful Watchman crawl starts a Watchman backend only on watch()', async () => {
mockWatchmanCrawl.mockImplementation(async () => crawlResult([]));
const watcher = createWatcher(true, null, true);
Expand Down
Loading
Loading