Skip to content

Commit bace817

Browse files
vrubezhnyvrubezhny
committed
Getting rid of the need to configure stdout maxBuffer length
No more maxBuffer limit, no preference for it is required Signed-off-by: Victor Rubezhny <[email protected]>
1 parent b5f0cbe commit bace817

File tree

6 files changed

+76
-62
lines changed

6 files changed

+76
-62
lines changed

‎package.json‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2223,11 +2223,6 @@
22232223
"default": 90000,
22242224
"description": "Controls stopping development mode timeout (in milliseconds)."
22252225
},
2226-
"openshiftToolkit.execMaxBufferLength": {
2227-
"type": "number",
2228-
"default": 4,
2229-
"description": "MiB of memory to allocate for stdout buffer when executing a binary"
2230-
},
22312226
"openshiftToolkit.disable-context-info-status-bar": {
22322227
"type": "boolean",
22332228
"description": "Disable displaying the current Kubernetes context in VS Code's status bar."

‎src/cli.ts‎

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
import { VSCodeSettings } from '@redhat-developer/vscode-redhat-telemetry/lib/common/vscode/settings';
77
import * as cp from 'child_process';
8-
import * as vscode from 'vscode';
98
import { CommandText } from './base/command';
109
import { ToolsConfig } from './tools';
1110
import { ChildProcessUtil, CliExitData } from './util/childProcessUtil';
@@ -77,14 +76,6 @@ export class CliChannel {
7776
}
7877

7978
if (result.error && fail) {
80-
if (result.error.code && result.error.code.toString() === 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER') {
81-
void vscode.window.showErrorMessage('Do you want to change the maximum \'stdout\' buffer size by modifying the \'openshiftToolkit.execMaxBufferLength\' preference value?', 'Yes', 'Cancel')
82-
.then((answer)=> {
83-
if (answer === 'Yes') {
84-
void vscode.commands.executeCommand('workbench.action.openSettings', 'openshiftToolkit.execMaxBufferLength');
85-
}
86-
});
87-
}
8879
throw new VsCommandError(`${result.error.message}`, `Error when running command: ${commandPrivacy}`, result.error);
8980
};
9081
return result;
@@ -101,14 +92,6 @@ export class CliChannel {
10192
toolLocation ? commandActual.replace(cmd, `"${toolLocation}"`) : commandActual, optsCopy, stdout);
10293

10394
if (result.error && fail) {
104-
if (result.error.code && result.error.code.toString() === 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER') {
105-
void vscode.window.showErrorMessage('Do you want to change the maximum \'stdout\' buffer size by modifying the \'openshiftToolkit.execMaxBufferLength\' preference value?', 'Yes', 'Cancel')
106-
.then((answer)=> {
107-
if (answer === 'Yes') {
108-
void vscode.commands.executeCommand('workbench.action.openSettings', 'openshiftToolkit.execMaxBufferLength');
109-
}
110-
});
111-
}
11295
throw new VsCommandError(`${result.error.message}`, `Error when running command: ${commandPrivacy}`, result.error);
11396
};
11497
return result;

‎src/util/childProcessUtil.ts‎

Lines changed: 23 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -98,27 +98,34 @@ export class ChildProcessUtil {
9898
this.odoChannel = odoChannel;
9999
}
100100

101-
public static getExecMaxBufferLength(): number {
102-
const execMaxBufferLengthFromSetting: string = vscode.workspace.getConfiguration('openshiftToolkit')
103-
.get('execMaxBufferLength');
104-
const length = parseInt(execMaxBufferLengthFromSetting, 10);
105-
return (!isNaN(length) && length > 0 ? length : 4) * 1024 * 1024;
106-
}
107-
108101
public execute(cmd: string, opts: ExecOptions = {}, outText?: string): Promise<CliExitData> {
109102
return new Promise<CliExitData>((resolve) => {
110-
if (opts.maxBuffer === undefined) {
111-
opts.maxBuffer = ChildProcessUtil.getExecMaxBufferLength();
112-
}
113-
const childProcess = cp.exec(cmd, opts, (error: ExecException, stdout: string, stderr: string) => {
114-
// filter out info about update
103+
const childProcess = cp.spawn(cmd, { ...opts, shell: true });
104+
105+
let stdout = '';
106+
let stderr = '';
107+
108+
childProcess.stdout.on('data', (data) => {
109+
stdout += data.toString();
110+
});
111+
112+
childProcess.stderr.on('data', (data) => {
113+
stderr += data.toString();
114+
});
115+
116+
childProcess.on('error', (error) => {
117+
this.odoChannel.print(cmd);
118+
this.odoChannel.print(stdout);
119+
this.odoChannel.print(stderr);
120+
resolve({ error, stdout: stdout.trim(), stderr: stderr.trim(), cwd: opts.cwd?.toString() });
121+
});
122+
123+
childProcess.on('close', (code) => {
115124
this.odoChannel.print(cmd);
116125
this.odoChannel.print(stdout);
117126
this.odoChannel.print(stderr);
118-
// do not reject it here, because caller in some cases need the error and the streams
119-
// to make a decision
120-
// Filter update message text which starts with `---`
121-
resolve({ error, stdout: stdout.trim(), stderr: stderr.trim(), cwd: opts?.cwd?.toString() });
127+
const error = code !== 0 ? new Error(`Exited with code ${code}`) : undefined;
128+
resolve({ error, stdout: stdout.trim(), stderr: stderr.trim(), cwd: opts.cwd?.toString() });
122129
});
123130

124131
if (childProcess && outText) {

‎src/util/utils.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See LICENSE file in the project root for license information.
44
*-----------------------------------------------------------------------------------------------*/
55

6-
import { exec as execOriginal } from 'child_process';
6+
import { exec as execOriginal, spawn as spawnOriginal } from 'child_process';
77
import { createHash } from 'crypto';
88
import {
99
createReadStream,
@@ -76,6 +76,7 @@ export const Util = {
7676

7777
// Wraps from child_process
7878
exec: execOriginal,
79+
spawn: spawnOriginal,
7980

8081
// Wraps from child_process
8182
decompress: decompressOriginal

‎test/fixtures/components/empty.code-workspace‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
{
22
"folders": [],
33
"settings": {
4-
"openshiftToolkit.execMaxBufferLength": 2,
54
"redhat.telemetry.enabled": false
65
},
76
}

‎test/unit/util/childProcessUtil.test.ts‎

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
*-----------------------------------------------------------------------------------------------*/
55

66
import * as chai from 'chai';
7-
import { ExecException, ExecOptions } from 'child_process';
7+
import { ExecOptions } from 'child_process';
88
import * as sinon from 'sinon';
99
import sinonChai from 'sinon-chai';
10-
import * as vscode from 'vscode';
10+
import { EventEmitter, PassThrough } from 'stream';
1111
import { ChildProcessUtil } from '../../../src/util/childProcessUtil';
1212
import { Util as childProcess } from '../../../src/util/utils';
1313

@@ -16,48 +16,77 @@ chai.use(sinonChai);
1616

1717
suite('ChildProcessUtil', function() {
1818
let sandbox: sinon.SinonSandbox;
19-
let execStub: sinon.SinonStub;
20-
const childProcessUtil = ChildProcessUtil.Instance;
19+
let spawnStub: sinon.SinonStub;
2120
const command = 'command';
2221
const options: ExecOptions = { cwd: 'cwd' };
2322
const stdout = 'Standard output';
2423
const stderr = 'Error output';
25-
const error: ExecException = {
26-
message: 'Fatal Error',
27-
name: 'name'
28-
};
2924

3025
setup(function() {
3126
sandbox = sinon.createSandbox();
32-
execStub = sandbox.stub(childProcess, 'exec');
3327
});
3428

3529
teardown(function() {
3630
sandbox.restore();
3731
});
3832

33+
function createFakeProcess() {
34+
const fake: any = new EventEmitter();
35+
fake.stdout = new PassThrough();
36+
fake.stderr = new PassThrough();
37+
fake.stdin = new PassThrough();
38+
return fake;
39+
}
40+
3941
test('execute runs the given command from shell', async function() {
40-
execStub.yields(null, stdout, '');
41-
const result = await childProcessUtil.execute(command, options);
42+
const fakeProcess: any = createFakeProcess();
43+
spawnStub = sandbox.stub(childProcess, 'spawn').returns(fakeProcess);
4244

43-
expect(execStub).calledWithExactly(command, options, sinon.match.func);
44-
expect(result).deep.equals({ error: null, stdout, stderr: '', cwd: 'cwd' });
45-
});
45+
// create util AFTER stubbing
46+
const childProcessUtil = ChildProcessUtil.Instance;
4647

47-
test('execute uses a 2MB buffer by default', async function() {
48-
// Change Exec Max Buffer Length to 2Mb (while the default value is 4Gb)
49-
await vscode.workspace.getConfiguration('openshiftToolkit').update('execMaxBufferLength', 2);
48+
// simulate normal completion
49+
setImmediate(() => {
50+
fakeProcess.stdout.emit('data', stdout);
51+
fakeProcess.stderr.emit('data', '');
52+
fakeProcess.emit('close', 0);
53+
});
54+
55+
const result = await childProcessUtil.execute(command, options);
5056

51-
execStub.yields(null, stdout, '');
52-
await childProcessUtil.execute(command);
57+
sinon.assert.calledOnce(spawnStub);
58+
sinon.assert.calledWith(spawnStub, command, sinon.match({ shell: true, cwd: 'cwd' }));
5359

54-
expect(execStub).calledOnceWith(command, { maxBuffer: 2*1024*1024 }, sinon.match.func);
60+
expect(result).deep.equals({
61+
error: undefined,
62+
stdout: 'Standard output',
63+
stderr: '',
64+
cwd: 'cwd'
65+
});
5566
});
5667

5768
test('execute passes errors into its exit data', async function() {
58-
execStub.yields(error, stdout, stderr);
69+
const fakeProcess: any = createFakeProcess();
70+
spawnStub = sandbox.stub(childProcess, 'spawn').returns(fakeProcess);
71+
72+
// create util AFTER stubbing
73+
const childProcessUtil = ChildProcessUtil.Instance;
74+
75+
// simulate error output + non-zero exit
76+
setTimeout(() => {
77+
fakeProcess.stdout.write(stdout);
78+
fakeProcess.stderr.write(stderr);
79+
fakeProcess.emit('close', 1);
80+
}, 0);
81+
5982
const result = await childProcessUtil.execute(command);
6083

61-
expect(result).deep.equals({ error, stdout, stderr , cwd: undefined});
84+
sinon.assert.calledOnce(spawnStub);
85+
sinon.assert.calledWith(spawnStub, command, sinon.match({ shell: true }));
86+
87+
expect(result.error).to.be.instanceOf(Error).and.have.property('message', 'Exited with code 1');
88+
expect(result.stdout).to.equal(stdout);
89+
expect(result.stderr).to.equal(stderr);
90+
expect(result.cwd).to.be.undefined;
6291
});
6392
});

0 commit comments

Comments
 (0)