Skip to content

Commit 27f2019

Browse files
bpaserobpasero
authored
watcher - consistent restart behaviour on errors (microsoft#135954)
1 parent 8feef7f commit 27f2019

File tree

5 files changed

+132
-108
lines changed

5 files changed

+132
-108
lines changed

‎src/vs/platform/files/common/watcher.ts

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Event } from 'vs/base/common/event';
7-
import { Disposable } from 'vs/base/common/lifecycle';
7+
import { Disposable, DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle';
88
import { isLinux } from 'vs/base/common/platform';
99
import { URI as uri } from 'vs/base/common/uri';
1010
import { FileChangeType, IFileChange, isParent } from 'vs/platform/files/common/files';
@@ -22,6 +22,13 @@ export interface IWatcherService {
2222
*/
2323
readonly onDidLogMessage: Event<ILogMessage>;
2424

25+
/**
26+
* An event to indicate an error occured from the watcher
27+
* that is unrecoverable. Listeners should restart the
28+
* service if possible.
29+
*/
30+
readonly onDidError: Event<string>;
31+
2532
/**
2633
* Configures the watcher service to watch according
2734
* to the requests. Any existing watched path that
@@ -41,8 +48,93 @@ export interface IWatcherService {
4148
stop(): Promise<void>;
4249
}
4350

51+
export abstract class AbstractWatcherService extends Disposable {
52+
53+
private static readonly MAX_RESTARTS = 5;
54+
55+
private service: IWatcherService | undefined;
56+
private readonly serviceDisposables = this._register(new MutableDisposable());
57+
58+
private requests: IWatchRequest[] | undefined = undefined;
59+
60+
private restartCounter = 0;
61+
62+
constructor(
63+
private readonly onFileChanges: (changes: IDiskFileChange[]) => void,
64+
private readonly onLogMessage: (msg: ILogMessage) => void,
65+
private verboseLogging: boolean
66+
) {
67+
super();
68+
}
69+
70+
protected abstract createService(disposables: DisposableStore): IWatcherService;
71+
72+
protected init(): void {
73+
74+
// Associate disposables to the service
75+
const disposables = new DisposableStore();
76+
this.serviceDisposables.value = disposables;
77+
78+
// Ask implementors to create the service
79+
this.service = this.createService(disposables);
80+
this.service.setVerboseLogging(this.verboseLogging);
81+
82+
// Wire in event handlers
83+
disposables.add(this.service.onDidChangeFile(e => this.onFileChanges(e)));
84+
disposables.add(this.service.onDidLogMessage(e => this.onLogMessage(e)));
85+
disposables.add(this.service.onDidError(e => this.onError(e)));
86+
}
87+
88+
protected onError(error: string): void {
89+
90+
// Restart up to N times
91+
if (this.restartCounter < AbstractWatcherService.MAX_RESTARTS && this.requests) {
92+
this.error(`restarting watcher after error: ${error}`);
93+
this.restart(this.requests);
94+
}
95+
96+
// Otherwise log that we have given up to restart
97+
else {
98+
this.error(`gave up attempting to restart watcher after error: ${error}`);
99+
}
100+
}
101+
102+
private restart(requests: IWatchRequest[]): void {
103+
this.restartCounter++;
104+
105+
this.init();
106+
this.watch(requests);
107+
}
108+
109+
async watch(requests: IWatchRequest[]): Promise<void> {
110+
this.requests = requests;
111+
112+
await this.service?.watch(requests);
113+
}
114+
115+
async setVerboseLogging(verboseLogging: boolean): Promise<void> {
116+
this.verboseLogging = verboseLogging;
117+
118+
await this.service?.setVerboseLogging(verboseLogging);
119+
}
120+
121+
private error(message: string) {
122+
this.onLogMessage({ type: 'error', message: `[File Watcher (parcel)] ${message}` });
123+
}
124+
125+
override dispose(): void {
126+
127+
// Render the serve invalid from here
128+
this.service = undefined;
129+
130+
return super.dispose();
131+
}
132+
}
133+
44134
/**
45135
* Base class of any watcher service we support.
136+
*
137+
* TODO@bpasero delete and replace with `AbstractWatcherService`
46138
*/
47139
export abstract class WatcherService extends Disposable {
48140

‎src/vs/platform/files/node/watcher/parcel/parcelWatcherService.ts

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,6 @@ export interface IWatcher extends IDisposable {
5656

5757
export class ParcelWatcherService extends Disposable implements IWatcherService {
5858

59-
private static readonly MAX_RESTARTS = 5; // number of restarts we allow before giving up in case of unexpected errors
60-
6159
private static readonly MAP_PARCEL_WATCHER_ACTION_TO_FILE_CHANGE = new Map<parcelWatcher.EventType, number>(
6260
[
6361
['create', FileChangeType.ADDED],
@@ -86,6 +84,9 @@ export class ParcelWatcherService extends Disposable implements IWatcherService
8684
private readonly _onDidLogMessage = this._register(new Emitter<ILogMessage>());
8785
readonly onDidLogMessage = this._onDidLogMessage.event;
8886

87+
private readonly _onDidError = this._register(new Emitter<string>());
88+
readonly onDidError = this._onDidError.event;
89+
8990
protected readonly watchers = new Map<string, IWatcher>();
9091

9192
private verboseLogging = false;
@@ -350,8 +351,12 @@ export class ParcelWatcherService extends Disposable implements IWatcherService
350351
return; // return early when disposed
351352
}
352353

354+
// In any case of an error, treat this like a unhandled exception
355+
// that might require the watcher to restart. We do not really know
356+
// the state of parcel at this point and as such will try to restart
357+
// up to our maximum of restarts.
353358
if (error) {
354-
this.error(`Unexpected error in event callback: ${toErrorMessage(error)}`, watcher);
359+
this.onUnexpectedError(error, watcher);
355360
}
356361

357362
// Handle & emit events
@@ -538,17 +543,9 @@ export class ParcelWatcherService extends Disposable implements IWatcherService
538543
// restart the watcher as a result to get into healthy
539544
// state again if possible and if not attempted too much
540545
else {
541-
if (watcher && watcher.restarts < ParcelWatcherService.MAX_RESTARTS) {
542-
if (existsSync(watcher.request.path)) {
543-
this.warn(`Watcher will be restarted due to unexpected error: ${error}`, watcher);
546+
this.error(`Unexpected error: ${msg} (EUNKNOWN)`, watcher);
544547

545-
this.restartWatching(watcher);
546-
} else {
547-
this.error(`Unexpected error: ${msg} (EUNKNOWN: path ${watcher.request.path} no longer exists)`, watcher);
548-
}
549-
} else {
550-
this.error(`Unexpected error: ${msg} (EUNKNOWN)`, watcher);
551-
}
548+
this._onDidError.fire(msg);
552549
}
553550
}
554551

‎src/vs/platform/files/node/watcher/parcel/watcherService.ts

Lines changed: 11 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,25 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle';
6+
import { DisposableStore } from 'vs/base/common/lifecycle';
77
import { FileAccess } from 'vs/base/common/network';
88
import { getNextTickChannel, ProxyChannel } from 'vs/base/parts/ipc/common/ipc';
99
import { Client } from 'vs/base/parts/ipc/node/ipc.cp';
10-
import { IDiskFileChange, ILogMessage, IWatcherService, IWatchRequest, WatcherService } from 'vs/platform/files/common/watcher';
10+
import { AbstractWatcherService, IDiskFileChange, ILogMessage, IWatcherService } from 'vs/platform/files/common/watcher';
1111

12-
export class FileWatcher extends WatcherService {
13-
14-
private static readonly MAX_RESTARTS = 5;
15-
16-
private service: IWatcherService | undefined;
17-
private serviceDisposables = this._register(new MutableDisposable());
18-
19-
private requests: IWatchRequest[] | undefined = undefined;
20-
21-
private restartCounter = 0;
12+
export class FileWatcher extends AbstractWatcherService {
2213

2314
constructor(
24-
private readonly onDidFilesChange: (changes: IDiskFileChange[]) => void,
25-
private readonly onLogMessage: (msg: ILogMessage) => void,
26-
private verboseLogging: boolean
15+
onFileChanges: (changes: IDiskFileChange[]) => void,
16+
onLogMessage: (msg: ILogMessage) => void,
17+
verboseLogging: boolean
2718
) {
28-
super();
19+
super(onFileChanges, onLogMessage, verboseLogging);
2920

3021
this.init();
3122
}
3223

33-
private init(): void {
34-
35-
// Associate disposables to the service
36-
const disposables = new DisposableStore();
37-
this.serviceDisposables.value = disposables;
24+
protected override createService(disposables: DisposableStore): IWatcherService {
3825

3926
// Fork the parcel file watcher and build a client around
4027
// its server for passing over requests and receiving events.
@@ -51,57 +38,9 @@ export class FileWatcher extends WatcherService {
5138
}
5239
));
5340

54-
disposables.add(client.onDidProcessExit(() => {
55-
// Our watcher app should never be completed because it keeps
56-
// on watching. being in here indicates that the watcher process
57-
// died and we want to restart it here. we only do it a max number
58-
// of times
59-
if (this.restartCounter <= FileWatcher.MAX_RESTARTS && this.requests) {
60-
this.error('terminated unexpectedly and is restarted again...');
61-
this.restart(this.requests);
62-
} else {
63-
this.error('failed to start after retrying for some time, giving up. Please report this as a bug report!');
64-
}
65-
}));
66-
67-
// Initialize watcher
68-
this.service = ProxyChannel.toService<IWatcherService>(getNextTickChannel(client.getChannel('watcher')));
69-
this.service.setVerboseLogging(this.verboseLogging);
70-
71-
// Wire in event handlers
72-
disposables.add(this.service.onDidChangeFile(e => this.onDidFilesChange(e)));
73-
disposables.add(this.service.onDidLogMessage(e => this.onLogMessage(e)));
74-
}
75-
76-
private restart(requests: IWatchRequest[]): void {
77-
this.error('terminated unexpectedly and is restarted again...');
78-
this.restartCounter++;
79-
80-
this.init();
81-
this.watch(requests);
82-
}
83-
84-
async watch(requests: IWatchRequest[]): Promise<void> {
85-
this.requests = requests;
86-
87-
await this.service?.watch(requests);
88-
}
89-
90-
async setVerboseLogging(verboseLogging: boolean): Promise<void> {
91-
this.verboseLogging = verboseLogging;
92-
93-
await this.service?.setVerboseLogging(verboseLogging);
94-
}
95-
96-
private error(message: string) {
97-
this.onLogMessage({ type: 'error', message: `[File Watcher (parcel)] ${message}` });
98-
}
99-
100-
override dispose(): void {
101-
102-
// Render the serve invalid from here
103-
this.service = undefined;
41+
// React on unexpected termination of the watcher process
42+
disposables.add(client.onDidProcessExit(({ code, signal }) => this.onError(`terminated by itself with code ${code}, signal: ${signal}`)));
10443

105-
return super.dispose();
44+
return ProxyChannel.toService<IWatcherService>(getNextTickChannel(client.getChannel('watcher')));
10645
}
10746
}

‎src/vs/platform/files/test/node/recursiveWatcher.integrationTest.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ flakySuite('Recursive Watcher (parcel)', () => {
7070
}
7171
});
7272

73+
service.onDidError(e => {
74+
if (loggingEnabled) {
75+
console.log(`[recursive watcher test error] ${e}`);
76+
}
77+
});
78+
7379
testDir = getRandomTestPath(tmpdir(), 'vsctests', 'filewatcher');
7480

7581
const sourceDir = getPathFromAmdModule(require, './fixtures/service');

‎src/vs/workbench/services/files/electron-sandbox/parcelWatcherService.ts

Lines changed: 12 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,42 +3,32 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { DisposableStore } from 'vs/base/common/lifecycle';
67
import { ProxyChannel } from 'vs/base/parts/ipc/common/ipc';
7-
import { IDiskFileChange, ILogMessage, IWatcherService, IWatchRequest, WatcherService } from 'vs/platform/files/common/watcher';
8+
import { AbstractWatcherService, IDiskFileChange, ILogMessage, IWatcherService } from 'vs/platform/files/common/watcher';
89
import { ISharedProcessWorkerWorkbenchService } from 'vs/workbench/services/sharedProcess/electron-sandbox/sharedProcessWorkerWorkbenchService';
910

10-
export class ParcelFileWatcher extends WatcherService {
11-
12-
private readonly service: IWatcherService;
11+
export class ParcelFileWatcher extends AbstractWatcherService {
1312

1413
constructor(
15-
private readonly onDidFilesChange: (changes: IDiskFileChange[]) => void,
16-
private readonly onLogMessage: (msg: ILogMessage) => void,
14+
onFileChanges: (changes: IDiskFileChange[]) => void,
15+
onLogMessage: (msg: ILogMessage) => void,
1716
verboseLogging: boolean,
1817
private readonly sharedProcessWorkerWorkbenchService: ISharedProcessWorkerWorkbenchService
1918
) {
20-
super();
19+
super(onFileChanges, onLogMessage, verboseLogging);
20+
21+
this.init();
22+
}
23+
24+
protected override createService(disposables: DisposableStore): IWatcherService {
2125

2226
// Acquire parcel watcher via shared process worker
2327
const watcherChannel = this.sharedProcessWorkerWorkbenchService.createWorkerChannel({
2428
moduleId: 'vs/platform/files/node/watcher/parcel/watcherApp',
2529
type: 'watcherServiceParcelSharedProcess'
2630
}, 'watcher').channel;
2731

28-
// Initialize watcher
29-
this.service = ProxyChannel.toService<IWatcherService>(watcherChannel);
30-
this.service.setVerboseLogging(verboseLogging);
31-
32-
// Wire in event handlers
33-
this._register(this.service.onDidChangeFile(e => this.onDidFilesChange(e)));
34-
this._register(this.service.onDidLogMessage(e => this.onLogMessage(e)));
35-
}
36-
37-
setVerboseLogging(verboseLogging: boolean): Promise<void> {
38-
return this.service.setVerboseLogging(verboseLogging);
39-
}
40-
41-
watch(requests: IWatchRequest[]): Promise<void> {
42-
return this.service.watch(requests);
32+
return ProxyChannel.toService<IWatcherService>(watcherChannel);
4333
}
4434
}

0 commit comments

Comments
 (0)