Merge pull request #3517 from github/robertbrignull/fix-token-alerts

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

Author: robertbrignull

extensions/ql-vscode/src/common/vscode/progress.ts

@@ -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>;

extensions/ql-vscode/src/databases/local-databases-ui.ts

@@ -645,14 +645,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,
           );
         }
       },
@@ -664,14 +663,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,
           );
         }
       },

extensions/ql-vscode/src/extension.ts

@@ -184,11 +184,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();

extensions/ql-vscode/src/model-editor/model-editor-view.ts

@@ -814,58 +814,61 @@ 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.databaseFetcher,
-        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.databaseFetcher,
+          this.variantAnalysisManager,
+          this.cliServer,
+          this.queryRunner,
+          this.queryStorageDir,
+          this.queryDir,
+          addedDatabase,
+          modelFile,
+          this.language,
+          Mode.Framework,
+        );
+        await view.openView();
+      },
+      { cancellable: true },
+    );
   }
 
   private async promptChooseNewOrExistingDatabase(

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

@@ -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.");
     }
@@ -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.");
     }
@@ -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(

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

@@ -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";
@@ -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);
   };
 
@@ -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);
   }
 
   /**
@@ -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",
         },
@@ -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)),
     );
   }