From c132bfcb15c3446b9cc2f8e6df67d98929f472b6 Mon Sep 17 00:00:00 2001 From: Aditya Samantaray Date: Thu, 22 Aug 2024 21:33:44 +0530 Subject: [PATCH 1/5] fix: use unzipper as fallback for decompress --- bin/helpers/buildArtifacts.js | 26 +++++++++++++++++++------- bin/helpers/constants.js | 5 +++++ package.json | 3 ++- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/bin/helpers/buildArtifacts.js b/bin/helpers/buildArtifacts.js index e4aacd6b..44b004e2 100644 --- a/bin/helpers/buildArtifacts.js +++ b/bin/helpers/buildArtifacts.js @@ -10,7 +10,7 @@ const logger = require('./logger').winstonLogger, const request = require('request'); const decompress = require('decompress'); - +const unzipper = require("unzipper"); let BUILD_ARTIFACTS_TOTAL_COUNT = 0; let BUILD_ARTIFACTS_FAIL_COUNT = 0; @@ -136,13 +136,25 @@ const downloadAndUnzip = async (filePath, fileName, url) => { const unzipFile = async (filePath, fileName) => { return new Promise( async (resolve, reject) => { - await decompress(path.join(filePath, fileName), filePath) - .then((files) => { + try { + await decompress(path.join(filePath, fileName), filePath) resolve(); - }) - .catch((error) => { - reject(error); - }); + } catch (error) { + logger.debug(`Error unzipping with decompress: ${error}, trying with unzipper.`); + try { + fs.createReadStream(path.join(filePath, fileName)) + .pipe(unzipper.Extract({ path: filePath })) + .on("close", () => { + resolve(); + }) + .on("error", (err) => { + reject(err); + }); + } catch (unzipperError) { + logger.debug(`Unzip unsuccessful with unzipper`); + } + reject(Constants.debugMessages.BUILD_ARTIFACTS_UNZIP_FAILURE); + } }); } diff --git a/bin/helpers/constants.js b/bin/helpers/constants.js index deba24db..94d98519 100644 --- a/bin/helpers/constants.js +++ b/bin/helpers/constants.js @@ -271,6 +271,10 @@ const cliMessages = { }, }; +const debugMessages = { + BUILD_ARTIFACTS_UNZIP_FAILURE: "Failed to unzip build artifacts", +}; + const messageTypes = { SUCCESS: "success", ERROR: "error", @@ -455,6 +459,7 @@ module.exports = Object.freeze({ syncCLI, userMessages, cliMessages, + debugMessages, validationMessages, messageTypes, allowedFileTypes, diff --git a/package.json b/package.json index 01975ee0..e8895ffc 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,8 @@ "windows-release": "^5.1.0", "winston": "2.4.4", "yargs": "14.2.3", - "decompress": "4.2.1" + "decompress": "4.2.1", + "unzipper": "^0.12.3" }, "repository": { "type": "git", From 63e96025da54d313d17d3903a1086f6141c59e8a Mon Sep 17 00:00:00 2001 From: Aditya Samantaray Date: Fri, 23 Aug 2024 02:37:52 +0530 Subject: [PATCH 2/5] chore: add debug log lines for unzipping build artifacts --- bin/helpers/buildArtifacts.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/helpers/buildArtifacts.js b/bin/helpers/buildArtifacts.js index 44b004e2..0388e04b 100644 --- a/bin/helpers/buildArtifacts.js +++ b/bin/helpers/buildArtifacts.js @@ -137,10 +137,10 @@ const downloadAndUnzip = async (filePath, fileName, url) => { const unzipFile = async (filePath, fileName) => { return new Promise( async (resolve, reject) => { try { - await decompress(path.join(filePath, fileName), filePath) + await decompress(path.join(filePath, fileName), filePath); resolve(); } catch (error) { - logger.debug(`Error unzipping with decompress: ${error}, trying with unzipper.`); + logger.debug(`Error unzipping with decompress, trying with unzipper. Stacktrace: ${error}.`); try { fs.createReadStream(path.join(filePath, fileName)) .pipe(unzipper.Extract({ path: filePath })) @@ -151,9 +151,9 @@ const unzipFile = async (filePath, fileName) => { reject(err); }); } catch (unzipperError) { - logger.debug(`Unzip unsuccessful with unzipper`); + logger.debug(`Unzipper package error: ${unzipperError}`); + reject(Constants.debugMessages.BUILD_ARTIFACTS_UNZIP_FAILURE); } - reject(Constants.debugMessages.BUILD_ARTIFACTS_UNZIP_FAILURE); } }); } From dd7ceb2abf2a9f9e49d9751c9ffdc21499c7d956 Mon Sep 17 00:00:00 2001 From: Aditya Samantaray Date: Fri, 23 Aug 2024 02:57:03 +0530 Subject: [PATCH 3/5] fix: handle promise rejections within stream CB --- bin/helpers/buildArtifacts.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/bin/helpers/buildArtifacts.js b/bin/helpers/buildArtifacts.js index 0388e04b..b2bc3385 100644 --- a/bin/helpers/buildArtifacts.js +++ b/bin/helpers/buildArtifacts.js @@ -121,13 +121,15 @@ const downloadAndUnzip = async (filePath, fileName, url) => { reject(err); }); writer.on('close', async () => { - if (!error) { - await unzipFile(filePath, fileName); - fs.unlinkSync(tmpFilePath); - resolve(true); + try { + if (!error) { + await unzipFile(filePath, fileName); + fs.unlinkSync(tmpFilePath); + } + } catch (error) { + reject(error); } - //no need to call the reject here, as it will have been called in the - //'error' stream; + resolve(true); }); } }); From 7cb8b2e3301683911135347ea922f184c64397dc Mon Sep 17 00:00:00 2001 From: Aditya Samantaray Date: Fri, 23 Aug 2024 11:20:44 +0530 Subject: [PATCH 4/5] refactor: add specs for buildArtifacts --- test/unit/bin/helpers/buildArtifacts.js | 68 +++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 test/unit/bin/helpers/buildArtifacts.js diff --git a/test/unit/bin/helpers/buildArtifacts.js b/test/unit/bin/helpers/buildArtifacts.js new file mode 100644 index 00000000..1be3c901 --- /dev/null +++ b/test/unit/bin/helpers/buildArtifacts.js @@ -0,0 +1,68 @@ +const { expect } = require("chai"); +const chai = require("chai"), + chaiAsPromised = require("chai-as-promised"), + sinon = require('sinon'), + rewire = require('rewire'); + +const fs = require('fs'), + path = require('path'), + request = require('request'), + unzipper = require("unzipper"), + decompress = require('decompress'); + Constants = require("../../../../bin/helpers/constants"), + logger = require("../../../../bin/helpers/logger").winstonLogger, + testObjects = require("../../support/fixtures/testObjects"), + formatRequest = require("../../../../bin/helpers/utils").formatRequest; + +chai.use(chaiAsPromised); +logger.transports["console.info"].silent = true; + +describe('unzipFile', () => { + let unzipFile; + let decompressStub; + let createReadStreamStub; + let unzipperStub; + let loggerStub; + let pathJoinStub; + let Constants; + + const filePath = '/some/path'; + const fileName = 'file.zip'; + + beforeEach(() => { + const unzipFileModule = rewire('../../../../bin/helpers/buildArtifacts'); + + decompressStub = sinon.stub(); + createReadStreamStub = sinon.stub(); + unzipperStub = { + Extract: sinon.stub(), + }; + loggerStub = { debug: sinon.stub() }; + pathJoinStub = sinon.stub().returns(`${filePath}/${fileName}`); + + Constants = { + debugMessages: { + BUILD_ARTIFACTS_UNZIP_FAILURE: 'Unzip failed', + }, + }; + + // Injecting the dependencies + unzipFileModule.__set__('decompress', decompressStub); + unzipFileModule.__set__('fs.createReadStream', createReadStreamStub); + unzipFileModule.__set__('unzipper', unzipperStub); + unzipFileModule.__set__('logger', loggerStub); + unzipFileModule.__set__('path.join', pathJoinStub); + unzipFileModule.__set__('Constants', Constants); + + unzipFile = unzipFileModule.__get__('unzipFile'); + }); + + it('should successfully unzip using decompress', async () => { + decompressStub.resolves(); + + await unzipFile(filePath, fileName); + + expect(decompressStub.calledWith(`${filePath}/${fileName}`, filePath)).to.be.true; + }); + +}); From e9fcb29c051a5322e01a0d25a22d3578b4ebbe6a Mon Sep 17 00:00:00 2001 From: Aditya Samantaray Date: Fri, 23 Aug 2024 12:19:52 +0530 Subject: [PATCH 5/5] refactor: address review comments --- bin/helpers/buildArtifacts.js | 2 +- bin/helpers/constants.js | 8 ++------ test/unit/bin/helpers/buildArtifacts.js | 12 ++---------- 3 files changed, 5 insertions(+), 17 deletions(-) diff --git a/bin/helpers/buildArtifacts.js b/bin/helpers/buildArtifacts.js index b2bc3385..6f361af9 100644 --- a/bin/helpers/buildArtifacts.js +++ b/bin/helpers/buildArtifacts.js @@ -154,7 +154,7 @@ const unzipFile = async (filePath, fileName) => { }); } catch (unzipperError) { logger.debug(`Unzipper package error: ${unzipperError}`); - reject(Constants.debugMessages.BUILD_ARTIFACTS_UNZIP_FAILURE); + reject(Constants.userMessages.BUILD_ARTIFACTS_UNZIP_FAILURE); } } }); diff --git a/bin/helpers/constants.js b/bin/helpers/constants.js index 94d98519..a19f188d 100644 --- a/bin/helpers/constants.js +++ b/bin/helpers/constants.js @@ -123,7 +123,8 @@ const userMessages = { CYPRESS_PORT_WARNING: "The requested port number is ignored. The default BrowserStack port will be used for this execution", CYPRESS_INTERACTIVE_SESSION_CONFLICT_VALUES: - "Conflicting values (True & False) were found for the interactive_debugging capability. Please resolve this issue to proceed further." + "Conflicting values (True & False) were found for the interactive_debugging capability. Please resolve this issue to proceed further.", + BUILD_ARTIFACTS_UNZIP_FAILURE: "Failed to unzip build artifacts.", }; const validationMessages = { @@ -271,10 +272,6 @@ const cliMessages = { }, }; -const debugMessages = { - BUILD_ARTIFACTS_UNZIP_FAILURE: "Failed to unzip build artifacts", -}; - const messageTypes = { SUCCESS: "success", ERROR: "error", @@ -459,7 +456,6 @@ module.exports = Object.freeze({ syncCLI, userMessages, cliMessages, - debugMessages, validationMessages, messageTypes, allowedFileTypes, diff --git a/test/unit/bin/helpers/buildArtifacts.js b/test/unit/bin/helpers/buildArtifacts.js index 1be3c901..08d2ef1a 100644 --- a/test/unit/bin/helpers/buildArtifacts.js +++ b/test/unit/bin/helpers/buildArtifacts.js @@ -4,15 +4,7 @@ const chai = require("chai"), sinon = require('sinon'), rewire = require('rewire'); -const fs = require('fs'), - path = require('path'), - request = require('request'), - unzipper = require("unzipper"), - decompress = require('decompress'); - Constants = require("../../../../bin/helpers/constants"), - logger = require("../../../../bin/helpers/logger").winstonLogger, - testObjects = require("../../support/fixtures/testObjects"), - formatRequest = require("../../../../bin/helpers/utils").formatRequest; +const logger = require("../../../../bin/helpers/logger").winstonLogger; chai.use(chaiAsPromised); logger.transports["console.info"].silent = true; @@ -41,7 +33,7 @@ describe('unzipFile', () => { pathJoinStub = sinon.stub().returns(`${filePath}/${fileName}`); Constants = { - debugMessages: { + userMessages: { BUILD_ARTIFACTS_UNZIP_FAILURE: 'Unzip failed', }, };