From 70d471157c0429ddd4e556fb151e1ea535c925ba Mon Sep 17 00:00:00 2001 From: Dmitrii Bobreshev <106314398+DmitriiBobreshev@users.noreply.github.com> Date: Thu, 11 Apr 2024 18:47:35 +0200 Subject: [PATCH] Fix CodeQL fail tests (#1034) * Fix CodeQL fail tests - Fixed error when the spawn method couldn't execute * temporary enable codeql for all branches * Revert "temporary enable codeql for all branches" This reverts commit 057dfb6ec2b884a707f6439be9151df1299d6fa1. * Increase timeout for codeql --- node/package-lock.json | 2 +- node/package.json | 2 +- node/test/toolrunnertests.ts | 2 +- node/toolrunner.ts | 123 ++++++++++++++++++++++------------- 4 files changed, 80 insertions(+), 49 deletions(-) diff --git a/node/package-lock.json b/node/package-lock.json index 734f07841..98d9176b5 100644 --- a/node/package-lock.json +++ b/node/package-lock.json @@ -1,6 +1,6 @@ { "name": "azure-pipelines-task-lib", - "version": "4.10.1", + "version": "4.11.0", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/node/package.json b/node/package.json index b50b86fe1..e47a4c06b 100644 --- a/node/package.json +++ b/node/package.json @@ -1,6 +1,6 @@ { "name": "azure-pipelines-task-lib", - "version": "4.10.1", + "version": "4.11.0", "description": "Azure Pipelines Task SDK", "main": "./task.js", "typings": "./task.d.ts", diff --git a/node/test/toolrunnertests.ts b/node/test/toolrunnertests.ts index ac0e0eeb1..10e8f5f6c 100644 --- a/node/test/toolrunnertests.ts +++ b/node/test/toolrunnertests.ts @@ -608,7 +608,7 @@ describe('Toolrunner Tests', function () { }); }) it('Exec pipe output to another tool, succeeds if both tools succeed', function (done) { - this.timeout(30000); + this.timeout(120000); var _testExecOptions = { cwd: __dirname, diff --git a/node/toolrunner.ts b/node/toolrunner.ts index 9705e0f45..48dcc2b73 100644 --- a/node/toolrunner.ts +++ b/node/toolrunner.ts @@ -1098,7 +1098,45 @@ export class ToolRunner extends events.EventEmitter { this._debug(message); }); - let cp = child.spawn(this._getSpawnFileName(options), this._getSpawnArgs(optionsNonNull), this._getSpawnOptions(options)); + var stdbuffer: string = ''; + var errbuffer: string = ''; + const emitDoneEvent = function (resolve, reject) { + state.on('done', (error: Error, exitCode: number) => { + if (stdbuffer.length > 0) { + this.emit('stdline', stdbuffer); + } + + if (errbuffer.length > 0) { + this.emit('errline', errbuffer); + } + + if (cp) { + cp.removeAllListeners(); + } + + if (error) { + reject(error); + } + else { + resolve(exitCode); + } + }); + } + + // Edge case when the node itself cant's spawn and emit event + let cp; + try { + cp = child.spawn(this._getSpawnFileName(options), this._getSpawnArgs(optionsNonNull), this._getSpawnOptions(options)); + } catch (error) { + return new Promise((resolve, reject) => { + emitDoneEvent(resolve, reject); + state.processError = error.message; + state.processExited = true; + state.processClosed = true; + state.CheckComplete(); + }); + } + this.childProcess = cp; // it is possible for the child process to end its last line without a new line. // because stdout is buffered, this causes the last line to not get sent to the parent @@ -1109,7 +1147,6 @@ export class ToolRunner extends events.EventEmitter { } }); - var stdbuffer: string = ''; cp.stdout?.on('data', (data: Buffer) => { this.emit('stdout', data); @@ -1122,8 +1159,6 @@ export class ToolRunner extends events.EventEmitter { }); }); - - var errbuffer: string = ''; cp.stderr?.on('data', (data: Buffer) => { state.processStderr = true; this.emit('stderr', data); @@ -1160,26 +1195,7 @@ export class ToolRunner extends events.EventEmitter { state.CheckComplete(); }); - return new Promise((resolve, reject) => { - state.on('done', (error: Error, exitCode: number) => { - if (stdbuffer.length > 0) { - this.emit('stdline', stdbuffer); - } - - if (errbuffer.length > 0) { - this.emit('errline', errbuffer); - } - - cp.removeAllListeners(); - - if (error) { - reject(error); - } - else { - resolve(exitCode); - } - }); - }); + return new Promise(emitDoneEvent); } /** @@ -1215,7 +1231,43 @@ export class ToolRunner extends events.EventEmitter { this._debug(message); }); - let cp = child.spawn(this._getSpawnFileName(options), this._getSpawnArgs(optionsNonNull), this._getSpawnOptions(options)); + var stdbuffer: string = ''; + var errbuffer: string = ''; + state.on('done', (error: Error, exitCode: number) => { + if (stdbuffer.length > 0) { + this.emit('stdline', stdbuffer); + } + + if (errbuffer.length > 0) { + this.emit('errline', errbuffer); + } + + if (cp) { + cp.removeAllListeners(); + } + + if (error) { + defer.reject(error); + } + else { + defer.resolve(exitCode); + } + }); + + + // Edge case when the node itself cant's spawn and emit event + let cp; + try { + cp = child.spawn(this._getSpawnFileName(options), this._getSpawnArgs(optionsNonNull), this._getSpawnOptions(options)); + } catch (error) { + state.processError = error.message; + state.processExited = true; + state.processClosed = true; + state.CheckComplete(); + + return defer.promise; + } + this.childProcess = cp; // it is possible for the child process to end its last line without a new line. // because stdout is buffered, this causes the last line to not get sent to the parent @@ -1226,7 +1278,6 @@ export class ToolRunner extends events.EventEmitter { } }); - var stdbuffer: string = ''; cp.stdout?.on('data', (data: Buffer) => { this.emit('stdout', data); @@ -1240,7 +1291,6 @@ export class ToolRunner extends events.EventEmitter { }); - var errbuffer: string = ''; cp.stderr?.on('data', (data: Buffer) => { state.processStderr = true; this.emit('stderr', data); @@ -1277,25 +1327,6 @@ export class ToolRunner extends events.EventEmitter { state.CheckComplete(); }); - state.on('done', (error: Error, exitCode: number) => { - if (stdbuffer.length > 0) { - this.emit('stdline', stdbuffer); - } - - if (errbuffer.length > 0) { - this.emit('errline', errbuffer); - } - - cp.removeAllListeners(); - - if (error) { - defer.reject(error); - } - else { - defer.resolve(exitCode); - } - }); - return defer.promise; }