diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts index 671ddb95e87..c3b48a9db63 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts @@ -14,6 +14,7 @@ import { sleep } from "../pure/time"; import { getErrorMessage } from "../pure/helpers-pure"; import { showAndLogWarningMessage } from "../helpers"; import { App } from "../common/app"; +import { extLogger } from "../common"; export class VariantAnalysisMonitor extends DisposableObject { // With a sleep of 5 seconds, the maximum number of attempts takes @@ -41,6 +42,8 @@ export class VariantAnalysisMonitor extends DisposableObject { let attemptCount = 0; const scannedReposDownloaded: number[] = []; + let lastErrorShown: string | undefined = undefined; + while (attemptCount <= VariantAnalysisMonitor.maxAttemptCount) { await sleep(VariantAnalysisMonitor.sleepTime); @@ -56,13 +59,22 @@ export class VariantAnalysisMonitor extends DisposableObject { variantAnalysis.id, ); } catch (e) { - void showAndLogWarningMessage( - `Error while monitoring variant analysis ${ - variantAnalysis.query.name - } (${variantAnalysis.query.language}) [${new Date( - variantAnalysis.executionStartTime, - ).toLocaleString(env.language)}]: ${getErrorMessage(e)}`, - ); + const errorMessage = getErrorMessage(e); + + const message = `Error while monitoring variant analysis ${ + variantAnalysis.query.name + } (${variantAnalysis.query.language}) [${new Date( + variantAnalysis.executionStartTime, + ).toLocaleString(env.language)}]: ${errorMessage}`; + + // If we have already shown this error to the user, don't show it again. + if (lastErrorShown === errorMessage) { + void extLogger.log(message); + } else { + void showAndLogWarningMessage(message); + lastErrorShown = errorMessage; + } + continue; } @@ -84,6 +96,9 @@ export class VariantAnalysisMonitor extends DisposableObject { } attemptCount++; + + // Reset the last error shown if we have successfully retrieved the variant analysis. + lastErrorShown = undefined; } } diff --git a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts index a8dd5d3e9ae..b42e059df3f 100644 --- a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts @@ -22,6 +22,7 @@ import { import { createMockVariantAnalysis } from "../../../factories/variant-analysis/shared/variant-analysis"; import { createMockApp } from "../../../__mocks__/appMock"; import { createMockCommandManager } from "../../../__mocks__/commandsMock"; +import * as helpers from "../../../../src/helpers"; jest.setTimeout(60_000); @@ -196,6 +197,93 @@ describe("Variant Analysis Monitor", () => { }); }); + describe("when some responses fail", () => { + let showAndLogWarningMessageSpy: jest.SpiedFunction< + typeof helpers.showAndLogWarningMessage + >; + + let scannedRepos: ApiVariantAnalysisScannedRepository[]; + + beforeEach(async () => { + showAndLogWarningMessageSpy = jest + .spyOn(helpers, "showAndLogWarningMessage") + .mockResolvedValue(undefined); + + scannedRepos = createMockScannedRepos([ + "pending", + "in_progress", + "in_progress", + "in_progress", + "pending", + "pending", + ]); + mockApiResponse = createMockApiResponse("in_progress", scannedRepos); + mockGetVariantAnalysis.mockResolvedValueOnce(mockApiResponse); + + mockGetVariantAnalysis.mockRejectedValueOnce( + new Error("No internet connection"), + ); + mockGetVariantAnalysis.mockRejectedValueOnce( + new Error("No internet connection"), + ); + mockGetVariantAnalysis.mockRejectedValueOnce( + new Error("My different error"), + ); + + let nextApiResponse = { + ...mockApiResponse, + scanned_repositories: [...scannedRepos.map((r) => ({ ...r }))], + }; + nextApiResponse.scanned_repositories[0].analysis_status = "succeeded"; + nextApiResponse.scanned_repositories[1].analysis_status = "succeeded"; + + mockGetVariantAnalysis.mockResolvedValueOnce(nextApiResponse); + + mockGetVariantAnalysis.mockRejectedValueOnce( + new Error("My different error"), + ); + mockGetVariantAnalysis.mockRejectedValueOnce( + new Error("My different error"), + ); + mockGetVariantAnalysis.mockRejectedValueOnce( + new Error("Another different error"), + ); + + nextApiResponse = { + ...mockApiResponse, + scanned_repositories: [...scannedRepos.map((r) => ({ ...r }))], + }; + nextApiResponse.scanned_repositories[2].analysis_status = "succeeded"; + nextApiResponse.scanned_repositories[3].analysis_status = "succeeded"; + nextApiResponse.scanned_repositories[4].analysis_status = "failed"; + nextApiResponse.scanned_repositories[5].analysis_status = "succeeded"; + nextApiResponse.status = "succeeded"; + mockGetVariantAnalysis.mockResolvedValueOnce(nextApiResponse); + }); + + it("should only trigger the warning once per error", async () => { + await variantAnalysisMonitor.monitorVariantAnalysis(variantAnalysis); + + expect(showAndLogWarningMessageSpy).toBeCalledTimes(4); + expect(showAndLogWarningMessageSpy).toHaveBeenNthCalledWith( + 1, + expect.stringMatching(/No internet connection/), + ); + expect(showAndLogWarningMessageSpy).toHaveBeenNthCalledWith( + 2, + expect.stringMatching(/My different error/), + ); + expect(showAndLogWarningMessageSpy).toHaveBeenNthCalledWith( + 3, + expect.stringMatching(/My different error/), + ); + expect(showAndLogWarningMessageSpy).toHaveBeenNthCalledWith( + 4, + expect.stringMatching(/Another different error/), + ); + }); + }); + describe("when there are no repos to scan", () => { beforeEach(async () => { scannedRepos = [];