Skip to content

Commit 3e6372d

Browse files
Alexander Eyers-Tayloraeisenberg
andauthored
Better error handling for unexpected query server termination. (#2404)
* Log stdout when servers are terminated with errors. This logs the last stdout chunk (probabaly the last line) if things went wrong. This can sometimes be useful for debugging. It also prints the signal when killed by a signal (rather than printing null) * Restart/Abort the queryserver if the process dies. This cancels any running tasks and gives a limited number of restarts. * Update extensions/ql-vscode/src/codeql-cli/cli.ts Co-authored-by: Andrew Eisenberg <aeisenberg@github.com> * Update extensions/ql-vscode/src/query-server/query-server-client.ts Co-authored-by: Andrew Eisenberg <aeisenberg@github.com> --------- Co-authored-by: Andrew Eisenberg <aeisenberg@github.com>
1 parent d0ca885 commit 3e6372d

File tree

2 files changed

+71
-5
lines changed

2 files changed

+71
-5
lines changed

extensions/ql-vscode/src/codeql-cli/cli.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,10 +1525,23 @@ export function spawnServer(
15251525
);
15261526
}
15271527

1528+
let lastStdout: any = undefined;
1529+
child.stdout!.on("data", (data) => {
1530+
lastStdout = data;
1531+
});
15281532
// Set up event listeners.
1529-
child.on("close", (code) =>
1530-
logger.log(`Child process exited with code ${code}`),
1531-
);
1533+
child.on("close", async (code, signal) => {
1534+
if (code !== null)
1535+
void logger.log(`Child process exited with code ${code}`);
1536+
if (signal)
1537+
void logger.log(
1538+
`Child process exited due to receipt of signal ${signal}`,
1539+
);
1540+
// If the process exited abnormally, log the last stdout message,
1541+
// It may be from the jvm.
1542+
if (code !== 0 && lastStdout !== undefined)
1543+
void logger.log(`Last stdout was "${lastStdout.toString()}"`);
1544+
});
15321545
child.stderr!.on("data", stderrListener);
15331546
if (stdoutListener !== undefined) {
15341547
child.stdout!.on("data", stdoutListener);

extensions/ql-vscode/src/query-server/query-server-client.ts

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,14 @@ import {
1111
ProgressMessage,
1212
WithProgressId,
1313
} from "../pure/new-messages";
14-
import { ProgressCallback, ProgressTask } from "../common/vscode/progress";
14+
import {
15+
ProgressCallback,
16+
ProgressTask,
17+
withProgress,
18+
} from "../common/vscode/progress";
1519
import { ServerProcess } from "./server-process";
1620
import { App } from "../common/app";
21+
import { showAndLogErrorMessage } from "../helpers";
1722

1823
type ServerOpts = {
1924
logger: Logger;
@@ -27,6 +32,8 @@ type WithProgressReporting = (
2732
) => Thenable<void>,
2833
) => Thenable<void>;
2934

35+
const MAX_UNEXPECTED_TERMINATIONS = 5;
36+
3037
/**
3138
* Client that manages a query server process.
3239
* The server process is started upon initialization and tracked during its lifetime.
@@ -40,6 +47,9 @@ export class QueryServerClient extends DisposableObject {
4047
};
4148
nextCallback: number;
4249
nextProgress: number;
50+
51+
unexpectedTerminationCount = 0;
52+
4353
withProgressReporting: WithProgressReporting;
4454

4555
private readonly queryServerStartListeners = [] as Array<ProgressTask<void>>;
@@ -91,10 +101,50 @@ export class QueryServerClient extends DisposableObject {
91101
}
92102
}
93103

94-
/** Restarts the query server by disposing of the current server process and then starting a new one. */
104+
/**
105+
* Restarts the query server by disposing of the current server process and then starting a new one.
106+
* This resets the unexpected termination count. As hopefully it is an indication that the user has fixed the
107+
* issue.
108+
*/
95109
async restartQueryServer(
96110
progress: ProgressCallback,
97111
token: CancellationToken,
112+
): Promise<void> {
113+
// Reset the unexpected termination count when we restart the query server manually
114+
// or due to config change
115+
this.unexpectedTerminationCount = 0;
116+
await this.restartQueryServerInternal(progress, token);
117+
}
118+
119+
/**
120+
* Try and restart the query server if it has unexpectedly terminated.
121+
*/
122+
private restartQueryServerOnFailure() {
123+
if (this.unexpectedTerminationCount < MAX_UNEXPECTED_TERMINATIONS) {
124+
void withProgress(
125+
async (progress, token) =>
126+
this.restartQueryServerInternal(progress, token),
127+
{
128+
title: "Restarting CodeQL query server due to unexpected termination",
129+
},
130+
);
131+
} else {
132+
void showAndLogErrorMessage(
133+
"The CodeQL query server has unexpectedly terminated too many times. Please check the logs for errors. You can manually restart the query server using the command 'CodeQL: Restart query server'.",
134+
);
135+
// Make sure we dispose anyway to reject all pending requests.
136+
this.serverProcess?.dispose();
137+
}
138+
this.unexpectedTerminationCount++;
139+
}
140+
141+
/**
142+
* Restarts the query server by disposing of the current server process and then starting a new one.
143+
* This does not reset the unexpected termination count.
144+
*/
145+
private async restartQueryServerInternal(
146+
progress: ProgressCallback,
147+
token: CancellationToken,
98148
): Promise<void> {
99149
this.stopQueryServer();
100150
await this.startQueryServer();
@@ -196,6 +246,9 @@ export class QueryServerClient extends DisposableObject {
196246
this.nextCallback = 0;
197247
this.nextProgress = 0;
198248
this.progressCallbacks = {};
249+
child.on("close", () => {
250+
this.restartQueryServerOnFailure();
251+
});
199252
}
200253

201254
get serverProcessPid(): number {

0 commit comments

Comments
 (0)