Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ToolRunner does not properly buffer the stdline and errline. #1032

Closed
EvanCahill opened this issue Apr 2, 2024 · 3 comments
Closed

ToolRunner does not properly buffer the stdline and errline. #1032

EvanCahill opened this issue Apr 2, 2024 · 3 comments

Comments

@EvanCahill
Copy link
Member

Environment

azure-pipelines-task-lib version: 4.10.1

Issue Description

The toolrunner does not properly buffer the stdline and errline. Specifically, when the toolrunner receives 'stdout' or 'stderr' data from the child process, it discards all the data after the last new line character.

This has implications for the AzureCLI@2 task which is listening to the errline event to implements its failOnStandardError functionality. When the standard error stream does not contain any newlines (or the wrong type e.g. lf endings on Windows) the errline event never fires due to the incorrect buffering.

Expected behaviour

The toolrunner should correctly buffer all of the stdout and stderr data and emit stdline and errline events for all lines of data.

Actual behaviour

The stdline and errline are missing data.

Steps to reproduce

Here is an example script to reproduce the issue:

// bufferedoutput.js
var os = require('os');
var stdout = process.stdout;
var stderr = process.stderr;

stdout.write('stdline 1' + os.EOL, function () {
    stdout.write('stdline 2', function () {
        stdout.write(os.EOL + 'stdline 3');
    });
});

stderr.write('errline 1' + os.EOL, function () {
    stderr.write('errline 2', function () {
        stderr.write(os.EOL + 'errline 3');
    });
});
// test.js
var assert = require('assert');
var scriptPath = path.join(__dirname, 'bufferedoutput.js');
var node = tl.tool(tl.which('node', true));
node.arg(scriptPath);

let stdlines = [];
let errlines = [];

node.on('stdline', function (line) {
    stdlines.push(line);
});

node.on('errline', function (line) {
    errlines.push(line);
});

node.exec({
    cwd: __dirname,
    env: {},
    silent: false,
    failOnStdErr: false,
    ignoreReturnCode: false,
    outStream: testutil.getNullStream(),
    errStream: testutil.getNullStream()
})
.then(function (code) {
    assert.equal(code, 0, 'return code of cmd should be 0');
    assert.deepStrictEqual(stdlines, ['stdline 1', 'stdline 2', 'stdline 3'], 'should have emitted stdlines');
    assert.deepStrictEqual(errlines, ['errline 1', 'errline 2', 'errline 3'], 'should have emitted errlines');
});

Logs

N/A

@EvanCahill
Copy link
Member Author

Pull Request for this issue: #1030

Copy link

This issue has had no activity in 90 days. Please comment if it is not actually stale

@KonstantinTyukalov
Copy link
Contributor

Fix was merged and published under the 4.17.2 version.
@EvanCahill thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants