microsoft/azure-pipelines-task-lib

ToolRunner does not properly buffer the `stdline` and `errline`.

EvanCahill opened this issue · 3 comments

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

Pull Request for this issue: #1030

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

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