Skip to content

Commit 79b9139

Browse files
nagilsonnagilson
authored
Improve VS Code SDK Error Diagnosability (#1822)
 * 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
1 parent 30beed1 commit 79b9139

30 files changed

+327
-159
lines changed

‎vscode-dotnet-runtime-extension/package-lock.json‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎vscode-dotnet-runtime-extension/src/extension.ts‎

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ import {
2020
DotnetExistingPathResolutionCompleted,
2121
DotnetRuntimeAcquisitionStarted,
2222
DotnetRuntimeAcquisitionTotalSuccessEvent,
23+
DotnetGlobalSDKAcquisitionTotalSuccessEvent,
2324
enableExtensionTelemetry,
25+
EventBasedError,
2426
ErrorConfiguration,
2527
ExistingPathResolver,
2628
ExtensionConfigurationWorker,
@@ -166,7 +168,8 @@ export function activate(context: vscode.ExtensionContext, extensionContext?: IE
166168
}
167169

168170
if (!commandContext.version || commandContext.version === 'latest') {
169-
throw new Error(`Cannot acquire .NET version "${commandContext.version}". Please provide a valid version.`);
171+
throw new EventBasedError('BadContextualVersion',
172+
`Cannot acquire .NET version "${commandContext.version}". Please provide a valid version.`);
170173
}
171174

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

204-
const pathResult = callWithErrorHandling(async () =>
207+
let fullyResolvedVersion = '';
208+
209+
const pathResult = await callWithErrorHandling(async () =>
205210
{
211+
// Warning: Between now and later in this call-stack, the context 'version' is incomplete as it has not been resolved.
212+
// Errors between here and the place where it is resolved cannot be routed to one another.
213+
214+
sdkAcquisitionWorker.setAcquisitionContext(commandContext);
215+
telemetryObserver?.setAcquisitionContext(sdkContext, commandContext);
216+
217+
if(commandContext.version === '' || !commandContext.version)
218+
{
219+
throw new EventCancellationError('BadContextualRuntimeVersionError',
220+
`No version was defined to install.`);
221+
}
222+
206223
globalEventStream.post(new DotnetSDKAcquisitionStarted(commandContext.requestingExtensionId));
207224
globalEventStream.post(new DotnetAcquisitionRequested(commandContext.version, commandContext.requestingExtensionId));
208225

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

232+
const globalInstallerResolver = new GlobalInstallerResolver(sdkContext, commandContext.version);
233+
fullyResolvedVersion = await globalInstallerResolver.getFullySpecifiedVersion();
234+
235+
// Reset context to point to the fully specified version so it is not possible for someone to access incorrect data during the install process.
236+
commandContext.version = fullyResolvedVersion;
215237
sdkAcquisitionWorker.setAcquisitionContext(commandContext);
216238
telemetryObserver?.setAcquisitionContext(sdkContext, commandContext);
217239

218-
if(commandContext.version === '' || !commandContext.version)
219-
{
220-
throw Error(`No version was defined to install.`);
221-
}
222-
223-
const globalInstallerResolver = new GlobalInstallerResolver(sdkContext, commandContext.version);
224240
outputChannel.show(true);
225241
const dotnetPath = await sdkAcquisitionWorker.acquireGlobalSDK(globalInstallerResolver);
226242

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

247+
const iKey = sdkAcquisitionWorker.getInstallKey(fullyResolvedVersion);
248+
const install = {installKey : iKey, version : fullyResolvedVersion, installMode: 'sdk', isGlobal: true,
249+
architecture: commandContext.architecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture()} as DotnetInstall;
250+
251+
globalEventStream.post(new DotnetGlobalSDKAcquisitionTotalSuccessEvent(commandContext.version, install, commandContext.requestingExtensionId ?? '', pathResult?.dotnetPath ?? ''));
231252
return pathResult;
232253
});
233254

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

246267
if (!activeSupportVersions || activeSupportVersions.length < 1)
247268
{
248-
const err = new Error(`An active-support version of dotnet couldn't be found. Discovered versions: ${JSON.stringify(availableVersions)}`);
249-
globalEventStream.post(new DotnetVersionResolutionError(err as EventCancellationError, null));
269+
const err = new EventCancellationError('DotnetVersionResolutionError', `An active-support version of dotnet couldn't be found. Discovered versions: ${JSON.stringify(availableVersions)}`);
270+
globalEventStream.post(new DotnetVersionResolutionError(err, null));
250271
if(!availableVersions || availableVersions.length < 1)
251272
{
252273
return [];

‎vscode-dotnet-runtime-extension/yarn.lock‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,8 +2481,8 @@
24812481
"@types/shelljs" "0.8.9"
24822482
"@types/vscode" "1.74.0"
24832483
"@vscode/sudo-prompt" "^9.3.1"
2484-
"axios" "^1.3.4"
2485-
"axios-cache-interceptor" "^1.0.1"
2484+
"axios" "^1.7.2"
2485+
"axios-cache-interceptor" "^1.5.3"
24862486
"axios-retry" "^3.4.0"
24872487
"chai" "4.3.4"
24882488
"chai-as-promised" "^7.1.1"

‎vscode-dotnet-runtime-library/src/Acquisition/AcquisitionInvoker.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
DotnetAcquisitionTimeoutError,
1616
DotnetAcquisitionUnexpectedError,
1717
DotnetOfflineFailure,
18+
EventBasedError,
1819
} from '../EventStream/EventStreamEvents';
1920

2021
import { timeoutConstants } from '../Utils/ErrorHandler';
@@ -100,19 +101,22 @@ You will need to restart VS Code after these changes. If PowerShell is still not
100101
{
101102
if (!(await this.isOnline(installContext)))
102103
{
103-
const offlineError = new Error('No internet connection detected: Cannot install .NET');
104+
const offlineError = new EventBasedError('DotnetOfflineFailure', 'No internet connection detected: Cannot install .NET');
104105
this.eventStream.post(new DotnetOfflineFailure(offlineError, installKey));
105106
reject(offlineError);
106107
}
107108
else if (error.signal === 'SIGKILL') {
108-
error.message = timeoutConstants.timeoutMessage;
109+
const newError = new EventBasedError('DotnetAcquisitionTimeoutError',
110+
`${timeoutConstants.timeoutMessage}, MESSAGE: ${error.message}, CODE: ${error.code}, KILLED: ${error.killed}`, error.stack);
109111
this.eventStream.post(new DotnetAcquisitionTimeoutError(error, installKey, installContext.timeoutSeconds));
110-
reject(error);
112+
reject(newError);
111113
}
112114
else
113115
{
114-
this.eventStream.post(new DotnetAcquisitionInstallError(error, installKey));
115-
reject(error);
116+
const newError = new EventBasedError('DotnetAcquisitionInstallError',
117+
`${timeoutConstants.timeoutMessage}, MESSAGE: ${error.message}, CODE: ${error.code}, SIGNAL: ${error.signal}`, error.stack);
118+
this.eventStream.post(new DotnetAcquisitionInstallError(newError, installKey));
119+
reject(newError);
116120
}
117121
}
118122
else if (stderr && stderr.length > 0)
@@ -127,10 +131,11 @@ You will need to restart VS Code after these changes. If PowerShell is still not
127131
}
128132
});
129133
}
130-
catch (error)
134+
catch (error : any)
131135
{
132-
this.eventStream.post(new DotnetAcquisitionUnexpectedError(error as Error, installKey));
133-
reject(error);
136+
const newError = new EventBasedError('DotnetAcquisitionUnexpectedError', error?.message, error?.stack)
137+
this.eventStream.post(new DotnetAcquisitionUnexpectedError(newError, installKey));
138+
reject(newError);
134139
}
135140
});
136141
}
@@ -211,7 +216,7 @@ If you cannot safely and confidently change the execution policy, try setting a
211216
error = err;
212217
}
213218
}
214-
catch(err)
219+
catch(err : any)
215220
{
216221
if(!knownError)
217222
{
@@ -222,7 +227,7 @@ If you cannot safely and confidently change the execution policy, try setting a
222227
if(error != null)
223228
{
224229
this.eventStream.post(new DotnetAcquisitionScriptError(error as Error, installKey));
225-
throw error;
230+
throw new EventBasedError('DotnetAcquisitionScriptError', error?.message, error?.stack);
226231
}
227232

228233
return command!.commandRoot;

‎vscode-dotnet-runtime-library/src/Acquisition/DotnetCoreAcquisitionWorker.ts‎

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,16 @@ import {
2929
DotnetCompletedGlobalInstallerExecution,
3030
DotnetFakeSDKEnvironmentVariableTriggered,
3131
SuppressedAcquisitionError,
32+
EventBasedError,
33+
EventCancellationError,
3234
} from '../EventStream/EventStreamEvents';
3335

3436
import { GlobalInstallerResolver } from './GlobalInstallerResolver';
3537
import { WinMacGlobalInstaller } from './WinMacGlobalInstaller';
3638
import { LinuxGlobalInstaller } from './LinuxGlobalInstaller';
3739
import { TelemetryUtilities } from '../EventStream/TelemetryUtilities';
3840
import { Debugging } from '../Utils/Debugging';
39-
import { IDotnetAcquireContext} from '../IDotnetAcquireContext';
41+
import { DotnetInstallType, IDotnetAcquireContext} from '../IDotnetAcquireContext';
4042
import { IGlobalInstaller } from './IGlobalInstaller';
4143
import { IVSCodeExtensionContext } from '../IVSCodeExtensionContext';
4244
import { IUtilityContext } from '../Utils/IUtilityContext';
@@ -128,7 +130,7 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
128130
*/
129131
public async acquireStatus(version: string, installMode: DotnetInstallMode, architecture? : string): Promise<IDotnetAcquireResult | undefined>
130132
{
131-
const install = GetDotnetInstallInfo(version, installMode, false, architecture ? architecture : this.installingArchitecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture())
133+
const install = GetDotnetInstallInfo(version, installMode, 'local', architecture ? architecture : this.installingArchitecture ?? DotnetCoreAcquisitionWorker.defaultArchitecture())
132134

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

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

211-
acquisitionPromise = this.acquireGlobalCore(globalInstallerResolver, install).catch(async (error: Error) => {
213+
acquisitionPromise = this.acquireGlobalCore(globalInstallerResolver, install).catch(async (error: any) =>
214+
{
212215
await this.installTracker.untrackInstallingVersion(install);
213-
error.message = `.NET Acquisition Failed: ${error.message}`;
214-
throw error;
216+
const err = this.getErrorOrStringAsEventError(error);
217+
throw err;
215218
});
216219
}
217220
else
218221
{
219-
Debugging.log(`The Acquisition Worker has Determined a Local Install was requested.`, this.context.eventStream);
220-
221-
acquisitionPromise = this.acquireLocalCore(version, mode, install, localInvoker!).catch(async (error: Error) => {
222+
acquisitionPromise = this.acquireLocalCore(version, mode, install, localInvoker!).catch(async (error: any) =>
223+
{
222224
await this.installTracker.untrackInstallingVersion(install);
223-
error.message = `.NET Acquisition Failed: ${error.message}`;
224-
throw error;
225+
const err = this.getErrorOrStringAsEventError(error);
226+
throw err;
225227
});
226228
}
227229

@@ -232,22 +234,23 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
232234
}
233235
}
234236

235-
public static getInstallKeyCustomArchitecture(version : string, architecture: string | null | undefined, isGlobal = false) : string
237+
public static getInstallKeyCustomArchitecture(version : string, architecture: string | null | undefined,
238+
installType : DotnetInstallType = 'local') : string
236239
{
237240
if(!architecture)
238241
{
239242
// Use the legacy method (no architecture) of installs
240-
return isGlobal ? `${version}-global` : version;
243+
return installType === 'global' ? `${version}-global` : version;
241244
}
242245
else
243246
{
244-
return isGlobal ? `${version}-global~${architecture}` : `${version}~${architecture}`;
247+
return installType === 'global' ? `${version}-global~${architecture}` : `${version}~${architecture}`;
245248
}
246249
}
247250

248251
public getInstallKey(version : string) : string
249252
{
250-
return DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(version, this.installingArchitecture, this.globalResolver !== null);
253+
return DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(version, this.installingArchitecture, this.globalResolver !== null ? 'global' : 'local');
251254
}
252255

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

@@ -345,6 +350,20 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
345350
return os.arch();
346351
}
347352

353+
private getErrorOrStringAsEventError(error : any)
354+
{
355+
if(error instanceof EventBasedError || error instanceof EventCancellationError)
356+
{
357+
error.message = `.NET Acquisition Failed: ${error.message}`;
358+
return error;
359+
}
360+
else
361+
{
362+
const newError = new EventBasedError('DotnetAcquisitionError', `.NET Acquisition Failed: ${error?.message ?? error}`);
363+
return newError;
364+
}
365+
}
366+
348367
private async acquireGlobalCore(globalInstallerResolver : GlobalInstallerResolver, install : DotnetInstall): Promise<string>
349368
{
350369
const installingVersion = await globalInstallerResolver.getFullySpecifiedVersion();
@@ -371,7 +390,8 @@ export class DotnetCoreAcquisitionWorker implements IDotnetCoreAcquisitionWorker
371390

372391
if(installerResult !== '0')
373392
{
374-
const err = new DotnetNonZeroInstallerExitCodeError(new Error(`An error was raised by the .NET SDK installer. The exit code it gave us: ${installerResult}`), install);
393+
const err = new DotnetNonZeroInstallerExitCodeError(new EventBasedError('DotnetNonZeroInstallerExitCodeError',
394+
`An error was raised by the .NET SDK installer. The exit code it gave us: ${installerResult}`), install);
375395
this.context.eventStream.post(err);
376396
throw err;
377397
}

‎vscode-dotnet-runtime-library/src/Acquisition/DotnetInstall.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* The .NET Foundation licenses this file to you under the MIT license.
44
*--------------------------------------------------------------------------------------------*/
55

6+
import { DotnetInstallType } from '..';
67
import { DotnetCoreAcquisitionWorker } from './DotnetCoreAcquisitionWorker';
78
import { DotnetInstallMode } from './DotnetInstallMode';
89

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

70-
export function GetDotnetInstallInfo(installVersion: string, installationMode: DotnetInstallMode, isGlobalInstall: boolean, installArchitecture: string): DotnetInstall {
71+
export function GetDotnetInstallInfo(installVersion: string, installationMode: DotnetInstallMode, installType: DotnetInstallType, installArchitecture: string): DotnetInstall {
7172
return {
72-
installKey: DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(installVersion, installArchitecture),
73+
installKey: DotnetCoreAcquisitionWorker.getInstallKeyCustomArchitecture(installVersion, installArchitecture, installType),
7374
version: installVersion,
7475
architecture: installArchitecture,
75-
isGlobal: isGlobalInstall,
76+
isGlobal: installType === 'global',
7677
installMode: installationMode,
7778
} as DotnetInstall;
7879
}

0 commit comments

Comments
 (0)