Skip to content

Commit

Permalink
add 'installSuccess' flag to telemetry, cache misses if npm install f…
Browse files Browse the repository at this point in the history
…ails (#12163)

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

* fix typo
  • Loading branch information
vladima authored and mhegazy committed Nov 11, 2016
1 parent cc81fa7 commit 09f29ad
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 7 deletions.
4 changes: 4 additions & 0 deletions src/server/protocol.ts
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion src/server/server.ts
Expand Up @@ -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";
Expand Down
1 change: 1 addition & 0 deletions src/server/types.d.ts
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/server/typingsInstaller/nodeTypingsInstaller.ts
Expand Up @@ -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);
});
}
}
Expand Down
16 changes: 12 additions & 4 deletions src/server/typingsInstaller/typingsInstaller.ts
Expand Up @@ -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;
}
Expand All @@ -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()) {
Expand Down Expand Up @@ -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`);
Expand All @@ -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;
}

Expand Down

0 comments on commit 09f29ad

Please sign in to comment.