Skip to content

Commit 3ef5fc8

Browse files
robhoganfacebook-github-bot
authored andcommitted
Handle missing asserted-states in Watchman subscribe response
Summary: By #1022, Watchman may send a `subscribe` response with null or undefined `asserted-states`. `asserted-states` were added in facebook/watchman@b114886 without an associated "capability". *Some versions of Watchman, e.g. 4.9.4, pass all of our required capability checks but don't include this field.* This makes `#deferringStates` nullable, and only used on `state-enter` / `state-leave` if `asserted-states` is present in the subscription response. (This prevents underreporting states in the case that the subscription is initially deferred.) Changelog: ``` - **[Fix]**: asserted-states is not iterable when using older Watchman versions. ``` Reviewed By: motiz88 Differential Revision: D47216959 fbshipit-source-id: 4f6d75958902737080afe2feea163ed3d976da29
1 parent f5a5f58 commit 3ef5fc8

File tree

3 files changed

+180
-17
lines changed

3 files changed

+180
-17
lines changed

flow-typed/fb-watchman.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ declare module 'fb-watchman' {
2424
...WatchmanBaseResponse,
2525
subscribe: string,
2626
warning?: string,
27-
'asserted-states': $ReadOnlyArray<string>,
27+
'asserted-states'?: $ReadOnlyArray<string>,
2828
}>;
2929

3030
declare type WatchmanWatchResponse = $ReadOnly<{

packages/metro-file-map/src/watchers/WatchmanWatcher.js

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export default class WatchmanWatcher extends EventEmitter {
5353
root: string,
5454
}>;
5555
watchmanDeferStates: $ReadOnlyArray<string>;
56-
#deferringStates: Set<string> = new Set();
56+
#deferringStates: ?Set<string> = null;
5757

5858
constructor(dir: string, opts: WatcherOptions) {
5959
super();
@@ -164,8 +164,8 @@ export default class WatchmanWatcher extends EventEmitter {
164164

165165
handleWarning(resp);
166166

167-
for (const state of resp['asserted-states']) {
168-
this.#deferringStates.add(state);
167+
if (resp['asserted-states'] != null) {
168+
this.#deferringStates = new Set(resp['asserted-states']);
169169
}
170170

171171
self.emit('ready');
@@ -207,7 +207,7 @@ export default class WatchmanWatcher extends EventEmitter {
207207
stateEnter != null &&
208208
(this.watchmanDeferStates ?? []).includes(stateEnter)
209209
) {
210-
this.#deferringStates.add(stateEnter);
210+
this.#deferringStates?.add(stateEnter);
211211
debug(
212212
'Watchman reports "%s" just started. Filesystem notifications are paused.',
213213
stateEnter,
@@ -217,7 +217,7 @@ export default class WatchmanWatcher extends EventEmitter {
217217
stateLeave != null &&
218218
(this.watchmanDeferStates ?? []).includes(stateLeave)
219219
) {
220-
this.#deferringStates.delete(stateLeave);
220+
this.#deferringStates?.delete(stateLeave);
221221
debug(
222222
'Watchman reports "%s" ended. Filesystem notifications resumed.',
223223
stateLeave,
@@ -318,21 +318,21 @@ export default class WatchmanWatcher extends EventEmitter {
318318
async close() {
319319
this.client.removeAllListeners();
320320
this.client.end();
321-
this.#deferringStates.clear();
321+
this.#deferringStates = null;
322322
}
323323

324324
getPauseReason(): ?string {
325-
if (this.#deferringStates.size) {
326-
const states = [...this.#deferringStates];
327-
if (states.length === 1) {
328-
return `The watch is in the '${states[0]}' state.`;
329-
}
330-
return `The watch is in the ${states
331-
.slice(0, -1)
332-
.map(s => `'${s}'`)
333-
.join(', ')} and '${states[states.length - 1]}' states.`;
325+
if (this.#deferringStates == null || this.#deferringStates.size === 0) {
326+
return null;
327+
}
328+
const states = [...this.#deferringStates];
329+
if (states.length === 1) {
330+
return `The watch is in the '${states[0]}' state.`;
334331
}
335-
return null;
332+
return `The watch is in the ${states
333+
.slice(0, -1)
334+
.map(s => `'${s}'`)
335+
.join(', ')} and '${states[states.length - 1]}' states.`;
336336
}
337337
}
338338

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
* @oncall react_native
10+
*/
11+
12+
import type {
13+
WatchmanSubscribeResponse,
14+
WatchmanClockResponse,
15+
WatchmanWatchResponse,
16+
} from 'fb-watchman';
17+
import WatchmanWatcher from '../WatchmanWatcher';
18+
import EventEmitter from 'events';
19+
20+
class MockClient extends EventEmitter {
21+
command: JestMockFn<$ReadOnlyArray<$FlowFixMe>, mixed> = jest.fn();
22+
}
23+
const mockClient = new MockClient();
24+
25+
const cmdCallback = <T>(err: ?Error, result: Partial<T>): void => {
26+
expect(mockClient.command.mock.lastCall[1]).toEqual(expect.any(Function));
27+
mockClient.command.mock.lastCall[1](err, result);
28+
};
29+
30+
jest.mock('fb-watchman', () => ({
31+
Client: jest.fn().mockImplementation(() => mockClient),
32+
}));
33+
34+
describe('WatchmanWatcher', () => {
35+
test('initializes with watch-project, clock, subscribe', () => {
36+
const watchmanWatcher = new WatchmanWatcher('/project/subdir/js', {
37+
dot: true,
38+
ignored: false,
39+
glob: ['**/*.js'],
40+
watchmanDeferStates: ['busy'],
41+
});
42+
const readyListener = jest.fn();
43+
watchmanWatcher.on('ready', readyListener);
44+
45+
expect(mockClient.command).toHaveBeenCalledWith(
46+
['watch-project', '/project/subdir/js'],
47+
expect.any(Function),
48+
);
49+
cmdCallback<WatchmanWatchResponse>(null, {
50+
watch: '/project',
51+
relative_path: 'subdir/js',
52+
});
53+
54+
expect(mockClient.command).toHaveBeenCalledWith(
55+
['clock', '/project'],
56+
expect.any(Function),
57+
);
58+
cmdCallback<WatchmanClockResponse>(null, {
59+
clock: 'c:1629095304.251049',
60+
});
61+
62+
expect(mockClient.command).toHaveBeenCalledWith(
63+
[
64+
'subscribe',
65+
'/project',
66+
watchmanWatcher.subscriptionName,
67+
{
68+
defer: ['busy'],
69+
fields: ['name', 'exists', 'new', 'type', 'size', 'mtime_ms'],
70+
relative_root: 'subdir/js',
71+
since: 'c:1629095304.251049',
72+
},
73+
],
74+
expect.any(Function),
75+
);
76+
77+
expect(readyListener).not.toHaveBeenCalled();
78+
cmdCallback<WatchmanSubscribeResponse>(null, {});
79+
expect(readyListener).toHaveBeenCalled();
80+
});
81+
82+
describe('getPauseReason', () => {
83+
let watchmanWatcher: WatchmanWatcher;
84+
85+
beforeEach(() => {
86+
watchmanWatcher = new WatchmanWatcher('/project/subdir/js', {
87+
dot: true,
88+
ignored: false,
89+
glob: ['**/*.js'],
90+
watchmanDeferStates: ['busy'],
91+
});
92+
cmdCallback<WatchmanWatchResponse>(null, {});
93+
cmdCallback<WatchmanClockResponse>(null, {});
94+
});
95+
96+
test('subscribe response is initally deferred', () => {
97+
cmdCallback<WatchmanSubscribeResponse>(null, {
98+
'asserted-states': ['busy'],
99+
});
100+
expect(watchmanWatcher.getPauseReason()).toMatch(/busy/);
101+
mockClient.emit('subscription', {
102+
subscription: watchmanWatcher.subscriptionName,
103+
'state-leave': 'busy',
104+
});
105+
expect(watchmanWatcher.getPauseReason()).toBe(null);
106+
});
107+
108+
test('unknown states are ignored', () => {
109+
cmdCallback<WatchmanSubscribeResponse>(null, {
110+
'asserted-states': [],
111+
});
112+
mockClient.emit('subscription', {
113+
subscription: watchmanWatcher.subscriptionName,
114+
'state-enter': 'unknown-state',
115+
});
116+
expect(watchmanWatcher.getPauseReason()).toBe(null);
117+
mockClient.emit('subscription', {
118+
subscription: watchmanWatcher.subscriptionName,
119+
'state-enter': 'busy',
120+
});
121+
expect(watchmanWatcher.getPauseReason()).toMatch(/busy/);
122+
});
123+
124+
test('known states are reported and cleared', () => {
125+
cmdCallback<WatchmanSubscribeResponse>(null, {
126+
'asserted-states': [],
127+
});
128+
mockClient.emit('subscription', {
129+
subscription: watchmanWatcher.subscriptionName,
130+
'state-enter': 'busy',
131+
});
132+
expect(watchmanWatcher.getPauseReason()).toMatch(/busy/);
133+
mockClient.emit('subscription', {
134+
subscription: watchmanWatcher.subscriptionName,
135+
'state-leave': 'busy',
136+
});
137+
expect(watchmanWatcher.getPauseReason()).toBe(null);
138+
});
139+
140+
test('missing asserted-states in subscribe response', () => {
141+
cmdCallback<WatchmanSubscribeResponse>(null, {});
142+
expect(watchmanWatcher.getPauseReason()).toBe(null);
143+
mockClient.emit('subscription', {
144+
subscription: watchmanWatcher.subscriptionName,
145+
'state-enter': 'busy',
146+
});
147+
// We don't know the original states, so don't attempt to report states.
148+
expect(watchmanWatcher.getPauseReason()).toBe(null);
149+
});
150+
151+
test('empty asserted-states in subscribe response', () => {
152+
cmdCallback<WatchmanSubscribeResponse>(null, {
153+
'asserted-states': [],
154+
});
155+
expect(watchmanWatcher.getPauseReason()).toBe(null);
156+
mockClient.emit('subscription', {
157+
subscription: watchmanWatcher.subscriptionName,
158+
'state-enter': 'busy',
159+
});
160+
expect(watchmanWatcher.getPauseReason()).toMatch(/busy/);
161+
});
162+
});
163+
});

0 commit comments

Comments
 (0)