Skip to content

Commit 7ec2211

Browse files
authored
microsoft#190233 fix error handling (microsoft#210565)
1 parent b78f20b commit 7ec2211

File tree

3 files changed

+30
-37
lines changed

3 files changed

+30
-37
lines changed

src/vs/platform/extensionManagement/common/abstractExtensionManagementService.ts

Lines changed: 27 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { distinct, isNonEmptyArray } from 'vs/base/common/arrays';
77
import { Barrier, CancelablePromise, createCancelablePromise } from 'vs/base/common/async';
88
import { CancellationToken } from 'vs/base/common/cancellation';
9-
import { CancellationError, getErrorMessage } from 'vs/base/common/errors';
9+
import { CancellationError, getErrorMessage, isCancellationError } from 'vs/base/common/errors';
1010
import { Emitter, Event } from 'vs/base/common/event';
1111
import { Disposable, toDisposable } from 'vs/base/common/lifecycle';
1212
import { isWeb } from 'vs/base/common/platform';
@@ -137,16 +137,6 @@ export abstract class AbstractExtensionManagementService extends Disposable impl
137137
results.push(...await this.installExtensions(installableExtensions));
138138
}
139139

140-
for (const result of results) {
141-
if (result.error) {
142-
this.logService.error(`Failed to install extension.`, result.identifier.id);
143-
this.logService.error(result.error);
144-
if (result.source && !URI.isUri(result.source)) {
145-
reportTelemetry(this.telemetryService, 'extensionGallery:install', { extensionData: getGalleryExtensionTelemetryData(result.source), error: result.error });
146-
}
147-
}
148-
}
149-
150140
return results;
151141
}
152142

@@ -320,7 +310,11 @@ export abstract class AbstractExtensionManagementService extends Disposable impl
320310
}
321311
}
322312
installExtensionResultsMap.set(key, { local, identifier: task.identifier, operation: task.operation, source: task.source, context: task.options.context, profileLocation: task.profileLocation, applicationScoped: local.isApplicationScoped });
323-
} catch (error) {
313+
} catch (e) {
314+
const error = toExtensionManagementError(e);
315+
if (!URI.isUri(task.source)) {
316+
reportTelemetry(this.telemetryService, task.operation === InstallOperation.Update ? 'extensionGallery:update' : 'extensionGallery:install', { extensionData: getGalleryExtensionTelemetryData(task.source), error });
317+
}
324318
installExtensionResultsMap.set(key, { error, identifier: task.identifier, operation: task.operation, source: task.source, context: task.options.context, profileLocation: task.profileLocation, applicationScoped: task.options.isApplicationScoped });
325319
this.logService.error('Error while installing the extension', task.identifier.id, getErrorMessage(error));
326320
throw error;
@@ -446,9 +440,21 @@ export abstract class AbstractExtensionManagementService extends Disposable impl
446440
}
447441
}
448442

449-
// If there are errors, throw the error.
443+
// Throw if there are errors
450444
if (errors.length) {
451-
throw joinErrors(errors);
445+
if (errors.length === 1) {
446+
throw errors[0];
447+
}
448+
449+
let error = new ExtensionManagementError('', ExtensionManagementErrorCode.Unknown);
450+
for (const current of errors) {
451+
const code = current instanceof ExtensionManagementError ? current.code : ExtensionManagementErrorCode.Unknown;
452+
error = new ExtensionManagementError(
453+
current.message ? `${current.message}, ${error.message}` : error.message,
454+
code !== ExtensionManagementErrorCode.Unknown && code !== ExtensionManagementErrorCode.Internal ? code : error.code
455+
);
456+
}
457+
throw error;
452458
}
453459

454460
return results;
@@ -660,7 +666,7 @@ export abstract class AbstractExtensionManagementService extends Disposable impl
660666
}
661667
postUninstallExtension(task.extension);
662668
} catch (e) {
663-
const error = e instanceof ExtensionManagementError ? e : new ExtensionManagementError(getErrorMessage(e), ExtensionManagementErrorCode.Internal);
669+
const error = toExtensionManagementError(e);
664670
postUninstallExtension(task.extension, error);
665671
throw error;
666672
} finally {
@@ -669,7 +675,7 @@ export abstract class AbstractExtensionManagementService extends Disposable impl
669675
}));
670676

671677
} catch (e) {
672-
const error = e instanceof ExtensionManagementError ? e : new ExtensionManagementError(getErrorMessage(e), ExtensionManagementErrorCode.Internal);
678+
const error = toExtensionManagementError(e);
673679
for (const task of allTasks) {
674680
// cancel the tasks
675681
try { task.cancel(); } catch (error) { /* ignore */ }
@@ -781,16 +787,6 @@ export abstract class AbstractExtensionManagementService extends Disposable impl
781787
protected abstract copyExtension(extension: ILocalExtension, fromProfileLocation: URI, toProfileLocation: URI, metadata?: Partial<Metadata>): Promise<ILocalExtension>;
782788
}
783789

784-
export function joinErrors(errorOrErrors: (Error | string) | (Array<Error | string>)): Error {
785-
const errors = Array.isArray(errorOrErrors) ? errorOrErrors : [errorOrErrors];
786-
if (errors.length === 1) {
787-
return errors[0] instanceof Error ? <Error>errors[0] : new Error(<string>errors[0]);
788-
}
789-
return errors.reduce<Error>((previousValue: Error, currentValue: Error | string) => {
790-
return new Error(`${previousValue.message}${previousValue.message ? ',' : ''}${currentValue instanceof Error ? currentValue.message : currentValue}`);
791-
}, new Error(''));
792-
}
793-
794790
export function toExtensionManagementError(error: Error): ExtensionManagementError {
795791
if (error instanceof ExtensionManagementError) {
796792
return error;
@@ -800,12 +796,12 @@ export function toExtensionManagementError(error: Error): ExtensionManagementErr
800796
e.stack = error.stack;
801797
return e;
802798
}
803-
const e = new ExtensionManagementError(error.message, ExtensionManagementErrorCode.Internal);
799+
const e = new ExtensionManagementError(error.message, isCancellationError(error) ? ExtensionManagementErrorCode.Cancelled : ExtensionManagementErrorCode.Internal);
804800
e.stack = error.stack;
805801
return e;
806802
}
807803

808-
function reportTelemetry(telemetryService: ITelemetryService, eventName: string, { extensionData, verificationStatus, duration, error, durationSinceUpdate }: { extensionData: any; verificationStatus?: ExtensionVerificationStatus; duration?: number; durationSinceUpdate?: number; error?: Error }): void {
804+
function reportTelemetry(telemetryService: ITelemetryService, eventName: string, { extensionData, verificationStatus, duration, error, durationSinceUpdate }: { extensionData: any; verificationStatus?: ExtensionVerificationStatus; duration?: number; durationSinceUpdate?: number; error?: ExtensionManagementError | ExtensionGalleryError }): void {
809805
let errorcode: string | undefined;
810806
let errorcodeDetail: string | undefined;
811807

@@ -822,13 +818,9 @@ function reportTelemetry(telemetryService: ITelemetryService, eventName: string,
822818
}
823819

824820
if (error) {
825-
if (error instanceof ExtensionManagementError || error instanceof ExtensionGalleryError) {
826-
errorcode = error.code;
827-
if (error.code === ExtensionManagementErrorCode.Signature) {
828-
errorcodeDetail = error.message;
829-
}
830-
} else {
831-
errorcode = ExtensionManagementErrorCode.Internal;
821+
errorcode = error.code;
822+
if (error.code === ExtensionManagementErrorCode.Signature) {
823+
errorcodeDetail = error.message;
832824
}
833825
}
834826

src/vs/platform/extensionManagement/common/extensionManagement.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,7 @@ export enum ExtensionManagementErrorCode {
426426
Signature = 'Signature',
427427
NotAllowed = 'NotAllowed',
428428
Gallery = 'Gallery',
429+
Cancelled = 'Cancelled',
429430
Unknown = 'Unknown',
430431
Internal = 'Internal',
431432
}

src/vs/platform/extensionManagement/node/extensionManagementService.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { extract, ExtractError, IFile, zip } from 'vs/base/node/zip';
2525
import * as nls from 'vs/nls';
2626
import { IDownloadService } from 'vs/platform/download/common/download';
2727
import { INativeEnvironmentService } from 'vs/platform/environment/common/environment';
28-
import { AbstractExtensionManagementService, AbstractExtensionTask, ExtensionVerificationStatus, IInstallExtensionTask, InstallExtensionTaskOptions, IUninstallExtensionTask, joinErrors, toExtensionManagementError, UninstallExtensionTaskOptions } from 'vs/platform/extensionManagement/common/abstractExtensionManagementService';
28+
import { AbstractExtensionManagementService, AbstractExtensionTask, ExtensionVerificationStatus, IInstallExtensionTask, InstallExtensionTaskOptions, IUninstallExtensionTask, toExtensionManagementError, UninstallExtensionTaskOptions } from 'vs/platform/extensionManagement/common/abstractExtensionManagementService';
2929
import {
3030
ExtensionManagementError, ExtensionManagementErrorCode, IExtensionGalleryService, IExtensionIdentifier, IExtensionManagementService, IGalleryExtension, ILocalExtension, InstallOperation,
3131
Metadata, InstallOptions,
@@ -980,7 +980,7 @@ export class InstallGalleryExtensionTask extends InstallExtensionTask {
980980
try {
981981
await getManifest(zipPath);
982982
} catch (error) {
983-
throw new ExtensionManagementError(joinErrors(error).message, ExtensionManagementErrorCode.Invalid);
983+
throw new ExtensionManagementError(error.message, ExtensionManagementErrorCode.Invalid);
984984
}
985985
}
986986

0 commit comments

Comments
 (0)