From 28b16ed148eed5c90286eeddd747a60300e18127 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 18:02:06 +0000 Subject: [PATCH 01/18] Initial plan From 1e178d6fe719945896b10d1243703602dd363191 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 18:11:32 +0000 Subject: [PATCH 02/18] Add isOptional field to packages and handle optional package failures Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- src/packageManager/IPackage.ts | 1 + src/packageManager/absolutePathPackage.ts | 6 +- .../downloadAndInstallPackages.ts | 5 ++ .../downloadAndInstallPackages.test.ts | 73 +++++++++++++++++++ 4 files changed, 83 insertions(+), 2 deletions(-) diff --git a/src/packageManager/IPackage.ts b/src/packageManager/IPackage.ts index 91d0d826dc..2ede628b38 100644 --- a/src/packageManager/IPackage.ts +++ b/src/packageManager/IPackage.ts @@ -13,4 +13,5 @@ export interface IPackage { platformId?: string; integrity?: string; isFramework?: boolean; + isOptional?: boolean; } diff --git a/src/packageManager/absolutePathPackage.ts b/src/packageManager/absolutePathPackage.ts index 5505dc43eb..578ff2d1e3 100644 --- a/src/packageManager/absolutePathPackage.ts +++ b/src/packageManager/absolutePathPackage.ts @@ -20,7 +20,8 @@ export class AbsolutePathPackage implements IPackage { public fallbackUrl?: string, public platformId?: string, public integrity?: string, - public isFramework?: boolean + public isFramework?: boolean, + public isOptional?: boolean ) {} public static getAbsolutePathPackage(pkg: Package, extensionPath: string) { @@ -36,7 +37,8 @@ export class AbsolutePathPackage implements IPackage { pkg.fallbackUrl, pkg.platformId, pkg.integrity, - pkg.isFramework + pkg.isFramework, + pkg.isOptional ); } } diff --git a/src/packageManager/downloadAndInstallPackages.ts b/src/packageManager/downloadAndInstallPackages.ts index a148a666e4..eccfdbf830 100644 --- a/src/packageManager/downloadAndInstallPackages.ts +++ b/src/packageManager/downloadAndInstallPackages.ts @@ -61,6 +61,11 @@ export async function downloadAndInstallPackages( eventStream.post(new InstallationFailure(installationStage, error)); } + // If the package is optional, log and continue with the next package + if (pkg.isOptional) { + continue; + } + return false; } finally { try { diff --git a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts index 9d8854a2bf..0d8cff9006 100644 --- a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts +++ b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts @@ -218,6 +218,79 @@ describe(`${downloadAndInstallPackages.name}`, () => { }); }); + describe('Optional packages', () => { + test('Returns true and continues when an optional package fails to download', async () => { + const optionalPackage = [ + { + url: `${server.baseUrl}/notDownloadablePackage`, + description: 'Optional Package', + installPath: new AbsolutePath(tmpDirPath), + isOptional: true, + }, + ]; + + const result = await downloadAndInstallPackages( + optionalPackage, + networkSettingsProvider, + eventStream, + downloadValidator + ); + expect(result).toBe(true); + }); + + test('Continues to install remaining packages after optional package fails', async () => { + const tmpInstallDir2 = await CreateTmpDir(true); + const mixedPackages = [ + { + url: `${server.baseUrl}/notDownloadablePackage`, + description: 'Optional Package', + installPath: new AbsolutePath(tmpDirPath), + isOptional: true, + }, + { + url: `${server.baseUrl}/downloadablePackage`, + description: packageDescription, + installPath: new AbsolutePath(tmpInstallDir2.name), + }, + ]; + + const result = await downloadAndInstallPackages( + mixedPackages, + networkSettingsProvider, + eventStream, + downloadValidator + ); + expect(result).toBe(true); + expect(await util.fileExists(path.join(tmpInstallDir2.name, 'install.Lock'))).toBe(true); + tmpInstallDir2.dispose(); + }); + + test('InstallationFailure event is logged for optional package', async () => { + const optionalPackage = [ + { + url: `${server.baseUrl}/notDownloadablePackage`, + description: 'Optional Package', + installPath: new AbsolutePath(tmpDirPath), + isOptional: true, + }, + ]; + + eventBus.getEvents(); // Clear any previous events + await downloadAndInstallPackages( + optionalPackage, + networkSettingsProvider, + eventStream, + downloadValidator + ); + const obtainedEvents = eventBus.getEvents(); + const installationFailureEvent = obtainedEvents.find( + (event) => event instanceof InstallationFailure + ) as InstallationFailure; + expect(installationFailureEvent).toBeDefined(); + expect(installationFailureEvent.stage).toEqual('downloadPackage'); + }); + }); + afterEach(async () => { if (tmpInstallDir) { tmpInstallDir.dispose(); From 1ffa0e09066bde0a58f33c2915e63622e58361e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 18:13:52 +0000 Subject: [PATCH 03/18] Fix linting formatting Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- .../packages/downloadAndInstallPackages.test.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts index 0d8cff9006..9be17a0f4b 100644 --- a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts +++ b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts @@ -276,12 +276,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { ]; eventBus.getEvents(); // Clear any previous events - await downloadAndInstallPackages( - optionalPackage, - networkSettingsProvider, - eventStream, - downloadValidator - ); + await downloadAndInstallPackages(optionalPackage, networkSettingsProvider, eventStream, downloadValidator); const obtainedEvents = eventBus.getEvents(); const installationFailureEvent = obtainedEvents.find( (event) => event instanceof InstallationFailure From e0152c9f78342a59c86f73087180d06ea559e6b9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 19:10:15 +0000 Subject: [PATCH 04/18] Add telemetry reporting for optional package installation failures Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- .gitignore | 1 + src/main.ts | 2 +- src/omnisharp/omnisharpDownloader.ts | 3 +- .../downloadAndInstallPackages.ts | 22 ++++++++++++ src/razor/razorOmnisharpDownloader.ts | 3 +- tasks/offlinePackagingTasks.ts | 4 ++- .../downloadAndInstallPackages.test.ts | 35 +++++++++++++------ 7 files changed, 56 insertions(+), 14 deletions(-) diff --git a/.gitignore b/.gitignore index 291b7b01bb..0dea598e3e 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,4 @@ coverage/ \.DS_Store vsix/ +*.bak diff --git a/src/main.ts b/src/main.ts index 977e415f5f..b001f2eb35 100644 --- a/src/main.ts +++ b/src/main.ts @@ -86,7 +86,7 @@ export async function activate( const networkSettingsProvider = vscodeNetworkSettingsProvider(vscode); const useFramework = useOmnisharpServer && omnisharpOptions.useModernNet !== true; const installDependencies: IInstallDependencies = async (dependencies: AbsolutePathPackage[]) => - downloadAndInstallPackages(dependencies, networkSettingsProvider, eventStream, isValidDownload); + downloadAndInstallPackages(dependencies, networkSettingsProvider, eventStream, isValidDownload, reporter); const runtimeDependenciesExist = await installRuntimeDependencies( context.extension.packageJSON, diff --git a/src/omnisharp/omnisharpDownloader.ts b/src/omnisharp/omnisharpDownloader.ts index 03be20748f..e90dc61662 100644 --- a/src/omnisharp/omnisharpDownloader.ts +++ b/src/omnisharp/omnisharpDownloader.ts @@ -56,7 +56,8 @@ export class OmnisharpDownloader { packagesToInstall, this.networkSettingsProvider, this.eventStream, - isValidDownload + isValidDownload, + undefined ) ) { this.eventStream.post(new InstallationSuccess()); diff --git a/src/packageManager/downloadAndInstallPackages.ts b/src/packageManager/downloadAndInstallPackages.ts index eccfdbf830..ce1853f205 100644 --- a/src/packageManager/downloadAndInstallPackages.ts +++ b/src/packageManager/downloadAndInstallPackages.ts @@ -16,12 +16,14 @@ import { mkdirpSync } from 'fs-extra'; import { PackageInstallStart } from '../shared/loggingEvents'; import { DownloadValidator } from './isValidDownload'; import { CancellationToken } from 'vscode'; +import { ITelemetryReporter } from '../shared/telemetryReporter'; export async function downloadAndInstallPackages( packages: AbsolutePathPackage[], provider: NetworkSettingsProvider, eventStream: EventStream, downloadValidator: DownloadValidator, + telemetryReporter?: ITelemetryReporter, token?: CancellationToken ): Promise { eventStream.post(new PackageInstallStart()); @@ -61,6 +63,26 @@ export async function downloadAndInstallPackages( eventStream.post(new InstallationFailure(installationStage, error)); } + // Send telemetry for the failure + if (telemetryReporter) { + const telemetryProperties: { [key: string]: string } = { + installStage: installationStage, + packageId: pkg.id, + isOptional: pkg.isOptional ? 'true' : 'false', + }; + + if (error instanceof NestedError && error.err instanceof PackageError) { + telemetryProperties['error.message'] = error.err.message; + telemetryProperties['error.packageUrl'] = error.err.pkg.url; + } else if (error instanceof PackageError) { + telemetryProperties['error.message'] = error.message; + telemetryProperties['error.packageUrl'] = error.pkg.url; + } + + const eventName = pkg.isOptional ? 'OptionalPackageInstallationFailed' : 'PackageInstallationFailed'; + telemetryReporter.sendTelemetryEvent(eventName, telemetryProperties); + } + // If the package is optional, log and continue with the next package if (pkg.isOptional) { continue; diff --git a/src/razor/razorOmnisharpDownloader.ts b/src/razor/razorOmnisharpDownloader.ts index 965f7cf99a..e87b0e3fcb 100644 --- a/src/razor/razorOmnisharpDownloader.ts +++ b/src/razor/razorOmnisharpDownloader.ts @@ -38,7 +38,8 @@ export class RazorOmnisharpDownloader { packagesToInstall, this.networkSettingsProvider, this.eventStream, - isValidDownload + isValidDownload, + undefined ) ) { this.eventStream.post(new InstallationSuccess()); diff --git a/tasks/offlinePackagingTasks.ts b/tasks/offlinePackagingTasks.ts index 531e0b7adf..fb9714b4c9 100644 --- a/tasks/offlinePackagingTasks.ts +++ b/tasks/offlinePackagingTasks.ts @@ -280,7 +280,9 @@ async function installPackageJsonDependency( codeExtensionPath ); const provider = () => new NetworkSettings('', true); - if (!(await downloadAndInstallPackages(packagesToInstall, provider, eventStream, isValidDownload, token))) { + if ( + !(await downloadAndInstallPackages(packagesToInstall, provider, eventStream, isValidDownload, undefined, token)) + ) { throw Error('Failed to download package.'); } } diff --git a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts index 9be17a0f4b..33647e8f90 100644 --- a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts +++ b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts @@ -87,7 +87,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); for (const elem of testZip.files) { const filePath = path.join(tmpDirPath, elem.path); @@ -100,7 +101,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); expect(await util.fileExists(path.join(tmpDirPath, 'install.Lock'))).toBe(true); }); @@ -119,7 +121,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); expect(eventBus.getEvents()).toStrictEqual(eventsSequence); }); @@ -153,7 +156,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); expect(eventBus.getEvents()).toStrictEqual(eventsSequence); }); @@ -180,7 +184,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); expect(eventBus.getEvents()).toStrictEqual(eventsSequence); }); @@ -196,7 +201,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { notDownloadablePackage, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); const obtainedEvents = eventBus.getEvents(); expect(obtainedEvents[0]).toStrictEqual(eventsSequence[0]); @@ -212,7 +218,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { notDownloadablePackage, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); expect(await util.fileExists(path.join(tmpDirPath, 'install.Lock'))).toBe(false); }); @@ -233,7 +240,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { optionalPackage, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); expect(result).toBe(true); }); @@ -258,7 +266,8 @@ describe(`${downloadAndInstallPackages.name}`, () => { mixedPackages, networkSettingsProvider, eventStream, - downloadValidator + downloadValidator, + undefined ); expect(result).toBe(true); expect(await util.fileExists(path.join(tmpInstallDir2.name, 'install.Lock'))).toBe(true); @@ -276,7 +285,13 @@ describe(`${downloadAndInstallPackages.name}`, () => { ]; eventBus.getEvents(); // Clear any previous events - await downloadAndInstallPackages(optionalPackage, networkSettingsProvider, eventStream, downloadValidator); + await downloadAndInstallPackages( + optionalPackage, + networkSettingsProvider, + eventStream, + downloadValidator, + undefined + ); const obtainedEvents = eventBus.getEvents(); const installationFailureEvent = obtainedEvents.find( (event) => event instanceof InstallationFailure From 9c0d446966f915bd3c6317b0788b254f7122b38e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 19:20:41 +0000 Subject: [PATCH 05/18] Handle optional components gracefully when files don't exist Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- src/lsptoolshost/extensions/builtInComponents.ts | 10 +++++++++- src/lsptoolshost/server/roslynLanguageServer.ts | 4 ++-- src/razor/src/extension.ts | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/lsptoolshost/extensions/builtInComponents.ts b/src/lsptoolshost/extensions/builtInComponents.ts index af657fa165..9e1868c7d8 100644 --- a/src/lsptoolshost/extensions/builtInComponents.ts +++ b/src/lsptoolshost/extensions/builtInComponents.ts @@ -44,12 +44,20 @@ export const componentInfo: { [key: string]: ComponentInfo } = { }, }; -export function getComponentPaths(componentName: string, options: LanguageServerOptions | undefined): string[] { +export function getComponentPaths( + componentName: string, + options: LanguageServerOptions | undefined, + isOptional = false +): string[] { const component = componentInfo[componentName]; const baseFolder = getComponentFolderPath(component, options); const paths = component.componentDllPaths.map((dllPath) => path.join(baseFolder, dllPath)); for (const dllPath of paths) { if (!fs.existsSync(dllPath)) { + if (isOptional) { + // Component is optional and doesn't exist - return empty array + return []; + } throw new Error(`Component DLL not found: ${dllPath}`); } } diff --git a/src/lsptoolshost/server/roslynLanguageServer.ts b/src/lsptoolshost/server/roslynLanguageServer.ts index 8d1951d1f4..57e18ac69b 100644 --- a/src/lsptoolshost/server/roslynLanguageServer.ts +++ b/src/lsptoolshost/server/roslynLanguageServer.ts @@ -1026,7 +1026,7 @@ export class RoslynLanguageServer { // Also include the Xaml Dev Kit extensions, if enabled. if (languageServerOptions.enableXamlTools) { - getComponentPaths('xamlTools', languageServerOptions).forEach((path) => + getComponentPaths('xamlTools', languageServerOptions, true).forEach((path) => additionalExtensionPaths.push(path) ); } @@ -1096,7 +1096,7 @@ export class RoslynLanguageServer { await exports.setupTelemetryEnvironmentAsync(env); } - getComponentPaths('roslynCopilot', languageServerOptions).forEach((extPath) => { + getComponentPaths('roslynCopilot', languageServerOptions, true).forEach((extPath) => { additionalExtensionPaths.push(extPath); }); } diff --git a/src/razor/src/extension.ts b/src/razor/src/extension.ts index 8fdac6c67b..990a8d380c 100644 --- a/src/razor/src/extension.ts +++ b/src/razor/src/extension.ts @@ -104,7 +104,7 @@ export async function activate( await setupDevKitEnvironment(dotnetInfo.env, csharpDevkitExtension, logger); if (vscode.env.isTelemetryEnabled) { - const razorComponentPaths = getComponentPaths('razorDevKit', undefined); + const razorComponentPaths = getComponentPaths('razorDevKit', undefined, true); if (razorComponentPaths.length !== 1) { logger.logError('Failed to find Razor DevKit telemetry extension path.', undefined); } else { From f611c8a4b746e55ffaa75b0b3ea5eded36cbeefd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 20:02:18 +0000 Subject: [PATCH 06/18] Refactor isOptional to be part of ComponentInfo interface Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- src/lsptoolshost/extensions/builtInComponents.ts | 12 ++++++------ src/lsptoolshost/server/roslynLanguageServer.ts | 4 ++-- src/razor/src/extension.ts | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/lsptoolshost/extensions/builtInComponents.ts b/src/lsptoolshost/extensions/builtInComponents.ts index 9e1868c7d8..119aca8bd7 100644 --- a/src/lsptoolshost/extensions/builtInComponents.ts +++ b/src/lsptoolshost/extensions/builtInComponents.ts @@ -11,6 +11,7 @@ interface ComponentInfo { defaultFolderName: string; optionName: string; componentDllPaths: string[]; + isOptional?: boolean; } export const componentInfo: { [key: string]: ComponentInfo } = { @@ -26,11 +27,13 @@ export const componentInfo: { [key: string]: ComponentInfo } = { 'Microsoft.VisualStudio.DesignTools.CodeAnalysis.dll', 'Microsoft.VisualStudio.DesignTools.CodeAnalysis.Diagnostics.dll', ], + isOptional: true, }, razorDevKit: { defaultFolderName: '.razorDevKit', optionName: 'razorDevKit', componentDllPaths: ['Microsoft.VisualStudio.DevKit.Razor.dll'], + isOptional: true, }, razorExtension: { defaultFolderName: '.razorExtension', @@ -41,20 +44,17 @@ export const componentInfo: { [key: string]: ComponentInfo } = { defaultFolderName: '.roslynCopilot', optionName: 'roslynCopilot', componentDllPaths: ['Microsoft.VisualStudio.Copilot.Roslyn.LanguageServer.dll'], + isOptional: true, }, }; -export function getComponentPaths( - componentName: string, - options: LanguageServerOptions | undefined, - isOptional = false -): string[] { +export function getComponentPaths(componentName: string, options: LanguageServerOptions | undefined): string[] { const component = componentInfo[componentName]; const baseFolder = getComponentFolderPath(component, options); const paths = component.componentDllPaths.map((dllPath) => path.join(baseFolder, dllPath)); for (const dllPath of paths) { if (!fs.existsSync(dllPath)) { - if (isOptional) { + if (component.isOptional) { // Component is optional and doesn't exist - return empty array return []; } diff --git a/src/lsptoolshost/server/roslynLanguageServer.ts b/src/lsptoolshost/server/roslynLanguageServer.ts index 57e18ac69b..8d1951d1f4 100644 --- a/src/lsptoolshost/server/roslynLanguageServer.ts +++ b/src/lsptoolshost/server/roslynLanguageServer.ts @@ -1026,7 +1026,7 @@ export class RoslynLanguageServer { // Also include the Xaml Dev Kit extensions, if enabled. if (languageServerOptions.enableXamlTools) { - getComponentPaths('xamlTools', languageServerOptions, true).forEach((path) => + getComponentPaths('xamlTools', languageServerOptions).forEach((path) => additionalExtensionPaths.push(path) ); } @@ -1096,7 +1096,7 @@ export class RoslynLanguageServer { await exports.setupTelemetryEnvironmentAsync(env); } - getComponentPaths('roslynCopilot', languageServerOptions, true).forEach((extPath) => { + getComponentPaths('roslynCopilot', languageServerOptions).forEach((extPath) => { additionalExtensionPaths.push(extPath); }); } diff --git a/src/razor/src/extension.ts b/src/razor/src/extension.ts index 990a8d380c..8fdac6c67b 100644 --- a/src/razor/src/extension.ts +++ b/src/razor/src/extension.ts @@ -104,7 +104,7 @@ export async function activate( await setupDevKitEnvironment(dotnetInfo.env, csharpDevkitExtension, logger); if (vscode.env.isTelemetryEnabled) { - const razorComponentPaths = getComponentPaths('razorDevKit', undefined, true); + const razorComponentPaths = getComponentPaths('razorDevKit', undefined); if (razorComponentPaths.length !== 1) { logger.logError('Failed to find Razor DevKit telemetry extension path.', undefined); } else { From bd12e03ef214890f3c40f116bb11cd261434475f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 20:14:25 +0000 Subject: [PATCH 07/18] Revert .gitignore change that added *.bak Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- .gitignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.gitignore b/.gitignore index 0dea598e3e..291b7b01bb 100644 --- a/.gitignore +++ b/.gitignore @@ -36,4 +36,3 @@ coverage/ \.DS_Store vsix/ -*.bak From 60894e9ebddbd35d82ad9972ae9bf5f42e9569bc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 20:45:52 +0000 Subject: [PATCH 08/18] Thread ITelemetryReporter through OmniSharp components Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- src/omnisharp/omnisharpDownloader.ts | 6 ++++-- src/omnisharp/omnisharpLanguageServer.ts | 9 ++++++--- src/omnisharp/server.ts | 7 +++++-- .../omnisharpUnitTests/omnisharpDownloader.test.ts | 3 ++- .../omnisharpUnitTests/omnisharpManager.test.ts | 3 ++- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/omnisharp/omnisharpDownloader.ts b/src/omnisharp/omnisharpDownloader.ts index e90dc61662..0924284585 100644 --- a/src/omnisharp/omnisharpDownloader.ts +++ b/src/omnisharp/omnisharpDownloader.ts @@ -19,6 +19,7 @@ import { getRuntimeDependenciesPackages } from '../tools/runtimeDependencyPackag import { getAbsolutePathPackagesToInstall } from '../packageManager/getAbsolutePathPackagesToInstall'; import { isValidDownload } from '../packageManager/isValidDownload'; import { LatestBuildDownloadStart } from './omnisharpLoggingEvents'; +import { ITelemetryReporter } from '../shared/telemetryReporter'; export class OmnisharpDownloader { public constructor( @@ -26,7 +27,8 @@ export class OmnisharpDownloader { private eventStream: EventStream, private packageJSON: any, private platformInfo: PlatformInformation, - private extensionPath: string + private extensionPath: string, + private reporter: ITelemetryReporter ) {} public async DownloadAndInstallOmnisharp( @@ -57,7 +59,7 @@ export class OmnisharpDownloader { this.networkSettingsProvider, this.eventStream, isValidDownload, - undefined + this.reporter ) ) { this.eventStream.post(new InstallationSuccess()); diff --git a/src/omnisharp/omnisharpLanguageServer.ts b/src/omnisharp/omnisharpLanguageServer.ts index 619a10ba1a..2226b49c6c 100644 --- a/src/omnisharp/omnisharpLanguageServer.ts +++ b/src/omnisharp/omnisharpLanguageServer.ts @@ -178,7 +178,8 @@ export async function activateOmniSharpLanguageServer( networkSettingsProvider, eventStream, context.extension.extensionPath, - omnisharpChannel + omnisharpChannel, + reporter ); } @@ -189,7 +190,8 @@ async function activate( provider: NetworkSettingsProvider, eventStream: EventStream, extensionPath: string, - outputChannel: vscode.OutputChannel + outputChannel: vscode.OutputChannel, + reporter: ITelemetryReporter ) { const disposables = new CompositeDisposable(); @@ -211,7 +213,8 @@ async function activate( omnisharpDotnetResolver, context, outputChannel, - languageMiddlewareFeature + languageMiddlewareFeature, + reporter ); const advisor = new Advisor(server); // create before server is started const testManager = new TestManager(server, eventStream, languageMiddlewareFeature); diff --git a/src/omnisharp/server.ts b/src/omnisharp/server.ts index bc60e6f8b8..49a2cc879d 100644 --- a/src/omnisharp/server.ts +++ b/src/omnisharp/server.ts @@ -34,6 +34,7 @@ import TestManager from './features/dotnetTest'; import { findLaunchTargets } from './launcher'; import { ProjectConfigurationMessage } from '../shared/projectConfiguration'; import { commonOptions, omnisharpOptions, razorOptions } from '../shared/options'; +import { ITelemetryReporter } from '../shared/telemetryReporter'; enum ServerState { Starting, @@ -117,14 +118,16 @@ export class OmniSharpServer { private dotnetResolver: IHostExecutableResolver, private context: ExtensionContext, private outputChannel: OutputChannel, - private languageMiddlewareFeature: LanguageMiddlewareFeature + private languageMiddlewareFeature: LanguageMiddlewareFeature, + reporter: ITelemetryReporter ) { const downloader = new OmnisharpDownloader( networkSettingsProvider, this.eventStream, this.packageJSON, platformInfo, - extensionPath + extensionPath, + reporter ); this._omnisharpManager = new OmnisharpManager(downloader, platformInfo); this.updateProjectDebouncer.pipe(debounceTime(1500)).subscribe(async (_) => { diff --git a/test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts b/test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts index 74d2b999f7..6047bbc87f 100644 --- a/test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts +++ b/test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts @@ -54,7 +54,8 @@ import { modernNetVersion } from '../../../src/omnisharp/omnisharpPackageCreator eventStream, testPackageJSON, platformInfo, - extensionPath + extensionPath, + undefined as any ); server = await MockHttpsServer.CreateMockHttpsServer(); testZip = await TestZip.createTestZipAsync(createTestFile('Foo', 'foo.txt')); diff --git a/test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts b/test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts index 78f882771e..586012ca8e 100644 --- a/test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts +++ b/test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts @@ -245,7 +245,8 @@ function GetTestOmniSharpManager( eventStream, testPackageJSON, platformInfo, - extensionPath + extensionPath, + undefined as any ); return new OmnisharpManager(downloader, platformInfo, serverUrl); } From e13d0b99a2999fa0f956a193a5d7cbd28554d92d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 21:49:02 +0000 Subject: [PATCH 09/18] Refactor telemetry and make reporter parameters optional Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- src/omnisharp/omnisharpDownloader.ts | 2 +- src/omnisharp/omnisharpLanguageServer.ts | 3 +- .../downloadAndInstallPackages.ts | 42 +++++++++++-------- src/razor/razorOmnisharpDownloader.ts | 6 ++- .../omnisharpDownloader.test.ts | 3 +- .../omnisharpManager.test.ts | 3 +- .../downloadAndInstallPackages.test.ts | 35 +++++----------- 7 files changed, 43 insertions(+), 51 deletions(-) diff --git a/src/omnisharp/omnisharpDownloader.ts b/src/omnisharp/omnisharpDownloader.ts index 0924284585..8b3ad510ea 100644 --- a/src/omnisharp/omnisharpDownloader.ts +++ b/src/omnisharp/omnisharpDownloader.ts @@ -28,7 +28,7 @@ export class OmnisharpDownloader { private packageJSON: any, private platformInfo: PlatformInformation, private extensionPath: string, - private reporter: ITelemetryReporter + private reporter?: ITelemetryReporter ) {} public async DownloadAndInstallOmnisharp( diff --git a/src/omnisharp/omnisharpLanguageServer.ts b/src/omnisharp/omnisharpLanguageServer.ts index 2226b49c6c..1c5325c328 100644 --- a/src/omnisharp/omnisharpLanguageServer.ts +++ b/src/omnisharp/omnisharpLanguageServer.ts @@ -154,7 +154,8 @@ export async function activateOmniSharpLanguageServer( eventStream, context.extension.packageJSON, platformInfo, - context.extension.extensionPath + context.extension.extensionPath, + reporter ); await razorOmnisharpDownloader.DownloadAndInstallRazorOmnisharp( diff --git a/src/packageManager/downloadAndInstallPackages.ts b/src/packageManager/downloadAndInstallPackages.ts index ce1853f205..04aa4811d9 100644 --- a/src/packageManager/downloadAndInstallPackages.ts +++ b/src/packageManager/downloadAndInstallPackages.ts @@ -26,6 +26,29 @@ export async function downloadAndInstallPackages( telemetryReporter?: ITelemetryReporter, token?: CancellationToken ): Promise { + function sendInstallationFailureTelemetry(pkg: AbsolutePathPackage, installationStage: string, error: any): void { + if (!telemetryReporter) { + return; + } + + const telemetryProperties: { [key: string]: string } = { + installStage: installationStage, + packageId: pkg.id, + isOptional: pkg.isOptional ? 'true' : 'false', + }; + + if (error instanceof NestedError && error.err instanceof PackageError) { + telemetryProperties['error.message'] = error.err.message; + telemetryProperties['error.packageUrl'] = error.err.pkg.url; + } else if (error instanceof PackageError) { + telemetryProperties['error.message'] = error.message; + telemetryProperties['error.packageUrl'] = error.pkg.url; + } + + const eventName = pkg.isOptional ? 'OptionalPackageInstallationFailed' : 'PackageInstallationFailed'; + telemetryReporter.sendTelemetryEvent(eventName, telemetryProperties); + } + eventStream.post(new PackageInstallStart()); for (const pkg of packages) { let installationStage = 'touchBeginFile'; @@ -64,24 +87,7 @@ export async function downloadAndInstallPackages( } // Send telemetry for the failure - if (telemetryReporter) { - const telemetryProperties: { [key: string]: string } = { - installStage: installationStage, - packageId: pkg.id, - isOptional: pkg.isOptional ? 'true' : 'false', - }; - - if (error instanceof NestedError && error.err instanceof PackageError) { - telemetryProperties['error.message'] = error.err.message; - telemetryProperties['error.packageUrl'] = error.err.pkg.url; - } else if (error instanceof PackageError) { - telemetryProperties['error.message'] = error.message; - telemetryProperties['error.packageUrl'] = error.pkg.url; - } - - const eventName = pkg.isOptional ? 'OptionalPackageInstallationFailed' : 'PackageInstallationFailed'; - telemetryReporter.sendTelemetryEvent(eventName, telemetryProperties); - } + sendInstallationFailureTelemetry(pkg, installationStage, error); // If the package is optional, log and continue with the next package if (pkg.isOptional) { diff --git a/src/razor/razorOmnisharpDownloader.ts b/src/razor/razorOmnisharpDownloader.ts index e87b0e3fcb..ffd433ba81 100644 --- a/src/razor/razorOmnisharpDownloader.ts +++ b/src/razor/razorOmnisharpDownloader.ts @@ -11,6 +11,7 @@ import { downloadAndInstallPackages } from '../packageManager/downloadAndInstall import { getRuntimeDependenciesPackages } from '../tools/runtimeDependencyPackageUtils'; import { getAbsolutePathPackagesToInstall } from '../packageManager/getAbsolutePathPackagesToInstall'; import { isValidDownload } from '../packageManager/isValidDownload'; +import { ITelemetryReporter } from '../shared/telemetryReporter'; export class RazorOmnisharpDownloader { public constructor( @@ -18,7 +19,8 @@ export class RazorOmnisharpDownloader { private eventStream: EventStream, private packageJSON: any, private platformInfo: PlatformInformation, - private extensionPath: string + private extensionPath: string, + private reporter?: ITelemetryReporter ) {} public async DownloadAndInstallRazorOmnisharp(version: string): Promise { @@ -39,7 +41,7 @@ export class RazorOmnisharpDownloader { this.networkSettingsProvider, this.eventStream, isValidDownload, - undefined + this.reporter ) ) { this.eventStream.post(new InstallationSuccess()); diff --git a/test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts b/test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts index 6047bbc87f..74d2b999f7 100644 --- a/test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts +++ b/test/omnisharp/omnisharpUnitTests/omnisharpDownloader.test.ts @@ -54,8 +54,7 @@ import { modernNetVersion } from '../../../src/omnisharp/omnisharpPackageCreator eventStream, testPackageJSON, platformInfo, - extensionPath, - undefined as any + extensionPath ); server = await MockHttpsServer.CreateMockHttpsServer(); testZip = await TestZip.createTestZipAsync(createTestFile('Foo', 'foo.txt')); diff --git a/test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts b/test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts index 586012ca8e..78f882771e 100644 --- a/test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts +++ b/test/omnisharp/omnisharpUnitTests/omnisharpManager.test.ts @@ -245,8 +245,7 @@ function GetTestOmniSharpManager( eventStream, testPackageJSON, platformInfo, - extensionPath, - undefined as any + extensionPath ); return new OmnisharpManager(downloader, platformInfo, serverUrl); } diff --git a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts index 33647e8f90..9be17a0f4b 100644 --- a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts +++ b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts @@ -87,8 +87,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); for (const elem of testZip.files) { const filePath = path.join(tmpDirPath, elem.path); @@ -101,8 +100,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); expect(await util.fileExists(path.join(tmpDirPath, 'install.Lock'))).toBe(true); }); @@ -121,8 +119,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); expect(eventBus.getEvents()).toStrictEqual(eventsSequence); }); @@ -156,8 +153,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); expect(eventBus.getEvents()).toStrictEqual(eventsSequence); }); @@ -184,8 +180,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { downloadablePackage, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); expect(eventBus.getEvents()).toStrictEqual(eventsSequence); }); @@ -201,8 +196,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { notDownloadablePackage, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); const obtainedEvents = eventBus.getEvents(); expect(obtainedEvents[0]).toStrictEqual(eventsSequence[0]); @@ -218,8 +212,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { notDownloadablePackage, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); expect(await util.fileExists(path.join(tmpDirPath, 'install.Lock'))).toBe(false); }); @@ -240,8 +233,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { optionalPackage, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); expect(result).toBe(true); }); @@ -266,8 +258,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { mixedPackages, networkSettingsProvider, eventStream, - downloadValidator, - undefined + downloadValidator ); expect(result).toBe(true); expect(await util.fileExists(path.join(tmpInstallDir2.name, 'install.Lock'))).toBe(true); @@ -285,13 +276,7 @@ describe(`${downloadAndInstallPackages.name}`, () => { ]; eventBus.getEvents(); // Clear any previous events - await downloadAndInstallPackages( - optionalPackage, - networkSettingsProvider, - eventStream, - downloadValidator, - undefined - ); + await downloadAndInstallPackages(optionalPackage, networkSettingsProvider, eventStream, downloadValidator); const obtainedEvents = eventBus.getEvents(); const installationFailureEvent = obtainedEvents.find( (event) => event instanceof InstallationFailure From bf75f6efb50fef296f72ad44738958c3caabb62d Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 15 Oct 2025 15:03:02 -0700 Subject: [PATCH 10/18] Mark the roslynCopilot dependency as optional --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index a08d4cc233..a2d842f480 100644 --- a/package.json +++ b/package.json @@ -414,7 +414,8 @@ "neutral" ], "installTestPath": "./.roslynCopilot/Microsoft.VisualStudio.Copilot.Roslyn.LanguageServer.dll", - "integrity": "0BD733B23A706226F2B429AB0CE35576FB474493C8E19C7B88311007876DE5CC" + "integrity": "0BD733B23A706226F2B429AB0CE35576FB474493C8E19C7B88311007876DE5CC", + "isOptional": true }, { "id": "Debugger", From bd3a4985de4b3f7fdc7d2ade8053bdfbe487f84c Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 15 Oct 2025 15:20:18 -0700 Subject: [PATCH 11/18] PR feedback --- package.json | 3 +- .../extensions/builtInComponents.ts | 2 - src/packageManager/IPackage.ts | 1 - src/packageManager/absolutePathPackage.ts | 6 +- .../downloadAndInstallPackages.ts | 55 +++++++-------- .../downloadAndInstallPackages.test.ts | 68 ------------------- 6 files changed, 28 insertions(+), 107 deletions(-) diff --git a/package.json b/package.json index a2d842f480..a08d4cc233 100644 --- a/package.json +++ b/package.json @@ -414,8 +414,7 @@ "neutral" ], "installTestPath": "./.roslynCopilot/Microsoft.VisualStudio.Copilot.Roslyn.LanguageServer.dll", - "integrity": "0BD733B23A706226F2B429AB0CE35576FB474493C8E19C7B88311007876DE5CC", - "isOptional": true + "integrity": "0BD733B23A706226F2B429AB0CE35576FB474493C8E19C7B88311007876DE5CC" }, { "id": "Debugger", diff --git a/src/lsptoolshost/extensions/builtInComponents.ts b/src/lsptoolshost/extensions/builtInComponents.ts index 119aca8bd7..9574df5556 100644 --- a/src/lsptoolshost/extensions/builtInComponents.ts +++ b/src/lsptoolshost/extensions/builtInComponents.ts @@ -27,13 +27,11 @@ export const componentInfo: { [key: string]: ComponentInfo } = { 'Microsoft.VisualStudio.DesignTools.CodeAnalysis.dll', 'Microsoft.VisualStudio.DesignTools.CodeAnalysis.Diagnostics.dll', ], - isOptional: true, }, razorDevKit: { defaultFolderName: '.razorDevKit', optionName: 'razorDevKit', componentDllPaths: ['Microsoft.VisualStudio.DevKit.Razor.dll'], - isOptional: true, }, razorExtension: { defaultFolderName: '.razorExtension', diff --git a/src/packageManager/IPackage.ts b/src/packageManager/IPackage.ts index 2ede628b38..91d0d826dc 100644 --- a/src/packageManager/IPackage.ts +++ b/src/packageManager/IPackage.ts @@ -13,5 +13,4 @@ export interface IPackage { platformId?: string; integrity?: string; isFramework?: boolean; - isOptional?: boolean; } diff --git a/src/packageManager/absolutePathPackage.ts b/src/packageManager/absolutePathPackage.ts index 578ff2d1e3..5505dc43eb 100644 --- a/src/packageManager/absolutePathPackage.ts +++ b/src/packageManager/absolutePathPackage.ts @@ -20,8 +20,7 @@ export class AbsolutePathPackage implements IPackage { public fallbackUrl?: string, public platformId?: string, public integrity?: string, - public isFramework?: boolean, - public isOptional?: boolean + public isFramework?: boolean ) {} public static getAbsolutePathPackage(pkg: Package, extensionPath: string) { @@ -37,8 +36,7 @@ export class AbsolutePathPackage implements IPackage { pkg.fallbackUrl, pkg.platformId, pkg.integrity, - pkg.isFramework, - pkg.isOptional + pkg.isFramework ); } } diff --git a/src/packageManager/downloadAndInstallPackages.ts b/src/packageManager/downloadAndInstallPackages.ts index 04aa4811d9..f213d961cb 100644 --- a/src/packageManager/downloadAndInstallPackages.ts +++ b/src/packageManager/downloadAndInstallPackages.ts @@ -26,29 +26,7 @@ export async function downloadAndInstallPackages( telemetryReporter?: ITelemetryReporter, token?: CancellationToken ): Promise { - function sendInstallationFailureTelemetry(pkg: AbsolutePathPackage, installationStage: string, error: any): void { - if (!telemetryReporter) { - return; - } - - const telemetryProperties: { [key: string]: string } = { - installStage: installationStage, - packageId: pkg.id, - isOptional: pkg.isOptional ? 'true' : 'false', - }; - - if (error instanceof NestedError && error.err instanceof PackageError) { - telemetryProperties['error.message'] = error.err.message; - telemetryProperties['error.packageUrl'] = error.err.pkg.url; - } else if (error instanceof PackageError) { - telemetryProperties['error.message'] = error.message; - telemetryProperties['error.packageUrl'] = error.pkg.url; - } - - const eventName = pkg.isOptional ? 'OptionalPackageInstallationFailed' : 'PackageInstallationFailed'; - telemetryReporter.sendTelemetryEvent(eventName, telemetryProperties); - } - + let downloadFailed = false; eventStream.post(new PackageInstallStart()); for (const pkg of packages) { let installationStage = 'touchBeginFile'; @@ -89,12 +67,7 @@ export async function downloadAndInstallPackages( // Send telemetry for the failure sendInstallationFailureTelemetry(pkg, installationStage, error); - // If the package is optional, log and continue with the next package - if (pkg.isOptional) { - continue; - } - - return false; + downloadFailed = true; } finally { try { if (await installFileExists(pkg.installPath, InstallFileType.Begin)) { @@ -106,5 +79,27 @@ export async function downloadAndInstallPackages( } } - return true; + return !downloadFailed; + + function sendInstallationFailureTelemetry(pkg: AbsolutePathPackage, installationStage: string, error: any): void { + if (!telemetryReporter) { + return; + } + + const telemetryProperties: { [key: string]: string } = { + installStage: installationStage, + packageId: pkg.id, + isOptional: pkg.isOptional ? 'true' : 'false', + }; + + if (error instanceof NestedError && error.err instanceof PackageError) { + telemetryProperties['error.message'] = error.err.message; + telemetryProperties['error.packageUrl'] = error.err.pkg.url; + } else if (error instanceof PackageError) { + telemetryProperties['error.message'] = error.message; + telemetryProperties['error.packageUrl'] = error.pkg.url; + } + + telemetryReporter.sendTelemetryEvent('PackageInstallationFailed', telemetryProperties); + } } diff --git a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts index 9be17a0f4b..9d8854a2bf 100644 --- a/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts +++ b/test/omnisharp/omnisharpUnitTests/packages/downloadAndInstallPackages.test.ts @@ -218,74 +218,6 @@ describe(`${downloadAndInstallPackages.name}`, () => { }); }); - describe('Optional packages', () => { - test('Returns true and continues when an optional package fails to download', async () => { - const optionalPackage = [ - { - url: `${server.baseUrl}/notDownloadablePackage`, - description: 'Optional Package', - installPath: new AbsolutePath(tmpDirPath), - isOptional: true, - }, - ]; - - const result = await downloadAndInstallPackages( - optionalPackage, - networkSettingsProvider, - eventStream, - downloadValidator - ); - expect(result).toBe(true); - }); - - test('Continues to install remaining packages after optional package fails', async () => { - const tmpInstallDir2 = await CreateTmpDir(true); - const mixedPackages = [ - { - url: `${server.baseUrl}/notDownloadablePackage`, - description: 'Optional Package', - installPath: new AbsolutePath(tmpDirPath), - isOptional: true, - }, - { - url: `${server.baseUrl}/downloadablePackage`, - description: packageDescription, - installPath: new AbsolutePath(tmpInstallDir2.name), - }, - ]; - - const result = await downloadAndInstallPackages( - mixedPackages, - networkSettingsProvider, - eventStream, - downloadValidator - ); - expect(result).toBe(true); - expect(await util.fileExists(path.join(tmpInstallDir2.name, 'install.Lock'))).toBe(true); - tmpInstallDir2.dispose(); - }); - - test('InstallationFailure event is logged for optional package', async () => { - const optionalPackage = [ - { - url: `${server.baseUrl}/notDownloadablePackage`, - description: 'Optional Package', - installPath: new AbsolutePath(tmpDirPath), - isOptional: true, - }, - ]; - - eventBus.getEvents(); // Clear any previous events - await downloadAndInstallPackages(optionalPackage, networkSettingsProvider, eventStream, downloadValidator); - const obtainedEvents = eventBus.getEvents(); - const installationFailureEvent = obtainedEvents.find( - (event) => event instanceof InstallationFailure - ) as InstallationFailure; - expect(installationFailureEvent).toBeDefined(); - expect(installationFailureEvent.stage).toEqual('downloadPackage'); - }); - }); - afterEach(async () => { if (tmpInstallDir) { tmpInstallDir.dispose(); From b4b22c11fc806610ceb2921c5effbfa145ee1b53 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Oct 2025 22:40:07 +0000 Subject: [PATCH 12/18] Add logging for missing optional components and restore isOptional property Co-authored-by: JoeRobich <611219+JoeRobich@users.noreply.github.com> --- .../extensions/builtInComponents.ts | 11 ++++++++-- .../server/roslynLanguageServer.ts | 20 +++++++++++-------- src/packageManager/IPackage.ts | 1 + src/packageManager/absolutePathPackage.ts | 6 ++++-- src/razor/src/extension.ts | 2 +- 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/src/lsptoolshost/extensions/builtInComponents.ts b/src/lsptoolshost/extensions/builtInComponents.ts index 9574df5556..d4120c1c80 100644 --- a/src/lsptoolshost/extensions/builtInComponents.ts +++ b/src/lsptoolshost/extensions/builtInComponents.ts @@ -46,14 +46,21 @@ export const componentInfo: { [key: string]: ComponentInfo } = { }, }; -export function getComponentPaths(componentName: string, options: LanguageServerOptions | undefined): string[] { +export function getComponentPaths( + componentName: string, + options: LanguageServerOptions | undefined, + channel?: { warn: (message: string) => void } +): string[] { const component = componentInfo[componentName]; const baseFolder = getComponentFolderPath(component, options); const paths = component.componentDllPaths.map((dllPath) => path.join(baseFolder, dllPath)); for (const dllPath of paths) { if (!fs.existsSync(dllPath)) { if (component.isOptional) { - // Component is optional and doesn't exist - return empty array + // Component is optional and doesn't exist - log warning and return empty array + if (channel) { + channel.warn(`Optional component '${componentName}' could not be found at '${dllPath}'.`); + } return []; } throw new Error(`Component DLL not found: ${dllPath}`); diff --git a/src/lsptoolshost/server/roslynLanguageServer.ts b/src/lsptoolshost/server/roslynLanguageServer.ts index 8d1951d1f4..6ddc1ccadc 100644 --- a/src/lsptoolshost/server/roslynLanguageServer.ts +++ b/src/lsptoolshost/server/roslynLanguageServer.ts @@ -651,7 +651,7 @@ export class RoslynLanguageServer { : razorOptions.razorServerPath; let razorComponentPath = ''; - getComponentPaths('razorExtension', languageServerOptions).forEach((extPath) => { + getComponentPaths('razorExtension', languageServerOptions, channel).forEach((extPath) => { additionalExtensionPaths.push(extPath); razorComponentPath = path.dirname(extPath); }); @@ -695,10 +695,10 @@ export class RoslynLanguageServer { // Set command enablement as soon as we know devkit is available. await vscode.commands.executeCommand('setContext', 'dotnet.server.activationContext', 'RoslynDevKit'); - const csharpDevKitArgs = this.getCSharpDevKitExportArgs(additionalExtensionPaths); + const csharpDevKitArgs = this.getCSharpDevKitExportArgs(additionalExtensionPaths, channel); args = args.concat(csharpDevKitArgs); - await this.setupDevKitEnvironment(dotnetInfo.env, csharpDevkitExtension, additionalExtensionPaths); + await this.setupDevKitEnvironment(dotnetInfo.env, csharpDevkitExtension, additionalExtensionPaths, channel); } else { // C# Dev Kit is not installed - continue C#-only activation. channel.info('Activating C# standalone...'); @@ -1012,10 +1012,13 @@ export class RoslynLanguageServer { ); } - private static getCSharpDevKitExportArgs(additionalExtensionPaths: string[]): string[] { + private static getCSharpDevKitExportArgs( + additionalExtensionPaths: string[], + channel: vscode.LogOutputChannel + ): string[] { const args: string[] = []; - const devKitDepsPath = getComponentPaths('roslynDevKit', languageServerOptions); + const devKitDepsPath = getComponentPaths('roslynDevKit', languageServerOptions, channel); if (devKitDepsPath.length > 1) { throw new Error('Expected only one devkit deps path'); } @@ -1026,7 +1029,7 @@ export class RoslynLanguageServer { // Also include the Xaml Dev Kit extensions, if enabled. if (languageServerOptions.enableXamlTools) { - getComponentPaths('xamlTools', languageServerOptions).forEach((path) => + getComponentPaths('xamlTools', languageServerOptions, channel).forEach((path) => additionalExtensionPaths.push(path) ); } @@ -1086,7 +1089,8 @@ export class RoslynLanguageServer { private static async setupDevKitEnvironment( env: NodeJS.ProcessEnv, csharpDevkitExtension: vscode.Extension, - additionalExtensionPaths: string[] + additionalExtensionPaths: string[], + channel: vscode.LogOutputChannel ): Promise { const exports: CSharpDevKitExports = await csharpDevkitExtension.activate(); @@ -1096,7 +1100,7 @@ export class RoslynLanguageServer { await exports.setupTelemetryEnvironmentAsync(env); } - getComponentPaths('roslynCopilot', languageServerOptions).forEach((extPath) => { + getComponentPaths('roslynCopilot', languageServerOptions, channel).forEach((extPath) => { additionalExtensionPaths.push(extPath); }); } diff --git a/src/packageManager/IPackage.ts b/src/packageManager/IPackage.ts index 91d0d826dc..2ede628b38 100644 --- a/src/packageManager/IPackage.ts +++ b/src/packageManager/IPackage.ts @@ -13,4 +13,5 @@ export interface IPackage { platformId?: string; integrity?: string; isFramework?: boolean; + isOptional?: boolean; } diff --git a/src/packageManager/absolutePathPackage.ts b/src/packageManager/absolutePathPackage.ts index 5505dc43eb..578ff2d1e3 100644 --- a/src/packageManager/absolutePathPackage.ts +++ b/src/packageManager/absolutePathPackage.ts @@ -20,7 +20,8 @@ export class AbsolutePathPackage implements IPackage { public fallbackUrl?: string, public platformId?: string, public integrity?: string, - public isFramework?: boolean + public isFramework?: boolean, + public isOptional?: boolean ) {} public static getAbsolutePathPackage(pkg: Package, extensionPath: string) { @@ -36,7 +37,8 @@ export class AbsolutePathPackage implements IPackage { pkg.fallbackUrl, pkg.platformId, pkg.integrity, - pkg.isFramework + pkg.isFramework, + pkg.isOptional ); } } diff --git a/src/razor/src/extension.ts b/src/razor/src/extension.ts index 8fdac6c67b..a2555ac0b7 100644 --- a/src/razor/src/extension.ts +++ b/src/razor/src/extension.ts @@ -104,7 +104,7 @@ export async function activate( await setupDevKitEnvironment(dotnetInfo.env, csharpDevkitExtension, logger); if (vscode.env.isTelemetryEnabled) { - const razorComponentPaths = getComponentPaths('razorDevKit', undefined); + const razorComponentPaths = getComponentPaths('razorDevKit', undefined, logger.outputChannel); if (razorComponentPaths.length !== 1) { logger.logError('Failed to find Razor DevKit telemetry extension path.', undefined); } else { From 5f0208d57dadcc6a8efafff57945cf036d4f3425 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 15 Oct 2025 15:45:28 -0700 Subject: [PATCH 13/18] Remove isOptional from IPackage --- src/lsptoolshost/extensions/builtInComponents.ts | 3 ++- src/packageManager/IPackage.ts | 1 - src/packageManager/absolutePathPackage.ts | 6 ++---- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/lsptoolshost/extensions/builtInComponents.ts b/src/lsptoolshost/extensions/builtInComponents.ts index d4120c1c80..4f67c80e28 100644 --- a/src/lsptoolshost/extensions/builtInComponents.ts +++ b/src/lsptoolshost/extensions/builtInComponents.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import * as vscode from 'vscode'; import * as fs from 'fs'; import * as path from 'path'; import { LanguageServerOptions } from '../../shared/options'; @@ -49,7 +50,7 @@ export const componentInfo: { [key: string]: ComponentInfo } = { export function getComponentPaths( componentName: string, options: LanguageServerOptions | undefined, - channel?: { warn: (message: string) => void } + channel?: vscode.LogOutputChannel ): string[] { const component = componentInfo[componentName]; const baseFolder = getComponentFolderPath(component, options); diff --git a/src/packageManager/IPackage.ts b/src/packageManager/IPackage.ts index 2ede628b38..91d0d826dc 100644 --- a/src/packageManager/IPackage.ts +++ b/src/packageManager/IPackage.ts @@ -13,5 +13,4 @@ export interface IPackage { platformId?: string; integrity?: string; isFramework?: boolean; - isOptional?: boolean; } diff --git a/src/packageManager/absolutePathPackage.ts b/src/packageManager/absolutePathPackage.ts index 578ff2d1e3..5505dc43eb 100644 --- a/src/packageManager/absolutePathPackage.ts +++ b/src/packageManager/absolutePathPackage.ts @@ -20,8 +20,7 @@ export class AbsolutePathPackage implements IPackage { public fallbackUrl?: string, public platformId?: string, public integrity?: string, - public isFramework?: boolean, - public isOptional?: boolean + public isFramework?: boolean ) {} public static getAbsolutePathPackage(pkg: Package, extensionPath: string) { @@ -37,8 +36,7 @@ export class AbsolutePathPackage implements IPackage { pkg.fallbackUrl, pkg.platformId, pkg.integrity, - pkg.isFramework, - pkg.isOptional + pkg.isFramework ); } } From 74c80730aa1b2f3c48bb11e436d70679cd171b66 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 15 Oct 2025 15:51:05 -0700 Subject: [PATCH 14/18] Remove isOptional from IPackage --- src/packageManager/downloadAndInstallPackages.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/packageManager/downloadAndInstallPackages.ts b/src/packageManager/downloadAndInstallPackages.ts index f213d961cb..327ae91359 100644 --- a/src/packageManager/downloadAndInstallPackages.ts +++ b/src/packageManager/downloadAndInstallPackages.ts @@ -89,7 +89,6 @@ export async function downloadAndInstallPackages( const telemetryProperties: { [key: string]: string } = { installStage: installationStage, packageId: pkg.id, - isOptional: pkg.isOptional ? 'true' : 'false', }; if (error instanceof NestedError && error.err instanceof PackageError) { From 504f41284e9a51afcd3236e180b193608f413200 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 15 Oct 2025 23:48:23 +0000 Subject: [PATCH 15/18] Report more granular dependency installation results --- src/installRuntimeDependencies.ts | 19 ++++++++++++------- src/main.ts | 2 +- src/packageManager/IInstallDependencies.ts | 6 ++++-- .../downloadAndInstallPackages.ts | 13 ++++++++----- .../installRuntimeDependencies.test.ts | 12 +++++++----- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/installRuntimeDependencies.ts b/src/installRuntimeDependencies.ts index 7218d6c931..771cf8dff2 100644 --- a/src/installRuntimeDependencies.ts +++ b/src/installRuntimeDependencies.ts @@ -8,7 +8,7 @@ import { PackageInstallation, LogPlatformInfo, InstallationSuccess } from './sha import { EventStream } from './eventStream'; import { getRuntimeDependenciesPackages } from './tools/runtimeDependencyPackageUtils'; import { getAbsolutePathPackagesToInstall } from './packageManager/getAbsolutePathPackagesToInstall'; -import IInstallDependencies from './packageManager/IInstallDependencies'; +import { DependencyInstallationResults, IInstallDependencies } from './packageManager/IInstallDependencies'; import { AbsolutePathPackage } from './packageManager/absolutePathPackage'; export async function installRuntimeDependencies( @@ -19,7 +19,7 @@ export async function installRuntimeDependencies( platformInfo: PlatformInformation, useFramework: boolean, requiredPackageIds: string[] -): Promise { +): Promise { const runTimeDependencies = getRuntimeDependenciesPackages(packageJSON); const packagesToInstall = await getAbsolutePathPackagesToInstall(runTimeDependencies, platformInfo, extensionPath); const filteredPackages = filterOmniSharpPackage(packagesToInstall, useFramework); @@ -30,15 +30,20 @@ export async function installRuntimeDependencies( // Display platform information and RID eventStream.post(new LogPlatformInfo(platformInfo)); - if (await installDependencies(filteredRequiredPackages)) { + const installationResults = await installDependencies(filteredRequiredPackages); + + const failedPackages = Object.entries(installationResults) + .filter(([, installed]) => !installed) + .map(([name]) => name); + if (failedPackages.length === 0) { eventStream.post(new InstallationSuccess()); - } else { - return false; } + + return installationResults; } - //All the required packages are already downloaded and installed - return true; + // There was nothing to install + return {}; } function filterOmniSharpPackage(packages: AbsolutePathPackage[], useFramework: boolean) { diff --git a/src/main.ts b/src/main.ts index b001f2eb35..6c1a80cdda 100644 --- a/src/main.ts +++ b/src/main.ts @@ -119,7 +119,7 @@ export async function activate( } else { const getCoreClrDebugPromise = async (languageServerStartedPromise: Promise) => { let coreClrDebugPromise = Promise.resolve(); - if (runtimeDependenciesExist) { + if (runtimeDependenciesExist['Debugger']) { // activate coreclr-debug coreClrDebugPromise = coreclrdebug.activate( context.extension, diff --git a/src/packageManager/IInstallDependencies.ts b/src/packageManager/IInstallDependencies.ts index 6ca5a476f1..cfaf2191a7 100644 --- a/src/packageManager/IInstallDependencies.ts +++ b/src/packageManager/IInstallDependencies.ts @@ -5,6 +5,8 @@ import { AbsolutePathPackage } from './absolutePathPackage'; -export default interface IInstallDependencies { - (packages: AbsolutePathPackage[]): Promise; +export type DependencyInstallationResults = { [name: string]: boolean }; + +export interface IInstallDependencies { + (packages: AbsolutePathPackage[]): Promise; } diff --git a/src/packageManager/downloadAndInstallPackages.ts b/src/packageManager/downloadAndInstallPackages.ts index 327ae91359..b0e747a6b8 100644 --- a/src/packageManager/downloadAndInstallPackages.ts +++ b/src/packageManager/downloadAndInstallPackages.ts @@ -17,6 +17,7 @@ import { PackageInstallStart } from '../shared/loggingEvents'; import { DownloadValidator } from './isValidDownload'; import { CancellationToken } from 'vscode'; import { ITelemetryReporter } from '../shared/telemetryReporter'; +import { DependencyInstallationResults } from './IInstallDependencies'; export async function downloadAndInstallPackages( packages: AbsolutePathPackage[], @@ -25,9 +26,9 @@ export async function downloadAndInstallPackages( downloadValidator: DownloadValidator, telemetryReporter?: ITelemetryReporter, token?: CancellationToken -): Promise { - let downloadFailed = false; +): Promise { eventStream.post(new PackageInstallStart()); + const results: DependencyInstallationResults = {}; for (const pkg of packages) { let installationStage = 'touchBeginFile'; try { @@ -51,12 +52,16 @@ export async function downloadAndInstallPackages( await InstallZip(buffer, pkg.description, pkg.installPath, pkg.binaries, eventStream); installationStage = 'touchLockFile'; await touchInstallFile(pkg.installPath, InstallFileType.Lock); + results[pkg.id] = true; break; } else { eventStream.post(new IntegrityCheckFailure(pkg.description, pkg.url, willTryInstallingPackage())); + results[pkg.id] = false; } } } catch (error) { + results[pkg.id] = false; + if (error instanceof NestedError) { const packageError = new PackageError(error.message, pkg, error.err); eventStream.post(new InstallationFailure(installationStage, packageError)); @@ -66,8 +71,6 @@ export async function downloadAndInstallPackages( // Send telemetry for the failure sendInstallationFailureTelemetry(pkg, installationStage, error); - - downloadFailed = true; } finally { try { if (await installFileExists(pkg.installPath, InstallFileType.Begin)) { @@ -79,7 +82,7 @@ export async function downloadAndInstallPackages( } } - return !downloadFailed; + return results; function sendInstallationFailureTelemetry(pkg: AbsolutePathPackage, installationStage: string, error: any): void { if (!telemetryReporter) { diff --git a/test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts b/test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts index e1a3b89f70..08eaf30900 100644 --- a/test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts +++ b/test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts @@ -5,7 +5,7 @@ import { describe, test, expect, beforeEach } from '@jest/globals'; import { installRuntimeDependencies } from '../../../src/installRuntimeDependencies'; -import IInstallDependencies from '../../../src/packageManager/IInstallDependencies'; +import { IInstallDependencies } from '../../../src/packageManager/IInstallDependencies'; import { EventStream } from '../../../src/eventStream'; import { PlatformInformation } from '../../../src/shared/platform'; import TestEventBus from './testAssets/testEventBus'; @@ -28,7 +28,8 @@ describe(`${installRuntimeDependencies.name}`, () => { beforeEach(() => { eventStream = new EventStream(); eventBus = new TestEventBus(eventStream); - installDependencies = async () => Promise.resolve(true); + installDependencies = async (packages) => + Promise.resolve(packages.reduce((acc, pkg) => ({ ...acc, [pkg.id]: true }), {})); }); describe('When all the dependencies already exist', () => { @@ -90,7 +91,7 @@ describe(`${installRuntimeDependencies.name}`, () => { let inputPackage: AbsolutePathPackage[]; installDependencies = async (packages) => { inputPackage = packages; - return Promise.resolve(true); + return Promise.resolve(packages.reduce((acc, pkg) => ({ ...acc, [pkg.id]: true }), {})); }; const installed = await installRuntimeDependencies( @@ -111,7 +112,8 @@ describe(`${installRuntimeDependencies.name}`, () => { }); test('Returns false when installDependencies returns false', async () => { - installDependencies = async () => Promise.resolve(false); + installDependencies = async (packages) => + Promise.resolve(packages.reduce((acc, pkg) => ({ ...acc, [pkg.id]: false }), {})); const installed = await installRuntimeDependencies( packageJSON, extensionPath, @@ -121,7 +123,7 @@ describe(`${installRuntimeDependencies.name}`, () => { useFramework, ['myPackage'] ); - expect(installed).toBe(false); + expect(installed['myPackage']).toBe(false); }); }); }); From 2f7cebe53634fa94345af21eb9dc6dac6d95b8c0 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Wed, 15 Oct 2025 23:52:28 +0000 Subject: [PATCH 16/18] Fix up tests --- .../omnisharpUnitTests/installRuntimeDependencies.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts b/test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts index 08eaf30900..0ce27b8f82 100644 --- a/test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts +++ b/test/omnisharp/omnisharpUnitTests/installRuntimeDependencies.test.ts @@ -49,7 +49,9 @@ describe(`${installRuntimeDependencies.name}`, () => { useFramework, ['Debugger', 'Omnisharp', 'Razor'] ); - expect(installed).toBe(true); + expect(installed['Debugger']).toBe(true); + expect(installed['Omnisharp']).toBe(true); + expect(installed['Razor']).toBe(true); }); test("Doesn't log anything to the eventStream", async () => { @@ -103,7 +105,7 @@ describe(`${installRuntimeDependencies.name}`, () => { useFramework, ['myPackage'] ); - expect(installed).toBe(true); + expect(installed['myPackage']).toBe(true); isNotNull(inputPackage!); expect(inputPackage).toHaveLength(1); expect(inputPackage[0]).toStrictEqual( From 6f63cf6e8c84bc8fba8a35b870032bf2c3fa8a50 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Thu, 16 Oct 2025 00:01:20 +0000 Subject: [PATCH 17/18] Fixup remaining usage of downloadAndInstallPackages --- src/main.ts | 2 +- src/omnisharp/omnisharpDownloader.ts | 20 +++++++++++--------- src/razor/razorOmnisharpDownloader.ts | 20 +++++++++++--------- tasks/offlinePackagingTasks.ts | 17 +++++++++++++---- 4 files changed, 36 insertions(+), 23 deletions(-) diff --git a/src/main.ts b/src/main.ts index 6c1a80cdda..434e2e9c52 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,7 +17,7 @@ import { vscodeNetworkSettingsProvider } from './networkSettings'; import createOptionStream from './shared/observables/createOptionStream'; import { AbsolutePathPackage } from './packageManager/absolutePathPackage'; import { downloadAndInstallPackages } from './packageManager/downloadAndInstallPackages'; -import IInstallDependencies from './packageManager/IInstallDependencies'; +import { IInstallDependencies } from './packageManager/IInstallDependencies'; import { installRuntimeDependencies } from './installRuntimeDependencies'; import { isValidDownload } from './packageManager/isValidDownload'; import { MigrateOptions } from './shared/migrateOptions'; diff --git a/src/omnisharp/omnisharpDownloader.ts b/src/omnisharp/omnisharpDownloader.ts index 8b3ad510ea..dd0ea5bab0 100644 --- a/src/omnisharp/omnisharpDownloader.ts +++ b/src/omnisharp/omnisharpDownloader.ts @@ -53,15 +53,17 @@ export class OmnisharpDownloader { if (packagesToInstall.length > 0) { this.eventStream.post(new PackageInstallation(`OmniSharp Version = ${version}`)); this.eventStream.post(new LogPlatformInfo(this.platformInfo)); - if ( - await downloadAndInstallPackages( - packagesToInstall, - this.networkSettingsProvider, - this.eventStream, - isValidDownload, - this.reporter - ) - ) { + const installationResults = await downloadAndInstallPackages( + packagesToInstall, + this.networkSettingsProvider, + this.eventStream, + isValidDownload, + this.reporter + ); + const failedPackages = Object.entries(installationResults) + .filter(([, installed]) => !installed) + .map(([name]) => name); + if (failedPackages.length === 0) { this.eventStream.post(new InstallationSuccess()); return true; } diff --git a/src/razor/razorOmnisharpDownloader.ts b/src/razor/razorOmnisharpDownloader.ts index ffd433ba81..83ffd4edc5 100644 --- a/src/razor/razorOmnisharpDownloader.ts +++ b/src/razor/razorOmnisharpDownloader.ts @@ -35,15 +35,17 @@ export class RazorOmnisharpDownloader { if (packagesToInstall.length > 0) { this.eventStream.post(new PackageInstallation(`Razor OmniSharp Version = ${version}`)); this.eventStream.post(new LogPlatformInfo(this.platformInfo)); - if ( - await downloadAndInstallPackages( - packagesToInstall, - this.networkSettingsProvider, - this.eventStream, - isValidDownload, - this.reporter - ) - ) { + const installationResults = await downloadAndInstallPackages( + packagesToInstall, + this.networkSettingsProvider, + this.eventStream, + isValidDownload, + this.reporter + ); + const failedPackages = Object.entries(installationResults) + .filter(([, installed]) => !installed) + .map(([name]) => name); + if (failedPackages.length === 0) { this.eventStream.post(new InstallationSuccess()); return true; } diff --git a/tasks/offlinePackagingTasks.ts b/tasks/offlinePackagingTasks.ts index fb9714b4c9..af16c0e1a0 100644 --- a/tasks/offlinePackagingTasks.ts +++ b/tasks/offlinePackagingTasks.ts @@ -280,10 +280,19 @@ async function installPackageJsonDependency( codeExtensionPath ); const provider = () => new NetworkSettings('', true); - if ( - !(await downloadAndInstallPackages(packagesToInstall, provider, eventStream, isValidDownload, undefined, token)) - ) { - throw Error('Failed to download package.'); + const installationResults = await downloadAndInstallPackages( + packagesToInstall, + provider, + eventStream, + isValidDownload, + undefined, + token + ); + const failedPackages = Object.entries(installationResults) + .filter(([, installed]) => !installed) + .map(([name]) => name); + if (failedPackages.length > 0) { + throw Error('The following packages failed to install: ' + failedPackages.join(', ')); } } From ef03174d81e5a34b67a6fb84165df4db0d84cb10 Mon Sep 17 00:00:00 2001 From: Joey Robichaud Date: Thu, 16 Oct 2025 13:26:00 +0000 Subject: [PATCH 18/18] Return installation status for already installed dependencies --- src/installRuntimeDependencies.ts | 40 +++++++++++-------- src/packageManager/IInstallDependencies.ts | 4 +- .../downloadAndInstallPackages.ts | 6 +-- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/installRuntimeDependencies.ts b/src/installRuntimeDependencies.ts index 771cf8dff2..3cdab86cd2 100644 --- a/src/installRuntimeDependencies.ts +++ b/src/installRuntimeDependencies.ts @@ -8,7 +8,7 @@ import { PackageInstallation, LogPlatformInfo, InstallationSuccess } from './sha import { EventStream } from './eventStream'; import { getRuntimeDependenciesPackages } from './tools/runtimeDependencyPackageUtils'; import { getAbsolutePathPackagesToInstall } from './packageManager/getAbsolutePathPackagesToInstall'; -import { DependencyInstallationResults, IInstallDependencies } from './packageManager/IInstallDependencies'; +import { DependencyInstallationStatus, IInstallDependencies } from './packageManager/IInstallDependencies'; import { AbsolutePathPackage } from './packageManager/absolutePathPackage'; export async function installRuntimeDependencies( @@ -19,31 +19,39 @@ export async function installRuntimeDependencies( platformInfo: PlatformInformation, useFramework: boolean, requiredPackageIds: string[] -): Promise { +): Promise { const runTimeDependencies = getRuntimeDependenciesPackages(packageJSON); const packagesToInstall = await getAbsolutePathPackagesToInstall(runTimeDependencies, platformInfo, extensionPath); + + // PackagesToInstall will only return packages that are not already installed. However, + // we need to return the installation status of all required packages, so we need to + // track which required packages are already installed, so that we can return true for them. + const installedPackages = requiredPackageIds.filter( + (id) => packagesToInstall.find((pkg) => pkg.id === id) === undefined + ); + const installedPackagesResults = installedPackages.reduce((acc, id) => ({ ...acc, [id]: true }), {}); + const filteredPackages = filterOmniSharpPackage(packagesToInstall, useFramework); const filteredRequiredPackages = filteredRequiredPackage(requiredPackageIds, filteredPackages); - if (filteredRequiredPackages.length > 0) { - eventStream.post(new PackageInstallation('C# dependencies')); - // Display platform information and RID - eventStream.post(new LogPlatformInfo(platformInfo)); + if (filteredRequiredPackages.length === 0) { + return installedPackagesResults; + } - const installationResults = await installDependencies(filteredRequiredPackages); + eventStream.post(new PackageInstallation('C# dependencies')); + // Display platform information and RID + eventStream.post(new LogPlatformInfo(platformInfo)); - const failedPackages = Object.entries(installationResults) - .filter(([, installed]) => !installed) - .map(([name]) => name); - if (failedPackages.length === 0) { - eventStream.post(new InstallationSuccess()); - } + const installationResults = await installDependencies(filteredRequiredPackages); - return installationResults; + const failedPackages = Object.entries(installationResults) + .filter(([, installed]) => !installed) + .map(([name]) => name); + if (failedPackages.length === 0) { + eventStream.post(new InstallationSuccess()); } - // There was nothing to install - return {}; + return { ...installedPackagesResults, ...installationResults }; } function filterOmniSharpPackage(packages: AbsolutePathPackage[], useFramework: boolean) { diff --git a/src/packageManager/IInstallDependencies.ts b/src/packageManager/IInstallDependencies.ts index cfaf2191a7..d4c12b3cfc 100644 --- a/src/packageManager/IInstallDependencies.ts +++ b/src/packageManager/IInstallDependencies.ts @@ -5,8 +5,8 @@ import { AbsolutePathPackage } from './absolutePathPackage'; -export type DependencyInstallationResults = { [name: string]: boolean }; +export type DependencyInstallationStatus = { [name: string]: boolean }; export interface IInstallDependencies { - (packages: AbsolutePathPackage[]): Promise; + (packages: AbsolutePathPackage[]): Promise; } diff --git a/src/packageManager/downloadAndInstallPackages.ts b/src/packageManager/downloadAndInstallPackages.ts index b0e747a6b8..da39e0e352 100644 --- a/src/packageManager/downloadAndInstallPackages.ts +++ b/src/packageManager/downloadAndInstallPackages.ts @@ -17,7 +17,7 @@ import { PackageInstallStart } from '../shared/loggingEvents'; import { DownloadValidator } from './isValidDownload'; import { CancellationToken } from 'vscode'; import { ITelemetryReporter } from '../shared/telemetryReporter'; -import { DependencyInstallationResults } from './IInstallDependencies'; +import { DependencyInstallationStatus } from './IInstallDependencies'; export async function downloadAndInstallPackages( packages: AbsolutePathPackage[], @@ -26,9 +26,9 @@ export async function downloadAndInstallPackages( downloadValidator: DownloadValidator, telemetryReporter?: ITelemetryReporter, token?: CancellationToken -): Promise { +): Promise { eventStream.post(new PackageInstallStart()); - const results: DependencyInstallationResults = {}; + const results: DependencyInstallationStatus = {}; for (const pkg of packages) { let installationStage = 'touchBeginFile'; try {