From f11e8a3c6987fa35d8cb982c2c785fcccd4b9e60 Mon Sep 17 00:00:00 2001
From: Vladimir Matveev <vladima@microsoft.com>
Date: Thu, 10 Nov 2016 16:52:36 -0800
Subject: [PATCH] add 'installSuccess' flag to telemetry, cache misses if npm
 install fails (#12163)

* add 'installSuccess' flag to telemetry, cache misses if npm install fails

* fix typo
---
 src/server/protocol.ts                           |  4 ++++
 src/server/server.ts                             |  3 ++-
 src/server/types.d.ts                            |  1 +
 .../typingsInstaller/nodeTypingsInstaller.ts     |  6 +++---
 src/server/typingsInstaller/typingsInstaller.ts  | 16 ++++++++++++----
 5 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/server/protocol.ts b/src/server/protocol.ts
index d13caf7f01..a112cd942b 100644
--- a/src/server/protocol.ts
+++ b/src/server/protocol.ts
@@ -2081,6 +2081,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 3b33554aab..c561277c7d 100644
--- a/src/server/server.ts
+++ b/src/server/server.ts
@@ -299,7 +299,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 aebc312125..3fd90e7fd3 100644
--- a/src/server/types.d.ts
+++ b/src/server/types.d.ts
@@ -68,6 +68,7 @@ declare namespace ts.server {
     export interface TypingsInstallEvent extends TypingInstallerResponse {
         readonly packagesToInstall: ReadonlyArray<string>;
         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 7020b6aa4f..74311dae4e 100644
--- a/src/server/typingsInstaller/nodeTypingsInstaller.ts
+++ b/src/server/typingsInstaller/nodeTypingsInstaller.ts
@@ -135,12 +135,12 @@ namespace ts.server.typingsInstaller {
             }
             const command = `${this.npmPath} install ${args.join(" ")} --save-dev`;
             const start = Date.now();
-            this.exec(command, { cwd }, (_err, stdout, stderr) => {
+            this.exec(command, { cwd }, (err, stdout, stderr) => {
                 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 absence of error as success
+                onRequestCompleted(!err);
             });
         }
     }
diff --git a/src/server/typingsInstaller/typingsInstaller.ts b/src/server/typingsInstaller/typingsInstaller.ts
index 0873d5c328..2ea0296e22 100644
--- a/src/server/typingsInstaller/typingsInstaller.ts
+++ b/src/server/typingsInstaller/typingsInstaller.ts
@@ -218,7 +218,7 @@ namespace ts.server.typingsInstaller {
             this.knownCachesSet[cacheLocation] = true;
         }
 
-        private filterAndMapToScopedName(typingsToInstall: string[]) {
+        private filterTypings(typingsToInstall: string[]) {
             if (typingsToInstall.length === 0) {
                 return typingsToInstall;
             }
@@ -230,7 +230,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()) {
@@ -286,7 +286,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`);
@@ -303,11 +304,18 @@ namespace ts.server.typingsInstaller {
                 if (this.telemetryEnabled) {
                     this.sendResponse(<TypingsInstallEvent>{
                         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;
                 }