From 1ad917fab416b54c4118329d318216b232e95aad Mon Sep 17 00:00:00 2001 From: Philipe Navarro Date: Mon, 18 Jul 2022 13:34:41 -0700 Subject: [PATCH 1/3] refactor: env var set error handling --- src/commands/env/var/set.ts | 142 ++++++++++++++++++------------------ 1 file changed, 71 insertions(+), 71 deletions(-) diff --git a/src/commands/env/var/set.ts b/src/commands/env/var/set.ts index 17d478e4..0bec9b6b 100644 --- a/src/commands/env/var/set.ts +++ b/src/commands/env/var/set.ts @@ -53,54 +53,57 @@ export default class ConfigSet extends Command { }, {}); } - parseErrors(errorObject: any, targetCompute: string) { - // may have create a cleaner way to parse the errors - // to fit the sfdx json error object, but this currently works - - const errObj = errorObject.stack; - const isInvalidEnvironment = errObj.includes("Error: Couldn't"); - const hasNoInput = errObj.includes('Error: Usage:'); - const isInvalidInput = !hasNoInput; - - if (isInvalidEnvironment) { - const errorStackStartIndex = errObj.indexOf('at'); - const errStack = errObj.substr(errorStackStartIndex); - errorObject.message = `Couldn't find that app <${targetCompute}>`; - errorObject.stack = errStack; - this.error(errObj); - } - - if (isInvalidInput) { - const errorIndex = errObj.indexOf(':') + 1; - const keyValueIndex = errObj.indexOf('value') + 5; - - // eslint-disable-next-line no-control-regex - const errMessage = errObj - .substr(errorIndex, keyValueIndex) - // eslint-disable-next-line no-control-regex - .replace(/\u001b\[.*?m/g, '') - .replace('\n', '') - .replace(' ', ''); - // eslint-disable-next-line no-control-regex - const errStack = errObj - .substr(keyValueIndex) - // eslint-disable-next-line no-control-regex - .replace(/\u001b\[.*?m\r?\n|\r/g, '') - .replace(' ', ''); - - errorObject.message = errMessage; - errorObject.stack = errStack; - this.error(errObj); - } - - if (hasNoInput) { - const errorStackStartIndex = errObj.indexOf('at'); - const errStack = errObj.substr(errorStackStartIndex); - errorObject.message = 'Must specify KEY and VALUE to set.'; - errorObject.stack = errStack; - this.error(errObj); - } - } + // keep this here for context + // delete by 1/1/2023 + // + // parseErrors(errorObject: any, targetCompute: string) { + // // may have create a cleaner way to parse the errors + // // to fit the sfdx json error object, but this currently works + + // const errObj = errorObject.stack; + // const isInvalidEnvironment = errObj.includes("Error: Couldn't"); + // const hasNoInput = errObj.includes('Error: Usage:'); + // const isInvalidInput = !hasNoInput; + + // if (isInvalidEnvironment) { + // const errorStackStartIndex = errObj.indexOf('at'); + // const errStack = errObj.substr(errorStackStartIndex); + // errorObject.message = `Couldn't find that app <${targetCompute}>`; + // errorObject.stack = errStack; + // this.error(errObj); + // } + + // if (isInvalidInput) { + // const errorIndex = errObj.indexOf(':') + 1; + // const keyValueIndex = errObj.indexOf('value') + 5; + + // // eslint-disable-next-line no-control-regex + // const errMessage = errObj + // .substr(errorIndex, keyValueIndex) + // // eslint-disable-next-line no-control-regex + // .replace(/\u001b\[.*?m/g, '') + // .replace('\n', '') + // .replace(' ', ''); + // // eslint-disable-next-line no-control-regex + // const errStack = errObj + // .substr(keyValueIndex) + // // eslint-disable-next-line no-control-regex + // .replace(/\u001b\[.*?m\r?\n|\r/g, '') + // .replace(' ', ''); + + // errorObject.message = errMessage; + // errorObject.stack = errStack; + // this.error(errObj); + // } + + // if (hasNoInput) { + // const errorStackStartIndex = errObj.indexOf('at'); + // const errStack = errObj.substr(errorStackStartIndex); + // errorObject.message = 'Must specify KEY and VALUE to set.'; + // errorObject.stack = errStack; + // this.error(errObj); + // } + // } async run() { const { flags, argv } = await this.parse(ConfigSet); @@ -123,37 +126,34 @@ export default class ConfigSet extends Command { const appName = await resolveAppNameForEnvironment(targetCompute); - if (flags.json) { - try { - const configPairs = this.parseKeyValuePairs(argv); + const configPairs = this.parseKeyValuePairs(argv); - await this.client.patch(`/apps/${appName}/config-vars`, { - data: configPairs, - }); + cli.action.start( + `Setting ${Object.keys(configPairs) + .map((key) => herokuColor.configVar(key)) + .join(', ')} and restarting ${herokuColor.app(targetCompute)}` + ); + + try { + await this.client.patch(`/apps/${appName}/config-vars`, { + data: configPairs, + }); + cli.action.stop(); + + if (flags.json) { cli.styledJSON({ status: 0, result: null, warnings: [], }); - return; - } catch (err: any) { - this.parseErrors(err, targetCompute); } - } else { - const configPairs = this.parseKeyValuePairs(argv); - - cli.action.start( - `Setting ${Object.keys(configPairs) - .map((key) => herokuColor.configVar(key)) - .join(', ')} and restarting ${herokuColor.app(targetCompute)}` - ); - - await this.client.patch(`/apps/${appName}/config-vars`, { - data: configPairs, - }); - - cli.action.stop(); + } catch (error: any) { + cli.action.stop('failed'); + if (error.data?.message?.includes("Couldn't find that app")) { + this.error(new Error(`Could not find environment <${appName}>`)); + } + this.error(error); } } } From b6b9406dbf9cf3c3cd0276ebb5824e17c319a8bc Mon Sep 17 00:00:00 2001 From: Philipe Navarro Date: Mon, 18 Jul 2022 13:51:28 -0700 Subject: [PATCH 2/3] test: retry test b/c of weird race condition --- test/commands/env/var/unset.test.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/commands/env/var/unset.test.ts b/test/commands/env/var/unset.test.ts index 8a0e01b6..fa51a6d1 100644 --- a/test/commands/env/var/unset.test.ts +++ b/test/commands/env/var/unset.test.ts @@ -24,6 +24,9 @@ describe('sf env:var:unset', () => { test .stdout() .stderr() + // Adding retries here because there is some kind of race condition that causes fancy-test to not + // fully capture the value of stderr when running in CI (╯°□°)╯︵ ┻━┻ + .retries(2) .nock('https://api.heroku.com', (api) => api .patch('/apps/my-environment/config-vars', { From a3f0f10e285c1e67f1b143f3fc48281814603713 Mon Sep 17 00:00:00 2001 From: Philipe Navarro Date: Tue, 19 Jul 2022 09:46:49 -0700 Subject: [PATCH 3/3] rm comment --- src/commands/env/var/set.ts | 52 ------------------------------------- 1 file changed, 52 deletions(-) diff --git a/src/commands/env/var/set.ts b/src/commands/env/var/set.ts index 0bec9b6b..69620577 100644 --- a/src/commands/env/var/set.ts +++ b/src/commands/env/var/set.ts @@ -53,58 +53,6 @@ export default class ConfigSet extends Command { }, {}); } - // keep this here for context - // delete by 1/1/2023 - // - // parseErrors(errorObject: any, targetCompute: string) { - // // may have create a cleaner way to parse the errors - // // to fit the sfdx json error object, but this currently works - - // const errObj = errorObject.stack; - // const isInvalidEnvironment = errObj.includes("Error: Couldn't"); - // const hasNoInput = errObj.includes('Error: Usage:'); - // const isInvalidInput = !hasNoInput; - - // if (isInvalidEnvironment) { - // const errorStackStartIndex = errObj.indexOf('at'); - // const errStack = errObj.substr(errorStackStartIndex); - // errorObject.message = `Couldn't find that app <${targetCompute}>`; - // errorObject.stack = errStack; - // this.error(errObj); - // } - - // if (isInvalidInput) { - // const errorIndex = errObj.indexOf(':') + 1; - // const keyValueIndex = errObj.indexOf('value') + 5; - - // // eslint-disable-next-line no-control-regex - // const errMessage = errObj - // .substr(errorIndex, keyValueIndex) - // // eslint-disable-next-line no-control-regex - // .replace(/\u001b\[.*?m/g, '') - // .replace('\n', '') - // .replace(' ', ''); - // // eslint-disable-next-line no-control-regex - // const errStack = errObj - // .substr(keyValueIndex) - // // eslint-disable-next-line no-control-regex - // .replace(/\u001b\[.*?m\r?\n|\r/g, '') - // .replace(' ', ''); - - // errorObject.message = errMessage; - // errorObject.stack = errStack; - // this.error(errObj); - // } - - // if (hasNoInput) { - // const errorStackStartIndex = errObj.indexOf('at'); - // const errStack = errObj.substr(errorStackStartIndex); - // errorObject.message = 'Must specify KEY and VALUE to set.'; - // errorObject.stack = errStack; - // this.error(errObj); - // } - // } - async run() { const { flags, argv } = await this.parse(ConfigSet); this.postParseHook(flags);