Skip to content

Commit

Permalink
Report current deferring states in Watchman health check message
Browse files Browse the repository at this point in the history
Summary: When watcher health checks are enabled (D40352039 (7adf468)), enhances the message displayed to the user when a health check fails by listing any Watchman states that may be causing events to be [deferred](https://facebook.github.io/metro/docs/configuration/#watchmandeferstates).

Reviewed By: robhogan

Differential Revision: D40367578

fbshipit-source-id: c90309f9bb64f32179d49725c57066dcb742ab02
  • Loading branch information
motiz88 authored and facebook-github-bot committed Oct 17, 2022
1 parent d49602b commit 39f6e50
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 12 deletions.
1 change: 1 addition & 0 deletions flow-typed/fb-watchman.js
Expand Up @@ -19,6 +19,7 @@ declare module 'fb-watchman' {
declare type WatchmanSubscribeResponse = $ReadOnly<{
subscribe: string,
warning?: string,
'asserted-states': $ReadOnlyArray<string>,
...
}>;

Expand Down
6 changes: 4 additions & 2 deletions packages/metro-file-map/src/Watcher.js
Expand Up @@ -55,6 +55,7 @@ type WatcherOptions = {
};

interface WatcherBackend {
getPauseReason(): ?string;
close(): Promise<void>;
}

Expand All @@ -63,7 +64,7 @@ let nextInstanceId = 0;
export type HealthCheckResult =
| {type: 'error', timeout: number, error: Error, watcher: ?string}
| {type: 'success', timeout: number, timeElapsed: number, watcher: ?string}
| {type: 'timeout', timeout: number, watcher: ?string};
| {type: 'timeout', timeout: number, watcher: ?string, pauseReason: ?string};

export class Watcher {
_options: WatcherOptions;
Expand Down Expand Up @@ -249,13 +250,14 @@ export class Watcher {
'-' +
healthCheckId;
const healthCheckPath = path.join(this._options.rootDir, basename);
let result;
let result: ?HealthCheckResult;
const timeoutPromise = new Promise(resolve =>
setTimeout(resolve, timeout),
).then(() => {
if (!result) {
result = {
type: 'timeout',
pauseReason: this._backends[0]?.getPauseReason(),
timeout,
watcher,
};
Expand Down
4 changes: 4 additions & 0 deletions packages/metro-file-map/src/watchers/FSEventsWatcher.js
Expand Up @@ -204,4 +204,8 @@ export default class FSEventsWatcher extends EventEmitter {
this.emit(type, file, this.root, stat);
this.emit(ALL_EVENT, type, file, this.root, stat);
}

getPauseReason(): ?string {
return null;
}
}
4 changes: 4 additions & 0 deletions packages/metro-file-map/src/watchers/NodeWatcher.js
Expand Up @@ -330,6 +330,10 @@ module.exports = class NodeWatcher extends EventEmitter {
this.emit(type, file, this.root, stat);
this.emit(ALL_EVENT, type, file, this.root, stat);
}

getPauseReason(): ?string {
return null;
}
};
/**
* Determine if a given FS error can be ignored
Expand Down
39 changes: 31 additions & 8 deletions packages/metro-file-map/src/watchers/WatchmanWatcher.js
Expand Up @@ -55,6 +55,7 @@ export default class WatchmanWatcher extends EventEmitter {
root: string,
}>;
watchmanDeferStates: $ReadOnlyArray<string>;
#deferringStates: Set<string> = new Set();

constructor(dir: string, opts: WatcherOptions) {
super();
Expand Down Expand Up @@ -157,16 +158,20 @@ export default class WatchmanWatcher extends EventEmitter {
);
}

function onSubscribe(error: ?Error, resp: WatchmanSubscribeResponse) {
const onSubscribe = (error: ?Error, resp: WatchmanSubscribeResponse) => {
if (handleError(self, error)) {
return;
}
debug('Received subscribe response: %s', resp.subscribe);

handleWarning(resp);

for (const state of resp['asserted-states']) {
this.#deferringStates.add(state);
}

self.emit('ready');
}
};

self.client.command(['watch-project', getWatchRoot()], onWatchProject);
}
Expand Down Expand Up @@ -199,22 +204,25 @@ export default class WatchmanWatcher extends EventEmitter {
if (Array.isArray(resp.files)) {
resp.files.forEach(change => this._handleFileChange(change));
}
const {'state-enter': stateEnter, 'state-leave': stateLeave} = resp;
if (
resp['state-enter'] != null &&
(this.watchmanDeferStates ?? []).includes(resp['state-enter'])
stateEnter != null &&
(this.watchmanDeferStates ?? []).includes(stateEnter)
) {
this.#deferringStates.add(stateEnter);
debug(
'Watchman reports "%s" just started. Filesystem notifications are paused.',
resp['state-enter'],
stateEnter,
);
}
if (
resp['state-leave'] != null &&
(this.watchmanDeferStates ?? []).includes(resp['state-leave'])
stateLeave != null &&
(this.watchmanDeferStates ?? []).includes(stateLeave)
) {
this.#deferringStates.delete(stateLeave);
debug(
'Watchman reports "%s" ended. Filesystem notifications resumed.',
resp['state-leave'],
stateLeave,
);
}
}
Expand Down Expand Up @@ -295,6 +303,21 @@ export default class WatchmanWatcher extends EventEmitter {
async close() {
this.client.removeAllListeners();
this.client.end();
this.#deferringStates.clear();
}

getPauseReason(): ?string {
if (this.#deferringStates.size) {
const states = [...this.#deferringStates];
if (states.length === 1) {
return `The watch is in the '${states[0]}' state.`;
}
return `The watch is in the ${states
.slice(0, -1)
.map(s => `'${s}'`)
.join(', ')} and '${states[states.length - 1]}' states.`;
}
return null;
}
}

Expand Down
13 changes: 11 additions & 2 deletions packages/metro/src/lib/TerminalReporter.js
Expand Up @@ -437,7 +437,11 @@ class TerminalReporter {
// Don't be spammy; only report changes in status.
if (
!this._prevHealthCheckResult ||
result.type !== this._prevHealthCheckResult.type
result.type !== this._prevHealthCheckResult.type ||
(result.type === 'timeout' &&
this._prevHealthCheckResult.type === 'timeout' &&
(result.pauseReason ?? null) !==
(this._prevHealthCheckResult.pauseReason ?? null))
) {
const watcherName = "'" + (result.watcher ?? 'unknown') + "'";
switch (result.type) {
Expand All @@ -456,9 +460,14 @@ class TerminalReporter {
);
break;
case 'timeout':
const why =
result.pauseReason != null
? ` This may be because: ${result.pauseReason}`
: '';
reporting.logWarning(
this.terminal,
`Watcher ${watcherName} failed to detect a file change within ${result.timeout}ms.`,
`Watcher ${watcherName} failed to detect a file change within ${result.timeout}ms.` +
why,
);
break;
}
Expand Down

0 comments on commit 39f6e50

Please sign in to comment.