Skip to content

Commit

Permalink
Improve VS Code SDK Error Diagnosability (#1822)
Browse files Browse the repository at this point in the history 

* Introduce error to wrap all global sdk errors

* Wrap installation in success event to improve trackability

* Expose the original event name for property expansion

* Respond to linter

* Improve Error Message Specificity

* Update the event stream to use the right install key

* Update Errors to be EventBasedErrors that contain the Event Name

This is a mildly ugly pattern. See the PR description for more detail.

* Make legacy code throw an error instead of throwing a string

Throwing other object types up makes it harder for us to get the callstack etc

* Fix bug with install key management where it would not have -global or the architecture would be unspecified

* respond to linter

* Fix whitespace

* Fix whitespace
  • Loading branch information
@nagilson
nagilson authored Jun 10, 2024
1 parent 30beed1 commit 79b9139
Show file tree
Hide file tree
Showing 30 changed files with 327 additions and 159 deletions.
8 changes: 4 additions & 4 deletions vscode-dotnet-runtime-extension/package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 31 additions & 10 deletions vscode-dotnet-runtime-extension/src/extension.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import {
DotnetExistingPathResolutionCompleted,
DotnetRuntimeAcquisitionStarted,
DotnetRuntimeAcquisitionTotalSuccessEvent,
DotnetGlobalSDKAcquisitionTotalSuccessEvent,
enableExtensionTelemetry,
EventBasedError,
ErrorConfiguration,
ExistingPathResolver,
ExtensionConfigurationWorker,
Expand Down Expand Up @@ -166,7 +168,8 @@ export function activate(context: vscode.ExtensionContext, extensionContext?: IE
}

if (!commandContext.version || commandContext.version === 'latest') {
throw new Error(`Cannot acquire .NET version "${commandContext.version}". Please provide a valid version.`);
throw new EventBasedError('BadContextualVersion',
`Cannot acquire .NET version "${commandContext.version}". Please provide a valid version.`);
}

const existingPath = await resolveExistingPathIfExists(existingPathConfigWorker, commandContext);
Expand Down Expand Up @@ -201,8 +204,22 @@ export function activate(context: vscode.ExtensionContext, extensionContext?: IE
return Promise.reject('No requesting extension id was provided.');
}

const pathResult = callWithErrorHandling(async () =>
let fullyResolvedVersion = '';

const pathResult = await callWithErrorHandling(async () =>
{
// Warning: Between now and later in this call-stack, the context 'version' is incomplete as it has not been resolved.
// Errors between here and the place where it is resolved cannot be routed to one another.

sdkAcquisitionWorker.setAcquisitionContext(commandContext);
telemetryObserver?.setAcquisitionContext(sdkContext, commandContext);

if(commandContext.version === '' || !commandContext.version)
{
throw new EventCancellationError('BadContextualRuntimeVersionError',
`No version was defined to install.`);
}

globalEventStream.post(new DotnetSDKAcquisitionStarted(commandContext.requestingExtensionId));
globalEventStream.post(new DotnetAcquisitionRequested(commandContext.version, commandContext.requestingExtensionId));

Expand All @@ -212,22 +229,26 @@ export function activate(context: vscode.ExtensionContext, extensionContext?: IE
return Promise.resolve(existingPath);
}

const globalInstallerResolver = new GlobalInstallerResolver(sdkContext, commandContext.version);
fullyResolvedVersion = await globalInstallerResolver.getFullySpecifiedVersion();

// Reset context to point to the fully specified version so it is not possible for someone to access incorrect data during the install process.
commandContext.version = fullyResolvedVersion;
sdkAcquisitionWorker.setAcquisitionContext(commandContext);
telemetryObserver?.setAcquisitionContext(sdkContext, commandContext);

if(commandContext.version === '' || !commandContext.version)
{
throw Error(`No version was defined to install.`);
}

const globalInstallerResolver = new GlobalInstallerResolver(sdkContext, commandContext.version);
outputChannel.show(true);
const dotnetPath = await sdkAcquisitionWorker.acquireGlobalSDK(globalInstallerResolver);

new CommandExecutor(sdkContext, utilContext).setPathEnvVar(dotnetPath.dotnetPath, moreInfoUrl, displayWorker, vsCodeExtensionContext, true);
return dotnetPath;
}, sdkIssueContextFunctor(commandContext.errorConfiguration, commandKeys.acquireGlobalSDK), commandContext.requestingExtensionId, sdkContext);

const iKey = sdkAcquisitionWorker.getInstallKey(fullyResolvedVersion);
const install = {installKey : iKey, version : fullyResolvedVersion, installMode: 'sdk', isGlobal: true,
architecture: commandContext.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture()} as DotnetInstall;

globalEventStream.post(new DotnetGlobalSDKAcquisitionTotalSuccessEvent(commandContext.version, install, commandContext.requestingExtensionId ?? '', pathResult?.dotnetPath ?? ''));
return pathResult;
});

Expand All @@ -245,8 +266,8 @@ export function activate(context: vscode.ExtensionContext, extensionContext?: IE

if (!activeSupportVersions || activeSupportVersions.length < 1)
{
const err = new Error(`An active-support version of dotnet couldn't be found. Discovered versions: ${JSON.stringify(availableVersions)}`);
globalEventStream.post(new DotnetVersionResolutionError(err as EventCancellationError, null));
const err = new EventCancellationError('DotnetVersionResolutionError', `An active-support version of dotnet couldn't be found. Discovered versions: ${JSON.stringify(availableVersions)}`);
globalEventStream.post(new DotnetVersionResolutionError(err, null));
if(!availableVersions || availableVersions.length < 1)
{
return [];
Expand Down
4 changes: 2 additions & 2 deletions vscode-dotnet-runtime-extension/yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2481,8 +2481,8 @@
"@types/shelljs" "0.8.9"
"@types/vscode" "1.74.0"
"@vscode/sudo-prompt" "^9.3.1"
"axios" "^1.3.4"
"axios-cache-interceptor" "^1.0.1"
"axios" "^1.7.2"
"axios-cache-interceptor" "^1.5.3"
"axios-retry" "^3.4.0"
"chai" "4.3.4"
"chai-as-promised" "^7.1.1"
Expand Down
25 changes: 15 additions & 10 deletions vscode-dotnet-runtime-library/src/Acquisition/AcquisitionInvoker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
DotnetAcquisitionTimeoutError,
DotnetAcquisitionUnexpectedError,
DotnetOfflineFailure,
EventBasedError,
} from '../EventStream/EventStreamEvents';

import { timeoutConstants } from '../Utils/ErrorHandler';
Expand Down Expand Up @@ -100,19 +101,22 @@ You will need to restart VS Code after these changes. If PowerShell is still not
{
if (!(await this.isOnline(installContext)))
{
const offlineError = new Error('No internet connection detected: Cannot install .NET');
const offlineError = new EventBasedError('DotnetOfflineFailure', 'No internet connection detected: Cannot install .NET');
this.eventStream.post(new DotnetOfflineFailure(offlineError, installKey));
reject(offlineError);
}
else if (error.signal === 'SIGKILL') {
error.message = timeoutConstants.timeoutMessage;
const newError = new EventBasedError('DotnetAcquisitionTimeoutError',
`${timeoutConstants.timeoutMessage}, MESSAGE: ${error.message}, CODE: ${error.code}, KILLED: ${error.killed}`, error.stack);
this.eventStream.post(new DotnetAcquisitionTimeoutError(error, installKey, installContext.timeoutSeconds));
reject(error);
reject(newError);
}
else
{
this.eventStream.post(new DotnetAcquisitionInstallError(error, installKey));
reject(error);
const newError = new EventBasedError('DotnetAcquisitionInstallError',
`${timeoutConstants.timeoutMessage}, MESSAGE: ${error.message}, CODE: ${error.code}, SIGNAL: ${error.signal}`, error.stack);
this.eventStream.post(new DotnetAcquisitionInstallError(newError, installKey));
reject(newError);
}
}
else if (stderr && stderr.length > 0)
Expand All @@ -127,10 +131,11 @@ You will need to restart VS Code after these changes. If PowerShell is still not
}
});
}
catch (error)
catch (error : any)
{
this.eventStream.post(new DotnetAcquisitionUnexpectedError(error as Error, installKey));
reject(error);
const newError = new EventBasedError('DotnetAcquisitionUnexpectedError', error?.message, error?.stack)
this.eventStream.post(new DotnetAcquisitionUnexpectedError(newError, installKey));
reject(newError);
}
});
}
Expand Down Expand Up @@ -211,7 +216,7 @@ If you cannot safely and confidently change the execution policy, try setting a
error = err;
}
}
catch(err)
catch(err : any)
{
if(!knownError)
{
Expand All @@ -222,7 +227,7 @@ If you cannot safely and confidently change the execution policy, try setting a
if(error != null)
{
this.eventStream.post(new DotnetAcquisitionScriptError(error as Error, installKey));
throw error;
throw new EventBasedError('DotnetAcquisitionScriptError', error?.message, error?.stack);
}

return command!.commandRoot;
Expand Down
58 changes: 39 additions & 19 deletions vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ import {
DotnetCompletedGlobalInstallerExecution,
DotnetFakeSDKEnvironmentVariableTriggered,
SuppressedAcquisitionError,
EventBasedError,
EventCancellationError,
} from '../EventStream/EventStreamEvents';

import { GlobalInstallerResolver } from './GlobalInstallerResolver';
import { WinMacGlobalInstaller } from './WinMacGlobalInstaller';
import { LinuxGlobalInstaller } from './LinuxGlobalInstaller';
import { TelemetryUtilities } from '../EventStream/TelemetryUtilities';
import { Debugging } from '../Utils/Debugging';
import { IDotnetAcquireContext} from '../IDotnetAcquireContext';
import { DotnetInstallType, IDotnetAcquireContext} from '../IDotnetAcquireContext';
import { IGlobalInstaller } from './IGlobalInstaller';
import { IVSCodeExtensionContext } from '../IVSCodeExtensionContext';
import { IUtilityContext } from '../Utils/IUtilityContext';
Expand Down Expand Up @@ -128,7 +130,7 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
*/
public async acquireStatus(version: string, installMode: DotnetInstallMode, architecture? : string): Promise<IDotnetAcquireResult | undefined>
{
const install = GetDotnetInstallInfo(version, installMode, false, architecture ? architecture : this.installingArchitecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture())
const install = GetDotnetInstallInfo(version, installMode, 'local', architecture ? architecture : this.installingArchitecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture())

const existingAcquisitionPromise = this.installTracker.getPromise(install);
if (existingAcquisitionPromise)
Expand Down Expand Up @@ -176,14 +178,14 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
private async acquire(version: string, mode: DotnetInstallMode,
globalInstallerResolver : GlobalInstallerResolver | null = null, localInvoker? : IAcquisitionInvoker): Promise<IDotnetAcquireResult>
{
let install = GetDotnetInstallInfo(version, mode, globalInstallerResolver !== null, this.installingArchitecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture());
let install = GetDotnetInstallInfo(version, mode, globalInstallerResolver !== null ? 'global' : 'local', this.installingArchitecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture());

// Allow for the architecture to be null, which is a legacy behavior.
if(this.context.acquisitionContext?.architecture === null && this.context.acquisitionContext?.architecture !== undefined)
{
install =
{
installKey: DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(version, null, globalInstallerResolver !== null),
installKey: DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(version, null, globalInstallerResolver !== null ? 'global' : 'local'),
version: install.version,
isGlobal: install.isGlobal,
installMode: mode,
Expand All @@ -208,20 +210,20 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
{
Debugging.log(`The Acquisition Worker has Determined a Global Install was requested.`, this.context.eventStream);

acquisitionPromise = this.acquireGlobalCore(globalInstallerResolver, install).catch(async (error: Error) => {
acquisitionPromise = this.acquireGlobalCore(globalInstallerResolver, install).catch(async (error: any) =>
{
await this.installTracker.untrackInstallingVersion(install);
error.message = `.NET Acquisition Failed: ${error.message}`;
throw error;
const err = this.getErrorOrStringAsEventError(error);
throw err;
});
}
else
{
Debugging.log(`The Acquisition Worker has Determined a Local Install was requested.`, this.context.eventStream);

acquisitionPromise = this.acquireLocalCore(version, mode, install, localInvoker!).catch(async (error: Error) => {
acquisitionPromise = this.acquireLocalCore(version, mode, install, localInvoker!).catch(async (error: any) =>
{
await this.installTracker.untrackInstallingVersion(install);
error.message = `.NET Acquisition Failed: ${error.message}`;
throw error;
const err = this.getErrorOrStringAsEventError(error);
throw err;
});
}

Expand All @@ -232,22 +234,23 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
}
}

public static getInstallKeyCustomArchitecture(version : string, architecture: string | null | undefined, isGlobal = false) : string
public static getInstallKeyCustomArchitecture(version : string, architecture: string | null | undefined,
installType : DotnetInstallType = 'local') : string
{
if(!architecture)
{
// Use the legacy method (no architecture) of installs
return isGlobal ? `${version}-global` : version;
return installType === 'global' ? `${version}-global` : version;
}
else
{
return isGlobal ? `${version}-global~${architecture}` : `${version}~${architecture}`;
return installType === 'global' ? `${version}-global~${architecture}` : `${version}~${architecture}`;
}
}

public getInstallKey(version : string) : string
{
return DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(version, this.installingArchitecture, this.globalResolver !== null);
return DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(version, this.installingArchitecture, this.globalResolver !== null ? 'global' : 'local');
}

/**
Expand Down Expand Up @@ -296,11 +299,13 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
timeoutSeconds: this.context.timeoutSeconds,
installRuntime : mode === 'runtime',
installMode : mode,
installType : this.context.acquisitionContext?.installType ?? 'local', // Before this API param existed, all calls were for local types.
architecture: this.installingArchitecture
} as IDotnetInstallationContext;
this.context.eventStream.post(new DotnetAcquisitionStarted(install, version, this.context.acquisitionContext?.requestingExtensionId));
await acquisitionInvoker.installDotnet(installContext, install).catch((reason) => {
throw Error(`Installation failed: ${reason}`);
await acquisitionInvoker.installDotnet(installContext, install).catch((reason) =>
{
throw reason; // This will get handled and cast into an event based error by its caller.
});
this.context.installationValidator.validateDotnetInstall(install, dotnetPath);

Expand Down Expand Up @@ -345,6 +350,20 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
return os.arch();
}

private getErrorOrStringAsEventError(error : any)
{
if(error instanceof EventBasedError || error instanceof EventCancellationError)
{
error.message = `.NET Acquisition Failed: ${error.message}`;
return error;
}
else
{
const newError = new EventBasedError('DotnetAcquisitionError', `.NET Acquisition Failed: ${error?.message ?? error}`);
return newError;
}
}

private async acquireGlobalCore(globalInstallerResolver : GlobalInstallerResolver, install : DotnetInstall): Promise<string>
{
const installingVersion = await globalInstallerResolver.getFullySpecifiedVersion();
Expand All @@ -371,7 +390,8 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker

if(installerResult !== '0')
{
const err = new DotnetNonZeroInstallerExitCodeError(new Error(`An error was raised by the .NET SDK installer. The exit code it gave us: ${installerResult}`), install);
const err = new DotnetNonZeroInstallerExitCodeError(new EventBasedError('DotnetNonZeroInstallerExitCodeError',
`An error was raised by the .NET SDK installer. The exit code it gave us: ${installerResult}`), install);
this.context.eventStream.post(err);
throw err;
}
Expand Down
7 changes: 4 additions & 3 deletions vscode-dotnet-runtime-library/src/Acquisition/DotnetInstall.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* The .NET Foundation licenses this file to you under the MIT license.
*--------------------------------------------------------------------------------------------*/

import { DotnetInstallType } from '..';
import { DotnetCoreAcquisitionWorker } from './DotnetCoreAcquisitionWorker';
import { DotnetInstallMode } from './DotnetInstallMode';

Expand Down Expand Up @@ -67,12 +68,12 @@ export function looksLikeRuntimeVersion(version: string): boolean {
return !band || band.length <= 2; // assumption : there exists no runtime version at this point over 99 sub versions
}

export function GetDotnetInstallInfo(installVersion: string, installationMode: DotnetInstallMode, isGlobalInstall: boolean, installArchitecture: string): DotnetInstall {
export function GetDotnetInstallInfo(installVersion: string, installationMode: DotnetInstallMode, installType: DotnetInstallType, installArchitecture: string): DotnetInstall {
return {
installKey: DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(installVersion, installArchitecture),
installKey: DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(installVersion, installArchitecture, installType),
version: installVersion,
architecture: installArchitecture,
isGlobal: isGlobalInstall,
isGlobal: installType === 'global',
installMode: installationMode,
} as DotnetInstall;
}
Loading

0 comments on commit 79b9139

Please sign in to comment.