add 'installSuccess' flag to telemetry, cache misses if npm install fails (#12163)

vladima · mhegazy · commit 09f29adde7f3 · 2016-11-10T16:52:36.000-08:00
* add 'installSuccess' flag to telemetry, cache misses if npm install fails

* fix typo
diff --git 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
@@ -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
@@ -62,6 +62,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
@@ -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 absence of error as success
+                onRequestCompleted(!err);
             });
         }
     }
diff --git 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(<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;
                 }
 

Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,7 @@ declare namespace ts.server {`
`62`	`62`	`export interface TypingsInstallEvent extends TypingInstallerResponse {`
`63`	`63`	`readonly packagesToInstall: ReadonlyArray<string>;`
`64`	`64`	`readonly kind: EventInstall;`
	`65`	`+ readonly installSuccess: boolean;`
`65`	`66`	`}`
`66`	`67`
`67`	`68`	`export interface InstallTypingHost extends JsTyping.TypingResolutionHost {`
Original file line number	Diff line number	Diff line change
`@@ -139,8 +139,8 @@ namespace ts.server.typingsInstaller {`
`139`	`139`	`if (this.log.isEnabled()) {`
`140`	`140`	this.log.writeLine(`npm install #${requestId} took: ${Date.now() - start} ms${sys.newLine}stdout: ${stdout}${sys.newLine}stderr: ${stderr}`);
`141`	`141`	`}`
`142`		`- // treat any output on stdout as success`
`143`		`- onRequestCompleted(!!stdout);`
	`142`	`+ // treat absence of error as success`
	`143`	`+ onRequestCompleted(!err);`
`144`	`144`	`});`
`145`	`145`	`}`
`146`	`146`	`}`