Skip to content

Commit 3330779

Browse files
committed
Improve server start implementation and provide better error reporting
- Report error for spawn() failures - Register callback for server exit, error and stderr to catch such errors - If error happens report the error with detailed message from stderr - Do healthCheck in 1 second intervals until it is successful or the overall timeout of 10s happens - Remove unnecessary warning messages when stopping server after startup failures. Signed-off-by: Bernd Hufmann <bernd.hufmann@ericsson.com>
1 parent a1da771 commit 3330779

File tree

1 file changed

+57
-24
lines changed

1 file changed

+57
-24
lines changed

src/trace-server.ts

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const key = 'pid';
1313
const millis = 10000;
1414
const none = -1;
1515
const prefix = 'Trace Server';
16-
const suffix = ' failure or so.';
16+
const suffix = ' failure.';
1717
const SHUTDOWN_DELAY = 2000;
1818

1919
export class TraceServer {
@@ -24,15 +24,21 @@ export class TraceServer {
2424
const server = spawn(this.getPath(from), this.getArgs(from));
2525

2626
if (!server.pid) {
27-
this.showError(prefix + ' startup' + suffix);
27+
// Failed to spawn the child process due to errors, PID is undefined and an error is emitted.
28+
await new Promise<void>(resolve => {
29+
server.once('error', error => {
30+
this.showError(prefix + ' startup' + suffix + ': ' + error);
31+
});
32+
resolve();
33+
});
2834
return;
2935
}
3036
this.server = server;
3137
await context?.workspaceState.update(key, this.server.pid);
3238
await this.waitFor(context);
3339
}
3440

35-
async stopOrReset(context: vscode.ExtensionContext | undefined) {
41+
async stopOrReset(context: vscode.ExtensionContext | undefined, reportNotStopped: boolean = true) {
3642
const pid: number | undefined = context?.workspaceState.get(key);
3743
const not = prefix + ' not stopped as none running or owned by us.';
3844
if (pid === none) {
@@ -66,7 +72,7 @@ export class TraceServer {
6672
});
6773
}
6874
);
69-
} else {
75+
} else if (reportNotStopped) {
7076
vscode.window.showWarningMessage(not);
7177
}
7278
await context?.workspaceState.update(key, none);
@@ -168,27 +174,57 @@ export class TraceServer {
168174
},
169175
async progress => {
170176
progress.report({ message: 'starting up...' });
171-
let timeout = false;
172-
const timeoutId = setTimeout(() => (timeout = true), millis);
173-
174-
// eslint-disable-next-line no-constant-condition
175-
while (true) {
176-
if (await this.isUp()) {
177-
this.showStatus(true);
178-
clearTimeout(timeoutId);
179-
break;
180-
}
181-
if (timeout) {
182-
this.showError(prefix + ' startup timed-out after ' + millis + 'ms.');
183-
await this.stopOrReset(context);
184-
break;
177+
try {
178+
let timeout = false;
179+
const timeoutId = setTimeout(() => (timeout = true), millis);
180+
181+
let stderr = '';
182+
this.server?.stderr?.on('data', data => {
183+
stderr = data;
184+
});
185+
186+
let errorString: string | undefined;
187+
this.server?.once('exit', code => {
188+
errorString = 'Code: ' + code + (stderr !== '.' ? '. Error message: ' + stderr : '');
189+
});
190+
191+
this.server?.once('error', error => {
192+
errorString = 'Error occurred. Error message: ' + error;
193+
});
194+
195+
// eslint-disable-next-line no-constant-condition
196+
while (true) {
197+
if (await this.isUp()) {
198+
clearTimeout(timeoutId);
199+
this.showStatus(true);
200+
break;
201+
}
202+
203+
// Check if trace server exited with error
204+
if (errorString !== undefined) {
205+
throw new Error(errorString);
206+
}
207+
208+
// Check for overall timeout
209+
if (timeout) {
210+
throw new Error(prefix + ' startup timed-out after ' + millis + 'ms.');
211+
}
212+
213+
// Wait before trying again
214+
await new Promise<void>(resolve => setTimeout(() => resolve(), 1000));
185215
}
216+
} catch (error) {
217+
this.showErrorDetailed(prefix + ' starting up' + suffix, error as Error);
218+
await this.stopOrReset(context, false);
219+
return;
220+
} finally {
221+
this.server?.removeAllListeners();
186222
}
187223
}
188224
);
189225
}
190226

191-
private async isUp() {
227+
private async isUp(): Promise<boolean> {
192228
const client = new TspClient(this.getServerUrl());
193229
const health = await client.checkHealth();
194230
const status = health.getModel()?.status;
@@ -198,18 +234,15 @@ export class TraceServer {
198234
private async showError(message: string) {
199235
console.error(message);
200236
vscode.window.showErrorMessage(message);
201-
const disclaimer = ' running, despite this error.';
202237
const up = await this.isUp();
203238
if (up) {
204-
vscode.window.showWarningMessage(prefix + ' is still' + disclaimer);
205-
} else {
206-
vscode.window.showWarningMessage(prefix + ' is not' + disclaimer);
239+
vscode.window.showWarningMessage(prefix + ' is still running, despite this error.');
207240
}
208241
this.setStatusIfAvailable(up);
209242
}
210243

211244
private showErrorDetailed(message: string, error: Error) {
212-
const details = error.name + ' - ' + error.message;
245+
const details = error.name + ' - ' + error?.message;
213246
vscode.window.showErrorMessage(details);
214247
console.error(details);
215248
this.showError(message);

0 commit comments

Comments
 (0)