From 9b9de5b8f3159dd97fbb9a53ac355056ac169f93 Mon Sep 17 00:00:00 2001 From: luca cappa Date: Fri, 25 Apr 2025 12:14:37 -0700 Subject: [PATCH] avoid reporting cancellation as errors --- .../src/LanguageServer/copilotProviders.ts | 21 +++++++++++-------- .../tests/copilotProviders.test.ts | 10 ++++----- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Extension/src/LanguageServer/copilotProviders.ts b/Extension/src/LanguageServer/copilotProviders.ts index 32a63013e..e0551edcb 100644 --- a/Extension/src/LanguageServer/copilotProviders.ts +++ b/Extension/src/LanguageServer/copilotProviders.ts @@ -31,7 +31,7 @@ export interface CopilotApi { uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken - ) => Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] }> + ) => Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] } | undefined> ): Disposable; getContextProviderAPI(version: string): Promise; } @@ -91,15 +91,18 @@ export async function registerRelatedFilesProvider(): Promise { return { entries: await includesPromise, traits: await traitsPromise }; } catch (exception) { - try { - const err: Error = exception as Error; - logger.getOutputChannelLogger().appendLine(localize("copilot.relatedfilesprovider.error", "Error while retrieving result. Reason: {0}", err.message)); + // Avoid logging the error message if it is a cancellation error. + if (exception instanceof vscode.CancellationError) { + telemetryProperties["error"] = "cancellation"; + telemetryProperties["cancellation"] = "true"; + throw exception; // Rethrow the cancellation error to be handled by the caller. + } else if (exception instanceof Error) { + telemetryProperties["error"] = "true"; + logger.getOutputChannelLogger().appendLine(localize("copilot.relatedfilesprovider.error", "Error while retrieving result. Reason: {0}", exception.message)); } - catch { - // Intentionally swallow any exception. - } - telemetryProperties["error"] = "true"; - throw exception; // Throw the exception for auto-retry. + + // In case of error retrieving the include files, we signal the caller of absence of the results by returning undefined. + return undefined; } finally { telemetryMetrics['duration'] = performance.now() - start; telemetry.logCopilotEvent('RelatedFilesProvider', telemetryProperties, telemetryMetrics); diff --git a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts index ba466323d..4a5c2c824 100644 --- a/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts +++ b/Extension/test/scenarios/SingleRootProject/tests/copilotProviders.test.ts @@ -24,7 +24,7 @@ describe('copilotProviders Tests', () => { let getClientsStub: sinon.SinonStub; let activeClientStub: sinon.SinonStubbedInstance; let vscodeGetExtensionsStub: sinon.SinonStub; - let callbackPromise: Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] }> | undefined; + let callbackPromise: Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] } | undefined> | undefined; let vscodeExtension: vscode.Extension; let telemetryStub: sinon.SinonStub; @@ -52,7 +52,7 @@ describe('copilotProviders Tests', () => { uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken - ) => Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] }> + ) => Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] } | undefined> ): vscode.Disposable & { [Symbol.dispose](): void } { return { dispose: () => { }, @@ -88,13 +88,13 @@ describe('copilotProviders Tests', () => { }); const arrange = ({ vscodeExtension, getIncludeFiles, projectContext, rootUri, flags }: - { vscodeExtension?: vscode.Extension; getIncludeFiles?: GetIncludesResult; projectContext?: ProjectContext; rootUri?: vscode.Uri; flags?: Record } = - { vscodeExtension: undefined, getIncludeFiles: undefined, projectContext: undefined, rootUri: undefined, flags: {} } + { vscodeExtension?: vscode.Extension; getIncludeFiles?: GetIncludesResult; projectContext?: ProjectContext; rootUri?: vscode.Uri; flags?: Record } = + { vscodeExtension: undefined, getIncludeFiles: undefined, projectContext: undefined, rootUri: undefined, flags: {} } ) => { activeClientStub.getIncludes.resolves(getIncludeFiles); sinon.stub(lmTool, 'getProjectContext').resolves(projectContext); sinon.stub(activeClientStub, 'RootUri').get(() => rootUri); - mockCopilotApi.registerRelatedFilesProvider.callsFake((_providerId: { extensionId: string; languageId: string }, callback: (uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken) => Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] }>) => { + mockCopilotApi.registerRelatedFilesProvider.callsFake((_providerId: { extensionId: string; languageId: string }, callback: (uri: vscode.Uri, context: { flags: Record }, cancellationToken: vscode.CancellationToken) => Promise<{ entries: vscode.Uri[]; traits?: CopilotTrait[] } | undefined>) => { if (_providerId.languageId === 'cpp') { const tokenSource = new vscode.CancellationTokenSource(); try {