From 38ff53be3bf69f9d5e20c8af84bfd6bed251fb5c Mon Sep 17 00:00:00 2001 From: Jairo Panduro Date: Tue, 19 Apr 2022 18:09:22 +0200 Subject: [PATCH] feat: STRF-9741 Verbose error logging in Stencil CLI --- CHANGELOG.md | 2 ++ bin/stencil-bundle.js | 9 ++---- bin/stencil-download.js | 9 ++---- bin/stencil-init.js | 8 ++---- bin/stencil-pull.js | 10 ++----- bin/stencil-push.js | 10 ++----- bin/stencil-start.js | 9 ++---- lib/StencilCLISettings.js | 21 ++++++++++++++ lib/StencilCLISettings.spec.js | 18 ++++++++++++ lib/cliCommon.js | 50 ++++++++++++++++++++++++++++++++++ lib/utils/NetworkUtils.js | 12 ++++++++ lib/utils/NetworkUtils.spec.js | 38 ++++++++++++++++++++++++++ 12 files changed, 159 insertions(+), 37 deletions(-) create mode 100644 lib/StencilCLISettings.js create mode 100644 lib/StencilCLISettings.spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fcb9832e..1724d5afa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ### Draft +- feat: STRF-9741 Verbose error logging in Stencil CLI by default ([x](https://github.com/bigcommerce/stencil-cli/pull/x)) + ### 4.0.0 (2022-04-11) - Added support for node 14 and drop node 10 diff --git a/bin/stencil-bundle.js b/bin/stencil-bundle.js index 041ef5d74..a95fb9ae8 100755 --- a/bin/stencil-bundle.js +++ b/bin/stencil-bundle.js @@ -7,7 +7,7 @@ const { THEME_PATH, PACKAGE_INFO } = require('../constants'); const ThemeConfig = require('../lib/theme-config'); const Bundle = require('../lib/stencil-bundle'); const { printCliResultErrorAndExit } = require('../lib/cliCommon'); -const { checkNodeVersion } = require('../lib/cliCommon'); +const { prepareCommand } = require('../lib/cliCommon'); const BuildConfigManager = require('../lib/BuildConfigManager'); program @@ -28,16 +28,13 @@ program '-t, --timeout [timeout]', 'Set a timeout for the bundle operation. Default is 20 secs', '60', - ) - .parse(process.argv); + ); -const cliOptions = program.opts(); +const cliOptions = prepareCommand(program); const themeConfig = ThemeConfig.getInstance(THEME_PATH); async function run() { try { - checkNodeVersion(); - if (cliOptions.dest === true) { throw new Error('You have to specify a value for -d or --dest'.red); } diff --git a/bin/stencil-download.js b/bin/stencil-download.js index c5061314e..0b92113d9 100755 --- a/bin/stencil-download.js +++ b/bin/stencil-download.js @@ -6,21 +6,18 @@ const program = require('../lib/commander'); const { PACKAGE_INFO } = require('../constants'); const stencilDownload = require('../lib/stencil-download'); -const { checkNodeVersion } = require('../lib/cliCommon'); +const { prepareCommand } = require('../lib/cliCommon'); const { printCliResultErrorAndExit } = require('../lib/cliCommon'); program .version(PACKAGE_INFO.version) - .option('-h, --host [hostname]', 'specify the api host') .option('-f, --file [filename]', 'specify the filename to download only') .option('-e, --exclude [exclude]', 'specify a directory to exclude from download') .option('-c, --channel_id [channelId]', 'specify the channel ID of the storefront', parseInt) - .option('-o, --overwrite', 'overwrite local with remote files') - .parse(process.argv); + .option('-o, --overwrite', 'overwrite local with remote files'); -checkNodeVersion(); -const cliOptions = program.opts(); +const cliOptions = prepareCommand(program); const extraExclude = cliOptions.exclude ? [cliOptions.exclude] : []; const options = { exclude: ['parsed', 'manifest.json', ...extraExclude], diff --git a/bin/stencil-init.js b/bin/stencil-init.js index 73da90719..fe8be3446 100755 --- a/bin/stencil-init.js +++ b/bin/stencil-init.js @@ -4,19 +4,17 @@ const program = require('../lib/commander'); const StencilInit = require('../lib/stencil-init'); const { PACKAGE_INFO } = require('../constants'); -const { checkNodeVersion, printCliResultErrorAndExit } = require('../lib/cliCommon'); +const { prepareCommand, printCliResultErrorAndExit } = require('../lib/cliCommon'); program .version(PACKAGE_INFO.version) .option('-u, --url [url]', 'Store URL') .option('-t, --token [token]', 'Access Token') .option('-p, --port [port]', 'Port') - .option('-h, --apiHost [host]', 'API Host') - .parse(process.argv); + .option('-h, --apiHost [host]', 'API Host'); -checkNodeVersion(); -const cliOptions = program.opts(); +const cliOptions = prepareCommand(program); new StencilInit() .run({ diff --git a/bin/stencil-pull.js b/bin/stencil-pull.js index 88ccfcc65..e93765734 100755 --- a/bin/stencil-pull.js +++ b/bin/stencil-pull.js @@ -5,13 +5,12 @@ require('colors'); const { PACKAGE_INFO } = require('../constants'); const program = require('../lib/commander'); const stencilPull = require('../lib/stencil-pull'); -const { checkNodeVersion } = require('../lib/cliCommon'); +const { prepareCommand } = require('../lib/cliCommon'); const { printCliResultErrorAndExit } = require('../lib/cliCommon'); program .version(PACKAGE_INFO.version) .option('-s, --saved', 'get the saved configuration instead of the active one') - .option('-h, --host [hostname]', 'specify the api host') .option( '-f, --filename [filename]', 'specify the filename to save the config as', @@ -21,12 +20,9 @@ program '-c, --channel_id [channelId]', 'specify the channel ID of the storefront to pull configuration from', parseInt, - ) - .parse(process.argv); - -checkNodeVersion(); + ); -const cliOptions = program.opts(); +const cliOptions = prepareCommand(program); const options = { apiHost: cliOptions.host, diff --git a/bin/stencil-push.js b/bin/stencil-push.js index 868ee465e..3d9b956c8 100755 --- a/bin/stencil-push.js +++ b/bin/stencil-push.js @@ -4,12 +4,11 @@ require('colors'); const { PACKAGE_INFO } = require('../constants'); const program = require('../lib/commander'); const stencilPush = require('../lib/stencil-push'); -const { checkNodeVersion } = require('../lib/cliCommon'); +const { prepareCommand } = require('../lib/cliCommon'); const { printCliResultErrorAndExit } = require('../lib/cliCommon'); program .version(PACKAGE_INFO.version) - .option('--host [hostname]', 'specify the api host') .option('-f, --file [filename]', 'specify the filename of the bundle to upload') .option('-s, --save [filename]', 'specify the filename to save the bundle as') .option('-a, --activate [variationname]', 'specify the variation of the theme to activate') @@ -18,12 +17,9 @@ program '-c, --channel_ids ', 'specify the channel IDs of the storefront to push the theme to', ) - .option('-allc, --all_channels', 'push a theme to all available channels') - .parse(process.argv); + .option('-allc, --all_channels', 'push a theme to all available channels'); -checkNodeVersion(); - -const cliOptions = program.opts(); +const cliOptions = prepareCommand(program); const options = { apiHost: cliOptions.host, channelIds: cliOptions.channel_ids, diff --git a/bin/stencil-start.js b/bin/stencil-start.js index cf636eb9b..c61120c83 100755 --- a/bin/stencil-start.js +++ b/bin/stencil-start.js @@ -4,7 +4,7 @@ require('colors'); const { PACKAGE_INFO } = require('../constants'); const program = require('../lib/commander'); const StencilStart = require('../lib/stencil-start'); -const { printCliResultErrorAndExit } = require('../lib/cliCommon'); +const { printCliResultErrorAndExit, prepareCommand } = require('../lib/cliCommon'); const BuildConfigManager = require('../lib/BuildConfigManager'); program @@ -12,7 +12,6 @@ program .option('-o, --open', 'Automatically open default browser') .option('-v, --variation [name]', 'Set which theme variation to use while developing') .option('-c, --channelId [channelId]', 'Set the channel id for the storefront') - .option('--host [hostname]', 'specify the api host') .option( '--tunnel [name]', 'Create a tunnel URL which points to your local server that anyone can use.', @@ -21,11 +20,9 @@ program '-n, --no-cache', 'Turns off caching for API resource data per storefront page. The cache lasts for 5 minutes before automatically refreshing.', ) - .option('-t, --timeout', 'Set a timeout for the bundle operation. Default is 20 secs', '60') - .parse(process.argv); - -const cliOptions = program.opts(); + .option('-t, --timeout', 'Set a timeout for the bundle operation. Default is 20 secs', '60'); +const cliOptions = prepareCommand(program); const options = { open: cliOptions.open, variation: cliOptions.variation, diff --git a/lib/StencilCLISettings.js b/lib/StencilCLISettings.js new file mode 100644 index 000000000..1ab2102e2 --- /dev/null +++ b/lib/StencilCLISettings.js @@ -0,0 +1,21 @@ + +class StencilCLISettings { + constructor() { + // The setting enables/disables verbose network requests logging + this.versboseNetworkLogging = true; + } + + setVerbose(flag) { + this.versboseNetworkLogging = flag; + } + + isVerbose() { + return this.versboseNetworkLogging === true; + } +} + + +const settings = new StencilCLISettings(); + + +module.exports = settings; diff --git a/lib/StencilCLISettings.spec.js b/lib/StencilCLISettings.spec.js new file mode 100644 index 000000000..59974ea39 --- /dev/null +++ b/lib/StencilCLISettings.spec.js @@ -0,0 +1,18 @@ +const stencilCLISettings = require('./StencilCLISettings'); + + +describe('StencilCLISettings', () => { + afterEach(() => { + // setting to default + stencilCLISettings.setVerbose(true); + }); + + it('should set network logging to verbose by default', () => { + expect(stencilCLISettings.isVerbose()).toBeTruthy(); + }); + + it('should set network logging to NOT verbose', () => { + stencilCLISettings.setVerbose(false); + expect(stencilCLISettings.isVerbose()).toBeFalsy(); + }) +}) diff --git a/lib/cliCommon.js b/lib/cliCommon.js index b8853618e..00d9427e5 100644 --- a/lib/cliCommon.js +++ b/lib/cliCommon.js @@ -1,5 +1,6 @@ const semver = require('semver'); const { PACKAGE_INFO } = require('../constants'); +const stencilCLISettings = require('./StencilCLISettings'); const messages = { visitTroubleshootingPage: @@ -24,11 +25,38 @@ function printCliResultError(error) { } } + if (error && error.response) { + printNetworkError(error); + } + console.log(messages.visitTroubleshootingPage); console.log(messages.submitGithubIssue); } +/** + * @param {Error} error + + * @returns {void} + */ +function printNetworkError(error) { + const { response } = error; + console.log(`URL: `.yellow + response.config.url); + console.log(`Method: `.yellow + response.config.method.toUpperCase()); + console.log(`Headers:`.yellow); + printObject(response.headers); + if (response.config.data) { + console.log(`Data: `.yellow); + printObject(response.config.data); + } +} + +function printObject(object) { + for (const property in object) { + console.log(`${property}: ${object[property]}`); + } +} + /** * @param {Error} error * @returns {void} @@ -52,9 +80,31 @@ function checkNodeVersion() { return satisfies; } +function applyCommonOptions(program) { + program + .option('-h, --host [hostname]', 'specify the api host') + .option('-nov, --no-verbose', 'supress verbose info logging', false) + .parse(process.argv); +} + +function setupCLI(options) { + stencilCLISettings.setVerbose(options.verbose); +} + +function prepareCommand(program) { + applyCommonOptions(program); + checkNodeVersion(); + + const options = program.opts(); + setupCLI(options); + + return options; +} + module.exports = { messages, printCliResultError, printCliResultErrorAndExit, checkNodeVersion, + prepareCommand, }; diff --git a/lib/utils/NetworkUtils.js b/lib/utils/NetworkUtils.js index 838c29e38..8cec5224a 100644 --- a/lib/utils/NetworkUtils.js +++ b/lib/utils/NetworkUtils.js @@ -7,6 +7,7 @@ const fsModule = require('fs'); const axios = require('axios'); const { PACKAGE_INFO } = require('../../constants'); +const stencilCLISettings = require('../StencilCLISettings'); const defaultHttpsAgent = new https.Agent({ rejectUnauthorized: false }); @@ -16,11 +17,13 @@ class NetworkUtils { httpsAgent = defaultHttpsAgent, reqLibrary = axios, packageInfo = PACKAGE_INFO, + logger = console, } = {}) { this._fs = fs; this._httpsAgent = httpsAgent; this._reqLibrary = reqLibrary; this._packageInfo = packageInfo; + this._logger = logger; } /** Used to send request to our (Bigcommerce) servers only. @@ -50,6 +53,10 @@ class NetworkUtils { reqConfig.headers['x-auth-token'] = accessToken; } + if (stencilCLISettings.isVerbose()) { + this.log(reqConfig); + } + return this._reqLibrary(reqConfig); } @@ -71,6 +78,11 @@ class NetworkUtils { .on('error', reject); }); } + + log(config) { + const method = config.method || 'GET'; + this._logger.info(`Upcoming request ${method.green}: ${config.url.green}`); + } } module.exports = NetworkUtils; diff --git a/lib/utils/NetworkUtils.spec.js b/lib/utils/NetworkUtils.spec.js index e46ddcd97..38d6ca559 100644 --- a/lib/utils/NetworkUtils.spec.js +++ b/lib/utils/NetworkUtils.spec.js @@ -1,8 +1,12 @@ const { PassThrough } = require('stream'); const MockWritableStream = require('../../test/_mocks/MockWritableStream'); const NetworkUtils = require('./NetworkUtils'); +require('colors'); describe('NetworkUtils', () => { + const loggerMock = { + info: jest.fn() + }; afterEach(() => jest.resetAllMocks()); afterAll(() => jest.restoreAllMocks()); @@ -23,6 +27,7 @@ describe('NetworkUtils', () => { reqLibrary: axiosMock, httpsAgent: httpsAgentMock, packageInfo: packageInfoMock, + logger: loggerMock, }); await networkUtils.sendApiRequest({ url }); @@ -56,6 +61,7 @@ describe('NetworkUtils', () => { reqLibrary: axiosMock, httpsAgent: httpsAgentMock, packageInfo: packageInfoMock, + logger: loggerMock, }); await networkUtils.sendApiRequest({ url, @@ -95,6 +101,7 @@ describe('NetworkUtils', () => { reqLibrary: axiosMock, httpsAgent: httpsAgentMock, packageInfo: packageInfoMock, + logger: loggerMock, }); await networkUtils.sendApiRequest({ url, @@ -130,6 +137,7 @@ describe('NetworkUtils', () => { reqLibrary: axiosMock, httpsAgent: httpsAgentMock, packageInfo: packageInfoMock, + logger: loggerMock, }); await networkUtils.sendApiRequest({ url, @@ -168,6 +176,7 @@ describe('NetworkUtils', () => { reqLibrary: axiosMock, httpsAgent: httpsAgentMock, packageInfo: packageInfoMock, + logger: loggerMock, }); const actualRes = await networkUtils.sendApiRequest({ url }); @@ -187,6 +196,7 @@ describe('NetworkUtils', () => { const resMockData = 'hello world'; const networkUtils = new NetworkUtils({ fs: fsMock, + logger: loggerMock, }); const sendApiRequestStub = jest .spyOn(networkUtils, 'sendApiRequest') @@ -212,4 +222,32 @@ describe('NetworkUtils', () => { expect(mockWritable.buffer).toEqual(resMockData); }); }); + + describe('log', () => { + it('should log the upcoming request', async () => { + const axiosMock = jest.fn(); + const httpsAgentMock = jest.fn(); + const packageInfoMock = { + version: '1', + config: { + stencil_version: '2', + }, + }; + const url = 'https://www.example.com/api/val'; + const accessToken = 'accessToken_value'; + + const networkUtils = new NetworkUtils({ + reqLibrary: axiosMock, + httpsAgent: httpsAgentMock, + packageInfo: packageInfoMock, + logger: loggerMock + }); + await networkUtils.sendApiRequest({ + url, + accessToken, + }); + + expect(loggerMock.info).toHaveBeenCalledWith(`Upcoming request ${`GET`.green}: ${url.green}`); + }); + }); });