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

vladima · vladima · commit f11e8a3c6987 · 2016-11-14T14:10:22.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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
                 }
 

Original file line number	Diff line number	Diff line change
`@@ -68,6 +68,7 @@ declare namespace ts.server {`
`68`	`68`	`export interface TypingsInstallEvent extends TypingInstallerResponse {`
`69`	`69`	`readonly packagesToInstall: ReadonlyArray<string>;`
`70`	`70`	`readonly kind: EventInstall;`
	`71`	`+ readonly installSuccess: boolean;`
`71`	`72`	`}`
`72`	`73`
`73`	`74`	`export interface InstallTypingHost extends JsTyping.TypingResolutionHost {`
Original file line number	Diff line number	Diff line change
`@@ -135,12 +135,12 @@ namespace ts.server.typingsInstaller {`
`135`	`135`	`}`
`136`	`136`	const command = `${this.npmPath} install ${args.join(" ")} --save-dev`;
`137`	`137`	`const start = Date.now();`
`138`		`- this.exec(command, { cwd }, (_err, stdout, stderr) => {`
	`138`	`+ this.exec(command, { cwd }, (err, stdout, stderr) => {`
`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`	`}`