Skip to content

Commit

Permalink
refactor(settings): non-async isPromptEnabled() #6255
Browse files Browse the repository at this point in the history
## Problem
There is no real reason this needs to be async. It is also now
inconsistent with `isExperimentEnabled`.

## Solution
Make non-async and add logged error if the promise rejects.
  • Loading branch information
Hweinstock authored Dec 17, 2024
1 parent b085dd9 commit 04a5b68
Show file tree
Hide file tree
Showing 16 changed files with 24 additions and 24 deletions.
2 changes: 1 addition & 1 deletion packages/core/src/auth/sso/ssoAccessTokenProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ class DiskCacheErrorMessage {
: ToolkitPromptSettings.instance

// We know 'ssoCacheError' is in all extension prompt settings
if (await promptSettings.isPromptEnabled('ssoCacheError')) {
if (promptSettings.isPromptEnabled('ssoCacheError')) {
const result = await showMessage()
if (result === dontShow) {
await promptSettings.disablePrompt('ssoCacheError')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export async function pauseService(node: AppRunnerServiceNode): Promise<void> {

try {
const prompts = ToolkitPromptSettings.instance
const shouldNotify = await prompts.isPromptEnabled('apprunnerNotifyPause')
const shouldNotify = prompts.isPromptEnabled('apprunnerNotifyPause')
const notifyPrompt = localize(
'aws.apprunner.pauseService.notify',
'Your service will be unavailable while paused. ' +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function makeDeployButtons() {
async function showDeploymentCostNotification(): Promise<void> {
const settings = ToolkitPromptSettings.instance

if (await settings.isPromptEnabled('apprunnerNotifyPricing')) {
if (settings.isPromptEnabled('apprunnerNotifyPricing')) {
const notice = localize(
'aws.apprunner.createService.priceNotice.message',
'App Runner automatic deployments incur an additional cost.'
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/awsService/ecs/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ async function runCommandWizard(

const wizard = new CommandWizard(
container,
await ToolkitPromptSettings.instance.isPromptEnabled('ecsRunCommand'),
ToolkitPromptSettings.instance.isPromptEnabled('ecsRunCommand'),
command
)
const response = await wizard.run()
Expand Down Expand Up @@ -75,7 +75,7 @@ export async function toggleExecuteCommandFlag(
'Disabling command execution will change the state of resources in your AWS account, including but not limited to stopping and restarting the service.\n Continue?'
)

if (await settings.isPromptEnabled(prompt)) {
if (settings.isPromptEnabled(prompt)) {
const choice = await window.showWarningMessage(warningMessage, yes, yesDontAskAgain, no)
if (choice === undefined || choice === no) {
throw new CancellationError('user')
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/awsService/s3/fileViewerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ export class S3FileViewerManager {
}

private async showEditNotification(): Promise<void> {
if (!(await this.settings.isPromptEnabled(promptOnEditKey))) {
if (!this.settings.isPromptEnabled(promptOnEditKey)) {
return
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/codecatalyst/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export async function activate(ctx: ExtContext): Promise<void> {
await showReadmeFileOnFirstLoad(ctx.extensionContext.workspaceState)

const settings = ToolkitPromptSettings.instance
if (await settings.isPromptEnabled('remoteConnected')) {
if (settings.isPromptEnabled('remoteConnected')) {
const message = localize(
'AWS.codecatalyst.connectedMessage',
'Welcome to your Amazon CodeCatalyst Dev Environment. For more options and information, view Dev Environment settings ({0} Extension > CodeCatalyst).',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class CodeWhispererCommandBackend {

const prompts = AmazonQPromptSettings.instance
// To check the condition If the user has already seen the welcome message
if (!(await prompts.isPromptEnabled('codeWhispererNewWelcomeMessage'))) {
if (!prompts.isPromptEnabled('codeWhispererNewWelcomeMessage')) {
telemetry.ui_click.emit({ elementId: 'codewhisperer_Learn_ButtonClick', passive: true })
}
return showCodeWhispererWebview(this.extContext, source)
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/codewhisperer/util/authUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ export class AuthUtil {
public async notifySessionConfiguration() {
const suppressId = 'amazonQSessionConfigurationMessage'
const settings = AmazonQPromptSettings.instance
const shouldShow = await settings.isPromptEnabled(suppressId)
const shouldShow = settings.isPromptEnabled(suppressId)
if (!shouldShow) {
return
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/codewhisperer/vue/backend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export async function showCodeWhispererWebview(
]
const prompts = AmazonQPromptSettings.instance
// To check the condition If the user has already seen the welcome message
if (await prompts.isPromptEnabled('codeWhispererNewWelcomeMessage')) {
if (prompts.isPromptEnabled('codeWhispererNewWelcomeMessage')) {
telemetry.ui_click.emit({ elementId: 'codewhisperer_Learn_PageOpen', passive: true })
} else {
telemetry.ui_click.emit({ elementId: 'codewhisperer_Learn_PageOpen', passive: false })
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/awsContextCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export class AwsContextCommands {
await this.editCredentials()
if (
credentialsFiles.length === 0 &&
(await ToolkitPromptSettings.instance.isPromptEnabled('createCredentialsProfile')) &&
ToolkitPromptSettings.instance.isPromptEnabled('createCredentialsProfile') &&
(await this.promptCredentialsSetup())
) {
await this.onCommandCreateCredentialsProfile()
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/extensionStartup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const localize = nls.loadMessageBundle()
*/
export async function maybeShowMinVscodeWarning(minVscode: string) {
const settings = isAmazonQ() ? AmazonQPromptSettings.instance : ToolkitPromptSettings.instance
if (!(await settings.isPromptEnabled('minIdeVersion'))) {
if (!settings.isPromptEnabled('minIdeVersion')) {
return
}
const updateButton = `Update ${vscode.env.appName}`
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/sam/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ async function createYamlExtensionPrompt(): Promise<void> {
// Show this only in VSCode since other VSCode-like IDEs (e.g. Theia) may
// not have a marketplace or contain the YAML plugin.
if (
(await settings.isPromptEnabled('yamlExtPrompt')) &&
settings.isPromptEnabled('yamlExtPrompt') &&
getIdeType() === 'vscode' &&
!vscode.extensions.getExtension(VSCODE_EXTENSION_ID.yaml)
) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/sam/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ async function updateSyncRecentResponse(region: string, key: string, value: stri
}

export async function confirmDevStack() {
const canPrompt = await ToolkitPromptSettings.instance.isPromptEnabled('samcliConfirmDevStack')
const canPrompt = ToolkitPromptSettings.instance.isPromptEnabled('samcliConfirmDevStack')
if (!canPrompt) {
return
}
Expand Down
16 changes: 8 additions & 8 deletions packages/core/src/shared/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ export function fromExtensionManifest<T extends TypeDescriptor & Partial<Section
*
* ### Usage:
* ```
* if (await settings.isPromptEnabled('myPromptName')) {
* if (settings.isPromptEnabled('myPromptName')) {
* // Show some sort of prompt
* const userResponse = await promptUser()
*
Expand All @@ -627,19 +627,19 @@ export class ToolkitPromptSettings
)
implements PromptSettings
{
public async isPromptEnabled(promptName: toolkitPromptName): Promise<boolean> {
public isPromptEnabled(promptName: toolkitPromptName): boolean {
try {
return !this._getOrThrow(promptName, false)
} catch (e) {
this._log('prompt check for "%s" failed: %s', promptName, (e as Error).message)
await this.reset()
this.reset().catch((e) => getLogger().error(`failed to reset prompt settings: %O`, (e as Error).message))

return true
}
}

public async disablePrompt(promptName: toolkitPromptName): Promise<void> {
if (await this.isPromptEnabled(promptName)) {
if (this.isPromptEnabled(promptName)) {
await this.update(promptName, true)
}
}
Expand All @@ -660,19 +660,19 @@ export class AmazonQPromptSettings
)
implements PromptSettings
{
public async isPromptEnabled(promptName: amazonQPromptName): Promise<boolean> {
public isPromptEnabled(promptName: amazonQPromptName): boolean {
try {
return !this._getOrThrow(promptName, false)
} catch (e) {
this._log('prompt check for "%s" failed: %s', promptName, (e as Error).message)
await this.reset()
this.reset().catch((e) => getLogger().error(`isPromptEnabled: reset() failed: %O`, (e as Error).message))

return true
}
}

public async disablePrompt(promptName: amazonQPromptName): Promise<void> {
if (await this.isPromptEnabled(promptName)) {
if (this.isPromptEnabled(promptName)) {
await this.update(promptName, true)
}
}
Expand All @@ -692,7 +692,7 @@ export class AmazonQPromptSettings
type AllPromptNames = amazonQPromptName | toolkitPromptName

export interface PromptSettings {
isPromptEnabled(promptName: AllPromptNames): Promise<boolean>
isPromptEnabled(promptName: AllPromptNames): boolean
disablePrompt(promptName: AllPromptNames): Promise<void>
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/shared/utilities/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export async function showReauthenticateMessage({
reauthFunc: () => Promise<void>
source?: string
}) {
const shouldShow = await settings.isPromptEnabled(suppressId as any)
const shouldShow = settings.isPromptEnabled(suppressId as any)
if (!shouldShow) {
return
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/test/shared/settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,7 @@ describe('PromptSetting', function () {
it(scenario.desc, async () => {
await settings.update(promptSettingKey, scenario.testValue)
const before = settings.get(promptSettingKey, Object, {})
const result = await sut.isPromptEnabled(promptName)
const result = sut.isPromptEnabled(promptName)

assert.deepStrictEqual(result, scenario.expected)
assert.deepStrictEqual(
Expand Down

0 comments on commit 04a5b68

Please sign in to comment.