From b8034fa1b2b356503be75ede75f5c5dd0bdadac7 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 10 Nov 2016 16:21:49 -0800 Subject: [PATCH 1/2] add 'installSuccess' flag to telemetry, cache misses if npm install fails --- src/server/protocol.ts | 4 ++++ src/server/server.ts | 3 ++- src/server/types.d.ts | 1 + .../typingsInstaller/nodeTypingsInstaller.ts | 4 ++-- src/server/typingsInstaller/typingsInstaller.ts | 16 ++++++++++++---- 5 files changed, 21 insertions(+), 7 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 986e030962f6f..638abfa588032 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1964,6 +1964,10 @@ namespace ts.server.protocol { * Comma separated list of installed typing packages */ installedPackages: string; + /** + * true if install request succeeded, otherwise - false + */ + installSuccess: boolean; } export interface NavBarResponse extends Response { diff --git a/src/server/server.ts b/src/server/server.ts index 30c9a157efe6e..9f74c5c912ba2 100644 --- a/src/server/server.ts +++ b/src/server/server.ts @@ -265,7 +265,8 @@ namespace ts.server { const body: protocol.TypingsInstalledTelemetryEventBody = { telemetryEventName: "typingsInstalled", payload: { - installedPackages: response.packagesToInstall.join(",") + installedPackages: response.packagesToInstall.join(","), + installSuccess: response.installSuccess } }; const eventName: protocol.TelemetryEventName = "telemetry"; diff --git a/src/server/types.d.ts b/src/server/types.d.ts index e6b93ca6ac86c..9227364e17ea4 100644 --- a/src/server/types.d.ts +++ b/src/server/types.d.ts @@ -62,6 +62,7 @@ declare namespace ts.server { export interface TypingsInstallEvent extends TypingInstallerResponse { readonly packagesToInstall: ReadonlyArray; readonly kind: EventInstall; + readonly installSuccess: boolean; } export interface InstallTypingHost extends JsTyping.TypingResolutionHost { diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index 2e55a2003f767..aaae78e8102fc 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -139,8 +139,8 @@ namespace ts.server.typingsInstaller { if (this.log.isEnabled()) { this.log.writeLine(`npm install #${requestId} took: ${Date.now() - start} ms${sys.newLine}stdout: ${stdout}${sys.newLine}stderr: ${stderr}`); } - // treat any output on stdout as success - onRequestCompleted(!!stdout); + // treat absense of error as success + onRequestCompleted(!err); }); } } diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts index ad346448b61c9..f95829e88fb9d 100644 --- a/src/server/typingsInstaller/typingsInstaller.ts +++ b/src/server/typingsInstaller/typingsInstaller.ts @@ -215,7 +215,7 @@ namespace ts.server.typingsInstaller { this.knownCachesSet[cacheLocation] = true; } - private filterAndMapToScopedName(typingsToInstall: string[]) { + private filterTypings(typingsToInstall: string[]) { if (typingsToInstall.length === 0) { return typingsToInstall; } @@ -227,7 +227,7 @@ namespace ts.server.typingsInstaller { const validationResult = validatePackageName(typing); if (validationResult === PackageNameValidationResult.Ok) { if (typing in this.typesRegistry) { - result.push(`@types/${typing}`); + result.push(typing); } else { if (this.log.isEnabled()) { @@ -280,7 +280,8 @@ namespace ts.server.typingsInstaller { if (this.log.isEnabled()) { this.log.writeLine(`Installing typings ${JSON.stringify(typingsToInstall)}`); } - const scopedTypings = this.filterAndMapToScopedName(typingsToInstall); + const filteredTypings = this.filterTypings(typingsToInstall); + const scopedTypings = filteredTypings.map(x => `@types/${x}`); if (scopedTypings.length === 0) { if (this.log.isEnabled()) { this.log.writeLine(`All typings are known to be missing or invalid - no need to go any further`); @@ -297,11 +298,18 @@ namespace ts.server.typingsInstaller { if (this.telemetryEnabled) { this.sendResponse({ kind: EventInstall, - packagesToInstall: scopedTypings + packagesToInstall: scopedTypings, + installSuccess: ok }); } if (!ok) { + if (this.log.isEnabled()) { + this.log.writeLine(`install request failed, marking packages as missing to prevent repeated requests: ${JSON.stringify(filteredTypings)}`); + } + for (const typing of filteredTypings) { + this.missingTypingsSet[typing] = true; + } return; } From eac7c576beb2802c05b8b5a0949eaea445cb6264 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 10 Nov 2016 16:24:05 -0800 Subject: [PATCH 2/2] fix typo --- src/server/typingsInstaller/nodeTypingsInstaller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/typingsInstaller/nodeTypingsInstaller.ts b/src/server/typingsInstaller/nodeTypingsInstaller.ts index aaae78e8102fc..20c022a227210 100644 --- a/src/server/typingsInstaller/nodeTypingsInstaller.ts +++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts @@ -139,7 +139,7 @@ namespace ts.server.typingsInstaller { if (this.log.isEnabled()) { this.log.writeLine(`npm install #${requestId} took: ${Date.now() - start} ms${sys.newLine}stdout: ${stdout}${sys.newLine}stderr: ${stderr}`); } - // treat absense of error as success + // treat absence of error as success onRequestCompleted(!err); }); }