Skip to content

Commit 917ae32

Browse files
author
Andy
authored
Always log output of execSync (#19110)
* Always log output of execSync * Fix lint
1 parent 7e1dd66 commit 917ae32

File tree

1 file changed

+33
-16
lines changed

1 file changed

+33
-16
lines changed

src/server/typingsInstaller/nodeTypingsInstaller.ts

+33-16
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,14 @@ namespace ts.server.typingsInstaller {
6868
return combinePaths(normalizeSlashes(globalTypingsCacheLocation), `node_modules/${TypesRegistryPackageName}/index.json`);
6969
}
7070

71-
type ExecSync = (command: string, options: { cwd: string, stdio?: "ignore" }) => any;
71+
interface ExecSyncOptions {
72+
cwd: string;
73+
encoding: "utf-8";
74+
}
75+
type ExecSync = (command: string, options: ExecSyncOptions) => string;
7276

7377
export class NodeTypingsInstaller extends TypingsInstaller {
74-
private readonly execSync: ExecSync;
78+
private readonly nodeExecSync: ExecSync;
7579
private readonly npmPath: string;
7680
readonly typesRegistry: Map<void>;
7781

@@ -95,15 +99,15 @@ namespace ts.server.typingsInstaller {
9599
this.log.writeLine(`Process id: ${process.pid}`);
96100
this.log.writeLine(`NPM location: ${this.npmPath} (explicit '${Arguments.NpmLocation}' ${npmLocation === undefined ? "not " : ""} provided)`);
97101
}
98-
({ execSync: this.execSync } = require("child_process"));
102+
({ execSync: this.nodeExecSync } = require("child_process"));
99103

100104
this.ensurePackageDirectoryExists(globalTypingsCacheLocation);
101105

102106
try {
103107
if (this.log.isEnabled()) {
104108
this.log.writeLine(`Updating ${TypesRegistryPackageName} npm package...`);
105109
}
106-
this.execSync(`${this.npmPath} install --ignore-scripts ${TypesRegistryPackageName}`, { cwd: globalTypingsCacheLocation, stdio: "ignore" });
110+
this.execSyncAndLog(`${this.npmPath} install --ignore-scripts ${TypesRegistryPackageName}`, { cwd: globalTypingsCacheLocation });
107111
if (this.log.isEnabled()) {
108112
this.log.writeLine(`Updated ${TypesRegistryPackageName} npm package`);
109113
}
@@ -155,22 +159,31 @@ namespace ts.server.typingsInstaller {
155159
}
156160
const command = `${this.npmPath} install --ignore-scripts ${args.join(" ")} --save-dev --user-agent="typesInstaller/${version}"`;
157161
const start = Date.now();
158-
let stdout: Buffer;
159-
let stderr: Buffer;
160-
let hasError = false;
161-
try {
162-
stdout = this.execSync(command, { cwd });
163-
}
164-
catch (e) {
165-
stdout = e.stdout;
166-
stderr = e.stderr;
167-
hasError = true;
168-
}
162+
const hasError = this.execSyncAndLog(command, { cwd });
169163
if (this.log.isEnabled()) {
170-
this.log.writeLine(`npm install #${requestId} took: ${Date.now() - start} ms${sys.newLine}stdout: ${stdout && stdout.toString()}${sys.newLine}stderr: ${stderr && stderr.toString()}`);
164+
this.log.writeLine(`npm install #${requestId} took: ${Date.now() - start} ms`);
171165
}
172166
onRequestCompleted(!hasError);
173167
}
168+
169+
/** Returns 'true' in case of error. */
170+
private execSyncAndLog(command: string, options: Pick<ExecSyncOptions, "cwd">): boolean {
171+
if (this.log.isEnabled()) {
172+
this.log.writeLine(`Exec: ${command}`);
173+
}
174+
try {
175+
const stdout = this.nodeExecSync(command, { ...options, encoding: "utf-8" });
176+
if (this.log.isEnabled()) {
177+
this.log.writeLine(` Succeeded. stdout:${indent(sys.newLine, stdout)}`);
178+
}
179+
return false;
180+
}
181+
catch (error) {
182+
const { stdout, stderr } = error;
183+
this.log.writeLine(` Failed. stdout:${indent(sys.newLine, stdout)}${sys.newLine} stderr:${indent(sys.newLine, stderr)}`);
184+
return true;
185+
}
186+
}
174187
}
175188

176189
const logFilePath = findArgument(server.Arguments.LogFile);
@@ -193,4 +206,8 @@ namespace ts.server.typingsInstaller {
193206
});
194207
const installer = new NodeTypingsInstaller(globalTypingsCacheLocation, typingSafeListLocation, typesMapLocation, npmLocation, /*throttleLimit*/5, log);
195208
installer.listen();
209+
210+
function indent(newline: string, string: string): string {
211+
return `${newline} ` + string.replace(/\r?\n/, `${newline} `);
212+
}
196213
}

0 commit comments

Comments
 (0)