Merge pull request #2996 from github/koesie10/query-save-dir

Fix results directory and evaluator log for cancelled queries

Author: koesie10

extensions/ql-vscode/CHANGELOG.md

@@ -6,6 +6,7 @@
 - The "Install Pack Dependencies" will now only list CodeQL packs located in the workspace. [#2960](https://github.com/github/vscode-codeql/pull/2960)
 - Fix a bug where the "View Query Log" action for a query history item was not working. [#2984](https://github.com/github/vscode-codeql/pull/2984)
 - Add a command to sort items in the databases view by language. [#2993](https://github.com/github/vscode-codeql/pull/2993)
+- Fix not being able to open the results directory or evaluator log for a cancelled local query run. [#2996](https://github.com/github/vscode-codeql/pull/2996)
 
 ## 1.9.2 - 12 October 2023
 

extensions/ql-vscode/src/local-queries/local-queries.ts

@@ -362,11 +362,15 @@ export class LocalQueries extends DisposableObject {
       );
     }
 
-    const initialInfo = await createInitialQueryInfo(selectedQuery, {
-      databaseUri: dbItem.databaseUri.toString(),
-      name: dbItem.name,
-      language: tryGetQueryLanguage(dbItem.language),
-    });
+    const initialInfo = await createInitialQueryInfo(
+      selectedQuery,
+      {
+        databaseUri: dbItem.databaseUri.toString(),
+        name: dbItem.name,
+        language: tryGetQueryLanguage(dbItem.language),
+      },
+      outputDir,
+    );
 
     // When cancellation is requested from the query history view, we just stop the debug session.
     const queryInfo = new LocalQueryInfo(initialInfo, tokenSource);

extensions/ql-vscode/src/local-queries/local-query-run.ts

@@ -97,6 +97,15 @@ export class LocalQueryRun {
    * Updates the UI in the case where query evaluation throws an exception.
    */
   public async fail(err: Error): Promise<void> {
+    const evalLogPaths = await this.summarizeEvalLog(
+      QueryResultType.OTHER_ERROR,
+      this.outputDir,
+      this.logger,
+    );
+    if (evalLogPaths !== undefined) {
+      this.queryInfo.setEvaluatorLogPaths(evalLogPaths);
+    }
+
     err.message = `Error running query: ${err.message}`;
     this.queryInfo.failureReason = err.message;
     await this.queryHistoryManager.refreshTreeView();

extensions/ql-vscode/src/query-history/query-history-manager.ts

@@ -702,32 +702,15 @@ export class QueryHistoryManager extends DisposableObject {
     }
   }
 
-  async getQueryHistoryItemDirectory(
-    queryHistoryItem: QueryHistoryInfo,
-  ): Promise<string> {
-    if (queryHistoryItem.t === "local") {
-      if (queryHistoryItem.completedQuery) {
-        return queryHistoryItem.completedQuery.query.querySaveDir;
-      }
-    } else if (queryHistoryItem.t === "variant-analysis") {
-      return this.variantAnalysisManager.getVariantAnalysisStorageLocation(
-        queryHistoryItem.variantAnalysis.id,
-      );
-    } else {
-      assertNever(queryHistoryItem);
-    }
-
-    throw new Error("Unable to get query directory");
-  }
-
   async handleOpenQueryDirectory(item: QueryHistoryInfo) {
     let externalFilePath: string | undefined;
     if (item.t === "local") {
-      if (item.completedQuery) {
-        externalFilePath = join(
-          item.completedQuery.query.querySaveDir,
-          "timestamp",
-        );
+      const querySaveDir =
+        item.initialInfo.outputDir?.querySaveDir ??
+        item.completedQuery?.query.querySaveDir;
+
+      if (querySaveDir) {
+        externalFilePath = join(querySaveDir, "timestamp");
       }
     } else if (item.t === "variant-analysis") {
       externalFilePath = join(
@@ -761,37 +744,51 @@ export class QueryHistoryManager extends DisposableObject {
           `Failed to open ${externalFilePath}: ${getErrorMessage(e)}`,
         );
       }
+    } else {
+      this.warnNoQueryDir();
     }
   }
 
-  private warnNoEvalLogs() {
+  private warnNoQueryDir() {
     void showAndLogWarningMessage(
       this.app.logger,
-      `Evaluator log, summary, and viewer are not available for this run. Perhaps it failed before evaluation?`,
+      `Results directory is not available for this run.`,
     );
   }
 
-  private warnInProgressEvalLogSummary() {
+  private warnNoEvalLogs() {
     void showAndLogWarningMessage(
       this.app.logger,
-      'The evaluator log summary is still being generated for this run. Please try again later. The summary generation process is tracked in the "CodeQL Extension Log" view.',
+      `Evaluator log, summary, and viewer are not available for this run. Perhaps it failed before evaluation?`,
     );
   }
 
-  private warnInProgressEvalLogViewer() {
-    void showAndLogWarningMessage(
-      this.app.logger,
-      "The viewer's data is still being generated for this run. Please try again or re-run the query.",
-    );
+  private async warnNoEvalLogSummary(item: LocalQueryInfo) {
+    const evalLogLocation =
+      item.evalLogLocation ?? item.initialInfo.outputDir?.evalLogPath;
+
+    // Summary log file doesn't exist.
+    if (evalLogLocation && (await pathExists(evalLogLocation))) {
+      // If raw log does exist, then the summary log is still being generated.
+      void showAndLogWarningMessage(
+        this.app.logger,
+        'The evaluator log summary is still being generated for this run. Please try again later. The summary generation process is tracked in the "CodeQL Extension Log" view.',
+      );
+    } else {
+      this.warnNoEvalLogs();
+    }
   }
 
   async handleShowEvalLog(item: QueryHistoryInfo) {
     if (item.t !== "local") {
       return;
     }
 
-    if (item.evalLogLocation) {
-      await tryOpenExternalFile(this.app.commands, item.evalLogLocation);
+    const evalLogLocation =
+      item.evalLogLocation ?? item.initialInfo.outputDir?.evalLogPath;
+
+    if (evalLogLocation && (await pathExists(evalLogLocation))) {
+      await tryOpenExternalFile(this.app.commands, evalLogLocation);
     } else {
       this.warnNoEvalLogs();
     }
@@ -802,18 +799,13 @@ export class QueryHistoryManager extends DisposableObject {
       return;
     }
 
-    if (item.evalLogSummaryLocation) {
-      await tryOpenExternalFile(this.app.commands, item.evalLogSummaryLocation);
+    // If the summary file location wasn't saved, display error
+    if (!item.evalLogSummaryLocation) {
+      await this.warnNoEvalLogSummary(item);
       return;
     }
 
-    // Summary log file doesn't exist.
-    if (item.evalLogLocation && (await pathExists(item.evalLogLocation))) {
-      // If raw log does exist, then the summary log is still being generated.
-      this.warnInProgressEvalLogSummary();
-    } else {
-      this.warnNoEvalLogs();
-    }
+    await tryOpenExternalFile(this.app.commands, item.evalLogSummaryLocation);
   }
 
   async handleShowEvalLogViewer(item: QueryHistoryInfo) {
@@ -823,7 +815,7 @@ export class QueryHistoryManager extends DisposableObject {
 
     // If the JSON summary file location wasn't saved, display error
     if (item.jsonEvalLogSummaryLocation === undefined) {
-      this.warnInProgressEvalLogViewer();
+      await this.warnNoEvalLogSummary(item);
       return;
     }
 

extensions/ql-vscode/src/query-history/store/query-history-local-query-domain-mapper.ts

@@ -109,6 +109,11 @@ function mapInitialQueryInfoToDto(
     },
     start: localQueryInitialInfo.start,
     id: localQueryInitialInfo.id,
+    outputDir: localQueryInitialInfo.outputDir
+      ? {
+          querySaveDir: localQueryInitialInfo.outputDir.querySaveDir,
+        }
+      : undefined,
   };
 }
 

extensions/ql-vscode/src/query-history/store/query-history-local-query-dto-mapper.ts

@@ -3,7 +3,7 @@ import {
   CompletedQueryInfo,
   InitialQueryInfo,
 } from "../../query-results";
-import { QueryEvaluationInfo } from "../../run-queries-shared";
+import { QueryEvaluationInfo, QueryOutputDir } from "../../run-queries-shared";
 import {
   CompletedQueryInfoDto,
   QueryEvaluationInfoDto,
@@ -26,7 +26,10 @@ export function mapLocalQueryItemToDomainModel(
   localQuery: QueryHistoryLocalQueryDto,
 ): LocalQueryInfo {
   return new LocalQueryInfo(
-    mapInitialQueryInfoToDomainModel(localQuery.initialInfo),
+    mapInitialQueryInfoToDomainModel(
+      localQuery.initialInfo,
+      localQuery.completedQuery?.query?.querySaveDir,
+    ),
     undefined,
     localQuery.failureReason,
     localQuery.completedQuery &&
@@ -72,7 +75,14 @@ function mapCompletedQueryInfoToDomainModel(
 
 function mapInitialQueryInfoToDomainModel(
   initialInfo: InitialQueryInfoDto,
+  // The completedQuerySaveDir is a migration to support old query items that don't have
+  // the querySaveDir in the initialInfo. It should be removed once all query
+  // items have the querySaveDir in the initialInfo.
+  completedQuerySaveDir?: string,
 ): InitialQueryInfo {
+  const querySaveDir =
+    initialInfo.outputDir?.querySaveDir ?? completedQuerySaveDir;
+
   return {
     userSpecifiedLabel: initialInfo.userSpecifiedLabel,
     queryText: initialInfo.queryText,
@@ -90,6 +100,7 @@ function mapInitialQueryInfoToDomainModel(
     },
     start: new Date(initialInfo.start),
     id: initialInfo.id,
+    outputDir: querySaveDir ? new QueryOutputDir(querySaveDir) : undefined,
   };
 }
 

extensions/ql-vscode/src/query-history/store/query-history-local-query-dto.ts

@@ -24,6 +24,11 @@ export interface InitialQueryInfoDto {
   databaseInfo: DatabaseInfoDto;
   start: Date;
   id: string;
+  outputDir?: QueryOutputDirDto; // Undefined for backwards compatibility
+}
+
+interface QueryOutputDirDto {
+  querySaveDir: string;
 }
 
 interface DatabaseInfoDto {

extensions/ql-vscode/src/query-results.ts

@@ -19,6 +19,7 @@ import { QueryStatus } from "./query-history/query-status";
 import {
   EvaluatorLogPaths,
   QueryEvaluationInfo,
+  QueryOutputDir,
   QueryWithResults,
 } from "./run-queries-shared";
 import { formatLegacyMessage } from "./query-server/legacy";
@@ -47,6 +48,7 @@ export interface InitialQueryInfo {
   readonly databaseInfo: DatabaseInfo;
   readonly start: Date;
   readonly id: string; // unique id for this query.
+  readonly outputDir?: QueryOutputDir; // If missing, we do not have a query save dir. The query may have been cancelled. This is only for backwards compatibility.
 }
 
 export class CompletedQueryInfo implements QueryWithResults {

extensions/ql-vscode/src/run-queries-shared.ts

@@ -562,11 +562,13 @@ async function convertToQlPath(filePath: string): Promise<string> {
  *
  * @param selectedQuery The query to run, including any quickeval info.
  * @param databaseInfo The database to run the query against.
+ * @param outputDir The output directory for this query.
  * @returns The initial information for the query to be run.
  */
 export async function createInitialQueryInfo(
   selectedQuery: SelectedQuery,
   databaseInfo: DatabaseInfo,
+  outputDir: QueryOutputDir,
 ): Promise<InitialQueryInfo> {
   const isQuickEval = selectedQuery.quickEval !== undefined;
   const isQuickEvalCount =
@@ -587,6 +589,7 @@ export async function createInitialQueryInfo(
       : {
           queryText: await readFile(selectedQuery.queryPath, "utf8"),
         }),
+    outputDir,
   };
 }
 

extensions/ql-vscode/test/factories/query-history/local-query-history-item.ts

@@ -2,6 +2,7 @@ import { faker } from "@faker-js/faker";
 import { InitialQueryInfo, LocalQueryInfo } from "../../../src/query-results";
 import {
   QueryEvaluationInfo,
+  QueryOutputDir,
   QueryWithResults,
 } from "../../../src/run-queries-shared";
 import { CancellationTokenSource } from "vscode";
@@ -18,6 +19,7 @@ export function createMockLocalQueryInfo({
   hasMetadata = false,
   queryWithResults = undefined,
   language = undefined,
+  outputDir = new QueryOutputDir("/a/b/c"),
 }: {
   startTime?: Date;
   resultCount?: number;
@@ -27,6 +29,7 @@ export function createMockLocalQueryInfo({
   hasMetadata?: boolean;
   queryWithResults?: QueryWithResults | undefined;
   language?: QueryLanguage;
+  outputDir?: QueryOutputDir | undefined;
 }): LocalQueryInfo {
   const cancellationToken = {
     dispose: () => {
@@ -48,6 +51,7 @@ export function createMockLocalQueryInfo({
     start: startTime,
     id: faker.number.int().toString(),
     userSpecifiedLabel,
+    outputDir,
   } as InitialQueryInfo;
 
   const localQuery = new LocalQueryInfo(initialQueryInfo, cancellationToken);

extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts

@@ -8,7 +8,10 @@ import {
   LocalQueryInfo,
   InitialQueryInfo,
 } from "../../../../../src/query-results";
-import { QueryWithResults } from "../../../../../src/run-queries-shared";
+import {
+  QueryOutputDir,
+  QueryWithResults,
+} from "../../../../../src/run-queries-shared";
 import { DatabaseInfo } from "../../../../../src/common/interface-types";
 import { CancellationTokenSource, Uri } from "vscode";
 import { tmpDir } from "../../../../../src/tmp-dir";
@@ -130,6 +133,38 @@ describe("write and read", () => {
     expect(allHistoryActual.length).toEqual(expectedHistory.length);
   });
 
+  it("should read query output dir from completed query if not present", async () => {
+    const historyPath = join(tmpDir.name, "workspace-query-history.json");
+
+    const queryItem = createMockFullQueryInfo(
+      "a",
+      createMockQueryWithResults(
+        `${queryPath}-a`,
+        false,
+        false,
+        "/a/b/c/a",
+        false,
+      ),
+      false,
+      null,
+    );
+
+    // write and read
+    await writeQueryHistoryToFile([queryItem], historyPath);
+    const actual = await readQueryHistoryFromFile(historyPath);
+
+    expect(actual).toHaveLength(1);
+
+    expect(actual[0].t).toEqual("local");
+
+    if (actual[0].t === "local") {
+      expect(actual[0].initialInfo.outputDir?.querySaveDir).not.toBeUndefined();
+      expect(actual[0].initialInfo.outputDir?.querySaveDir).toEqual(
+        queryItem.completedQuery?.query?.querySaveDir,
+      );
+    }
+  });
+
   it("should remove remote queries from the history", async () => {
     const path = join(tmpDir.name, "query-history-with-remote.json");
     await writeFile(
@@ -205,6 +240,9 @@ describe("write and read", () => {
     dbName = "a",
     queryWithResults?: QueryWithResults,
     isFail = false,
+    outputDir: QueryOutputDir | null = new QueryOutputDir(
+      "/path/to/output/dir",
+    ),
   ): LocalQueryInfo {
     const fqi = new LocalQueryInfo(
       {
@@ -218,6 +256,7 @@ describe("write and read", () => {
         isQuickQuery: false,
         isQuickEval: false,
         id: `some-id-${dbName}`,
+        outputDir: outputDir ? outputDir : undefined,
       } as InitialQueryInfo,
       {
         dispose: () => {