Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark progress bars as cancellable where it appears we are respecting the token #3517

Merged
merged 6 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion extensions/ql-vscode/src/common/vscode/progress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ type ProgressOptions = Optional<VSCodeProgressOptions, "location">;
* denote some progress being achieved on this task.
* @param token a cancellation token
*/
export type ProgressTask<R> = (
type ProgressTask<R> = (
progress: ProgressCallback,
token: CancellationToken,
) => Thenable<R>;
Expand Down
6 changes: 2 additions & 4 deletions extensions/ql-vscode/src/databases/local-databases-ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -660,14 +660,13 @@ export class DatabaseUI extends DisposableObject {

private async handleClearCache(): Promise<void> {
return withProgress(
async (_progress, token) => {
async () => {
if (
this.queryServer !== undefined &&
this.databaseManager.currentDatabaseItem !== undefined
) {
await this.queryServer.clearCacheInDatabase(
this.databaseManager.currentDatabaseItem,
token,
);
}
},
Expand All @@ -679,14 +678,13 @@ export class DatabaseUI extends DisposableObject {

private async handleTrimCache(): Promise<void> {
return withProgress(
async (_progress, token) => {
async () => {
if (
this.queryServer !== undefined &&
this.databaseManager.currentDatabaseItem !== undefined
) {
await this.queryServer.trimCacheInDatabase(
this.databaseManager.currentDatabaseItem,
token,
);
}
},
Expand Down
4 changes: 2 additions & 2 deletions extensions/ql-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,11 @@ function getCommands(

const restartQueryServer = async () =>
withProgress(
async (progress: ProgressCallback, token: CancellationToken) => {
async (progress: ProgressCallback) => {
// Restart all of the spawned servers: cli, query, and language.
cliServer.restartCliServer();
await Promise.all([
queryRunner.restartQueryServer(progress, token),
queryRunner.restartQueryServer(progress),
async () => {
if (languageClient.isRunning()) {
await languageClient.restart();
Expand Down
97 changes: 50 additions & 47 deletions extensions/ql-vscode/src/model-editor/model-editor-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -810,57 +810,60 @@ export class ModelEditorView extends AbstractWebview<
}

private async modelDependency(): Promise<void> {
return withProgress(async (progress, token) => {
const addedDatabase =
await this.promptChooseNewOrExistingDatabase(progress);
if (!addedDatabase || token.isCancellationRequested) {
return;
}
return withProgress(
async (progress, token) => {
const addedDatabase =
await this.promptChooseNewOrExistingDatabase(progress);
if (!addedDatabase || token.isCancellationRequested) {
return;
}

const addedDbUri = addedDatabase.databaseUri.toString();
if (this.modelingStore.isDbOpen(addedDbUri)) {
this.modelingEvents.fireFocusModelEditorEvent(addedDbUri);
return;
}
const addedDbUri = addedDatabase.databaseUri.toString();
if (this.modelingStore.isDbOpen(addedDbUri)) {
this.modelingEvents.fireFocusModelEditorEvent(addedDbUri);
return;
}

const modelFile = await pickExtensionPack(
this.cliServer,
addedDatabase,
this.modelConfig,
this.app.logger,
progress,
token,
3,
);
if (!modelFile) {
return;
}
const modelFile = await pickExtensionPack(
this.cliServer,
addedDatabase,
this.modelConfig,
this.app.logger,
progress,
token,
3,
);
if (!modelFile) {
return;
}

// Check again just before opening the editor to ensure no model editor has been opened between
// our first check and now.
if (this.modelingStore.isDbOpen(addedDbUri)) {
this.modelingEvents.fireFocusModelEditorEvent(addedDbUri);
return;
}
// Check again just before opening the editor to ensure no model editor has been opened between
// our first check and now.
if (this.modelingStore.isDbOpen(addedDbUri)) {
this.modelingEvents.fireFocusModelEditorEvent(addedDbUri);
return;
}

const view = new ModelEditorView(
this.app,
this.modelingStore,
this.modelingEvents,
this.modelConfig,
this.databaseManager,
this.variantAnalysisManager,
this.cliServer,
this.queryRunner,
this.queryStorageDir,
this.queryDir,
addedDatabase,
modelFile,
this.language,
Mode.Framework,
);
await view.openView();
});
const view = new ModelEditorView(
this.app,
this.modelingStore,
this.modelingEvents,
this.modelConfig,
this.databaseManager,
this.variantAnalysisManager,
this.cliServer,
this.queryRunner,
this.queryStorageDir,
this.queryDir,
addedDatabase,
modelFile,
this.language,
Mode.Framework,
);
await view.openView();
},
{ cancellable: true },
);
}

private async promptChooseNewOrExistingDatabase(
Expand Down
28 changes: 7 additions & 21 deletions extensions/ql-vscode/src/query-server/query-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,26 +91,15 @@ export class QueryRunner {
return this.qs.logger;
}

async restartQueryServer(
progress: ProgressCallback,
token: CancellationToken,
): Promise<void> {
await this.qs.restartQueryServer(progress, token);
async restartQueryServer(progress: ProgressCallback): Promise<void> {
await this.qs.restartQueryServer(progress);
}

onStart(
callBack: (
progress: ProgressCallback,
token: CancellationToken,
) => Promise<void>,
) {
onStart(callBack: (progress: ProgressCallback) => Promise<void>) {
this.qs.onDidStartQueryServer(callBack);
}

async clearCacheInDatabase(
dbItem: DatabaseItem,
token: CancellationToken,
): Promise<void> {
async clearCacheInDatabase(dbItem: DatabaseItem): Promise<void> {
if (dbItem.contents === undefined) {
throw new Error("Can't clear the cache in an invalid database.");
}
Expand All @@ -120,13 +109,10 @@ export class QueryRunner {
dryRun: false,
db,
};
await this.qs.sendRequest(clearCache, params, token);
await this.qs.sendRequest(clearCache, params);
}

async trimCacheInDatabase(
dbItem: DatabaseItem,
token: CancellationToken,
): Promise<void> {
async trimCacheInDatabase(dbItem: DatabaseItem): Promise<void> {
if (dbItem.contents === undefined) {
throw new Error("Can't trim the cache in an invalid database.");
}
Expand All @@ -135,7 +121,7 @@ export class QueryRunner {
const params: TrimCacheParams = {
db,
};
await this.qs.sendRequest(trimCache, params, token);
await this.qs.sendRequest(trimCache, params);
}

public async compileAndRunQueryAgainstDatabaseCore(
Expand Down
23 changes: 11 additions & 12 deletions extensions/ql-vscode/src/query-server/query-server-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type { ProgressReporter } from "../common/logging/vscode";
import { extLogger } from "../common/logging/vscode";
import type { ProgressMessage, WithProgressId } from "./messages";
import { progress } from "./messages";
import type { ProgressCallback, ProgressTask } from "../common/vscode/progress";
import type { ProgressCallback } from "../common/vscode/progress";
import { withProgress } from "../common/vscode/progress";
import { ServerProcess } from "./server-process";
import type { App } from "../common/app";
Expand Down Expand Up @@ -51,12 +51,16 @@ export class QueryServerClient extends DisposableObject {

withProgressReporting: WithProgressReporting;

private readonly queryServerStartListeners = [] as Array<ProgressTask<void>>;
private readonly queryServerStartListeners = [] as Array<
(progress: ProgressCallback) => void
>;

// Can't use standard vscode EventEmitter here since they do not cause the calling
// function to fail if one of the event handlers fail. This is something that
// we need here.
readonly onDidStartQueryServer = (e: ProgressTask<void>) => {
readonly onDidStartQueryServer = (
e: (progress: ProgressCallback) => void,
) => {
this.queryServerStartListeners.push(e);
};

Expand Down Expand Up @@ -105,14 +109,11 @@ export class QueryServerClient extends DisposableObject {
* This resets the unexpected termination count. As hopefully it is an indication that the user has fixed the
* issue.
*/
async restartQueryServer(
progress: ProgressCallback,
token: CancellationToken,
): Promise<void> {
async restartQueryServer(progress: ProgressCallback): Promise<void> {
// Reset the unexpected termination count when we restart the query server manually
// or due to config change
this.unexpectedTerminationCount = 0;
await this.restartQueryServerInternal(progress, token);
await this.restartQueryServerInternal(progress);
}

/**
Expand All @@ -121,8 +122,7 @@ export class QueryServerClient extends DisposableObject {
private restartQueryServerOnFailure() {
if (this.unexpectedTerminationCount < MAX_UNEXPECTED_TERMINATIONS) {
void withProgress(
async (progress, token) =>
this.restartQueryServerInternal(progress, token),
async (progress) => this.restartQueryServerInternal(progress),
{
title: "Restarting CodeQL query server due to unexpected termination",
},
Expand All @@ -144,15 +144,14 @@ export class QueryServerClient extends DisposableObject {
*/
private async restartQueryServerInternal(
progress: ProgressCallback,
token: CancellationToken,
): Promise<void> {
this.stopQueryServer();
await this.startQueryServer();

// Ensure we await all responses from event handlers so that
// errors can be properly reported to the user.
await Promise.all(
this.queryServerStartListeners.map((handler) => handler(progress, token)),
this.queryServerStartListeners.map((handler) => handler(progress)),
);
}

Expand Down
Loading