Skip to content

Commit 94dab3a

Browse files
committed
Fix the error output
The true problem was that the error type was bundled inside another error type due to some old architectural decision. CoPilot had put a band aid over this issue and it was only partially fixed. I took the liberty of improving the error messages as well. We shouldnt include a stack in a publicly facing message.
1 parent bf3965d commit 94dab3a

File tree

4 files changed

+36
-90
lines changed

4 files changed

+36
-90
lines changed

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ import
3535
DotnetWSLSecurityError,
3636
EventBasedError,
3737
EventCancellationError,
38-
SuppressedAcquisitionError,
39-
UtilizingExistingInstallPromise
38+
SuppressedAcquisitionError
4039
} from '../EventStream/EventStreamEvents';
4140
import * as versionUtils from './VersionUtilities';
4241

@@ -399,12 +398,16 @@ To keep your .NET version up to date, please reconnect to the internet at your s
399398
{
400399
if (error instanceof EventBasedError || error instanceof EventCancellationError)
401400
{
402-
error.message = `.NET Acquisition Failed: ${error.message}, ${error?.stack}`;
403401
return error;
404402
}
405403
else
406404
{
407-
// Remove this when https://github.com/typescript-eslint/typescript-eslint/issues/2728 is done
405+
if (error?.error?.message !== undefined) // DotnetAcquisitionError is a bad but common pattern where the error is included in the thrown object
406+
{
407+
const convertedError = new EventBasedError(error.constructor.name, error.error.message, error?.stack);
408+
return convertedError;
409+
}
410+
408411
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
409412
const newError = new EventBasedError('DotnetAcquisitionError', `.NET Acquisition Failed: ${error?.message ?? JSON.stringify(error)}`);
410413
return newError;

vscode-dotnet-runtime-library/src/Acquisition/WinMacGlobalInstaller.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,7 @@ We cannot verify our .NET file host at this time. Please try again later or inst
9191
*/
9292
public static IsUserFriendlyExitCode(code: string): boolean
9393
{
94-
const userFriendlyExitCodes = ['5', '112', '255', '1618', '2147942405', UNABLE_TO_ACQUIRE_GLOBAL_LOCK_ERR];
95-
return userFriendlyExitCodes.includes(code);
94+
return this.InterpretExitCode(code) !== '';
9695
}
9796

9897
public static InterpretExitCode(code: string): string
@@ -108,23 +107,23 @@ This report should be made at https://github.com/dotnet/vscode-dotnet-runtime/is
108107
case '5':
109108
return `Insufficient permissions are available to install .NET. Please run the installer as an administrator.`;
110109
case '67':
111-
return `The network name cannot be found. ${reportLogMessage}`;
110+
return `The network name cannot be found to install .NET. ${reportLogMessage}`;
112111
case '112':
113112
return `The disk is full. Please free up space and try again.`;
114113
case '255':
115114
return `The .NET Installer was terminated by another process unexpectedly. Please try again.`;
116115
case '1260':
117-
return `The .NET SDK is blocked by group policy. Can you please report this at https://github.com/dotnet/vscode-dotnet-runtime/issues`
116+
return `The .NET SDK Install is blocked by group policy. Please contact your IT Admin to install .NET.`
118117
case '1460':
119118
return `The .NET SDK had a timeout error. ${reportLogMessage}`;
120119
case '1603':
121120
return `Fatal error during .NET SDK installation. ${reportLogMessage}`;
122121
case '1618':
123-
return `Another installation is already in progress. Complete that installation before proceeding with this install.`;
122+
return `Another installation of .NET is already in progress. Complete that installation before proceeding with this install.`;
124123
case '000751':
125-
return `Page fault was satisfied by reading from a secondary storage device. ${reportLogMessage}`;
124+
return `.NET Installer Failed: A page fault was satisfied by reading from a secondary storage device. ${reportLogMessage}`;
126125
case '2147500037':
127-
return `An unspecified error occurred. ${reportLogMessage}`;
126+
return `.NET Installer Failed: An unspecified error occurred. ${reportLogMessage}`;
128127
case '2147942405':
129128
return `Insufficient permissions are available to install .NET. Please try again as an administrator.`;
130129
case UNABLE_TO_ACQUIRE_GLOBAL_LOCK_ERR:

vscode-dotnet-runtime-library/src/EventStream/OutputChannelObserver.ts

Lines changed: 2 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,14 @@ import
1414
DotnetDebuggingMessage,
1515
DotnetExistingPathResolutionCompleted,
1616
DotnetInstallExpectedAbort,
17-
DotnetNonZeroInstallerExitCodeError,
1817
DotnetOfflineInstallUsed,
1918
DotnetOfflineWarning,
2019
DotnetUpgradedEvent,
21-
DotnetVisibleWarningEvent,
20+
DotnetVisibleWarningEvent
2221
} from './EventStreamEvents';
2322
import { EventType } from './EventType';
2423
import { IEvent } from './IEvent';
2524
import { IEventStreamObserver } from './IEventStreamObserver';
26-
import { WinMacGlobalInstaller } from '../Acquisition/WinMacGlobalInstaller';
2725

2826
export class OutputChannelObserver implements IEventStreamObserver
2927
{
@@ -117,37 +115,7 @@ export class OutputChannelObserver implements IEventStreamObserver
117115
if (this.inProgressDownloads.includes(error?.install?.installId ?? ''))
118116
{
119117
this.outputChannel.appendLine(`Failed to download .NET ${error?.install?.installId}:`);
120-
121-
// For user-friendly installer exit codes, show only the clean interpreted message
122-
if (error instanceof DotnetNonZeroInstallerExitCodeError)
123-
{
124-
const errorMessage = error?.error?.message || '';
125-
const exitCodeMatch = errorMessage.match(/The exit code it gave us: (\w+)\./);
126-
if (exitCodeMatch && WinMacGlobalInstaller.IsUserFriendlyExitCode(exitCodeMatch[1]))
127-
{
128-
// Extract just the interpreted message (everything after the exit code line)
129-
const lines = errorMessage.split('\n');
130-
if (lines.length > 1)
131-
{
132-
this.outputChannel.appendLine(lines.slice(1).join('\n'));
133-
}
134-
else
135-
{
136-
this.outputChannel.appendLine(errorMessage);
137-
}
138-
}
139-
else
140-
{
141-
// Show full error message for non-user-friendly codes
142-
this.outputChannel.appendLine(error?.error?.message);
143-
}
144-
}
145-
else
146-
{
147-
// Show full error message for non-installer errors
148-
this.outputChannel.appendLine(error?.error?.message);
149-
}
150-
118+
this.outputChannel.appendLine(error?.error?.message);
151119
this.outputChannel.appendLine('');
152120

153121
this.updateDownloadIndicators(error.install?.installId);

vscode-dotnet-runtime-library/src/test/unit/WinMacGlobalInstaller.test.ts

Lines changed: 21 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -223,49 +223,25 @@ ${fs.readdirSync(installerDownloadFolder).join(', ')}`);
223223
shouldNotExistOptionPath = await installer.getExpectedGlobalSDKPath(sdkVersionThatShouldNotExist, os.arch());
224224
assert.equal(shouldNotExistOptionPath, standardHostPath, 'It wont use the emu path if it does not exist');
225225
}
226-
}
227-
}).timeout(standardTimeoutTime);
228-
229-
test('InterpretExitCode returns user-friendly messages for common exit codes', () =>
230-
{
231-
// Test exit code 5 - insufficient permissions
232-
const exitCode5Message = WinMacGlobalInstaller.InterpretExitCode('5');
233-
assert.equal(exitCode5Message, 'Insufficient permissions are available to install .NET. Please run the installer as an administrator.');
234-
assert.isFalse(exitCode5Message.includes('report'), 'Exit code 5 should not ask for bug reports');
235-
assert.isTrue(WinMacGlobalInstaller.IsUserFriendlyExitCode('5'), 'Exit code 5 should be marked as user-friendly');
236-
237-
// Test exit code 1618 - another installation in progress
238-
const exitCode1618Message = WinMacGlobalInstaller.InterpretExitCode('1618');
239-
assert.equal(exitCode1618Message, 'Another installation is already in progress. Complete that installation before proceeding with this install.');
240-
assert.isFalse(exitCode1618Message.includes('report'), 'Exit code 1618 should not ask for bug reports');
241-
assert.isTrue(WinMacGlobalInstaller.IsUserFriendlyExitCode('1618'), 'Exit code 1618 should be marked as user-friendly');
242-
243-
// Test exit code 112 - disk full
244-
const exitCode112Message = WinMacGlobalInstaller.InterpretExitCode('112');
245-
assert.equal(exitCode112Message, 'The disk is full. Please free up space and try again.');
246-
assert.isFalse(exitCode112Message.includes('report'), 'Exit code 112 should not ask for bug reports');
247-
assert.isTrue(WinMacGlobalInstaller.IsUserFriendlyExitCode('112'), 'Exit code 112 should be marked as user-friendly');
248-
249-
// Test exit code 255 - terminated by another process
250-
const exitCode255Message = WinMacGlobalInstaller.InterpretExitCode('255');
251-
assert.equal(exitCode255Message, 'The .NET Installer was terminated by another process unexpectedly. Please try again.');
252-
assert.isFalse(exitCode255Message.includes('report'), 'Exit code 255 should not ask for bug reports');
253-
assert.isTrue(WinMacGlobalInstaller.IsUserFriendlyExitCode('255'), 'Exit code 255 should be marked as user-friendly');
254-
255-
// Test exit code 2147942405 - insufficient permissions (alternative code)
256-
const exitCode2147942405Message = WinMacGlobalInstaller.InterpretExitCode('2147942405');
257-
assert.equal(exitCode2147942405Message, 'Insufficient permissions are available to install .NET. Please try again as an administrator.');
258-
assert.isFalse(exitCode2147942405Message.includes('report'), 'Exit code 2147942405 should not ask for bug reports');
259-
assert.isTrue(WinMacGlobalInstaller.IsUserFriendlyExitCode('2147942405'), 'Exit code 2147942405 should be marked as user-friendly');
260-
261-
// Test exit code 1 - generic failure (should include report message)
262-
const exitCode1Message = WinMacGlobalInstaller.InterpretExitCode('1');
263-
assert.isTrue(exitCode1Message.includes('report'), 'Exit code 1 should ask for bug reports');
264-
assert.isFalse(WinMacGlobalInstaller.IsUserFriendlyExitCode('1'), 'Exit code 1 should not be marked as user-friendly');
265-
266-
// Test unknown exit code
267-
const unknownCodeMessage = WinMacGlobalInstaller.InterpretExitCode('9999');
268-
assert.equal(unknownCodeMessage, '', 'Unknown exit codes should return empty string');
269-
assert.isFalse(WinMacGlobalInstaller.IsUserFriendlyExitCode('9999'), 'Unknown exit codes should not be marked as user-friendly');
270-
});
226+
}
227+
}).timeout(standardTimeoutTime);
228+
229+
test('InterpretExitCode returns user-friendly messages for common exit codes', () =>
230+
{
231+
// Test exit code 5 - insufficient permissions
232+
const exitCode5Message = WinMacGlobalInstaller.InterpretExitCode('5');
233+
assert.equal(exitCode5Message, 'Insufficient permissions are available to install .NET. Please run the installer as an administrator.');
234+
assert.isFalse(exitCode5Message.includes('report'), 'Exit code 5 should not ask for bug reports');
235+
assert.isTrue(WinMacGlobalInstaller.IsUserFriendlyExitCode('5'), 'Exit code 5 should be marked as user-friendly');
236+
237+
// Test exit code 1 - generic failure (should include report message)
238+
const exitCode1Message = WinMacGlobalInstaller.InterpretExitCode('1');
239+
assert.isTrue(exitCode1Message.includes('report'), 'Exit code 1 should ask for bug reports');
240+
assert.isFalse(WinMacGlobalInstaller.IsUserFriendlyExitCode('1'), 'Exit code 1 should not be marked as user-friendly');
241+
242+
// Test unknown exit code
243+
const unknownCodeMessage = WinMacGlobalInstaller.InterpretExitCode('9999');
244+
assert.equal(unknownCodeMessage, '', 'Unknown exit codes should return empty string');
245+
assert.isFalse(WinMacGlobalInstaller.IsUserFriendlyExitCode('9999'), 'Unknown exit codes should not be marked as user-friendly');
246+
});
271247
});

0 commit comments

Comments
 (0)