diff --git a/packages/core/src/awsService/cloudformation/extension.ts b/packages/core/src/awsService/cloudformation/extension.ts index 7818df8de0a..f3c858557dd 100644 --- a/packages/core/src/awsService/cloudformation/extension.ts +++ b/packages/core/src/awsService/cloudformation/extension.ts @@ -52,7 +52,7 @@ import { AwsCredentialsService, encryptionKey } from './auth/credentials' import { ExtensionId, ExtensionName, Version, CloudFormationTelemetrySettings } from './extensionConfig' import { commandKey } from './utils' import { CloudFormationExplorer } from './explorer/explorer' -import { promptTelemetryOptInWithTimeout } from './telemetryOptIn' +import { handleTelemetryOptIn } from './telemetryOptIn' import { refreshCommand, StacksManager } from './stacks/stacksManager' import { StackOverviewWebviewProvider } from './ui/stackOverviewWebviewProvider' @@ -89,7 +89,7 @@ let clientDisposables: Disposable[] = [] async function startClient(context: ExtensionContext) { const cfnTelemetrySettings = new CloudFormationTelemetrySettings() - const telemetryEnabled = await promptTelemetryOptInWithTimeout(context, cfnTelemetrySettings) + const telemetryEnabled = await handleTelemetryOptIn(context, cfnTelemetrySettings) const cfnLspConfig = { ...DevSettings.instance.getServiceConfig('cloudformationLsp', {}), diff --git a/packages/core/src/awsService/cloudformation/telemetryOptIn.ts b/packages/core/src/awsService/cloudformation/telemetryOptIn.ts index 9a8e5e0d09e..0b1f927e30b 100644 --- a/packages/core/src/awsService/cloudformation/telemetryOptIn.ts +++ b/packages/core/src/awsService/cloudformation/telemetryOptIn.ts @@ -7,74 +7,165 @@ import { ExtensionContext, env, Uri, window } from 'vscode' import { CloudFormationTelemetrySettings } from './extensionConfig' import { commandKey } from './utils' import { isAutomation } from '../../shared/vscode/env' +import { getLogger } from '../../shared/logger/logger' +import globals from '../../shared/extensionGlobals' -export async function promptTelemetryOptInWithTimeout( - context: ExtensionContext, - cfnTelemetrySettings: CloudFormationTelemetrySettings -): Promise { - const promptPromise = promptTelemetryOptIn(context, cfnTelemetrySettings) - const timeoutPromise = new Promise((resolve) => setTimeout(() => resolve(false), 2500)) +enum TelemetryChoice { + Allow = 'Yes, Allow', + Later = 'Not Now', + Never = 'Never', + LearnMore = 'Learn More', +} - const result = await Promise.race([promptPromise, timeoutPromise]) +const telemetryKeys = { + hasResponded: commandKey('telemetry.hasResponded'), + lastPromptDate: commandKey('telemetry.lastPromptDate'), + unpersistedResponse: commandKey('telemetry.unpersistedResponse'), +} as const - // Keep prompt alive in background - void promptPromise +const telemetrySettings = { + enabled: 'enabled', +} as const - return result -} +const thirtyDaysMs = 30 * 24 * 60 * 60 * 1000 +const promptTimeoutMs = 2500 +const telemetryDocsUrl = 'https://github.com/aws-cloudformation/cloudformation-languageserver/tree/main/src/telemetry' /* eslint-disable aws-toolkits/no-banned-usages */ -async function promptTelemetryOptIn( +export async function handleTelemetryOptIn( context: ExtensionContext, cfnTelemetrySettings: CloudFormationTelemetrySettings ): Promise { - const telemetryEnabled = cfnTelemetrySettings.get('enabled', false) - if (isAutomation()) { - return telemetryEnabled + // If previous choice failed to persist, persist it now and return + const unpersistedResponse = (await context.globalState.get(telemetryKeys.unpersistedResponse)) as string + const hasResponded = context.globalState.get(telemetryKeys.hasResponded) + const lastPromptDate = context.globalState.get(telemetryKeys.lastPromptDate) + if (unpersistedResponse) { + // May still raise popup if user lacks permission or file is corrupted + const didSave = await saveTelemetryResponse(unpersistedResponse, cfnTelemetrySettings) + await context.globalState.update(telemetryKeys.unpersistedResponse, undefined) + // If we still couldn't save, clear everything so they get asked again until the file/perms is fixed + if (!didSave) { + getLogger().warn( + 'CloudFormation telemetry choice was not saved successfully after restart. Clearing related globalState keys for next restart' + ) + await context.globalState.update(telemetryKeys.hasResponded, undefined) + await context.globalState.update(telemetryKeys.lastPromptDate, undefined) + } + return logAndReturnTelemetryChoice( + unpersistedResponse === TelemetryChoice.Allow.toString(), + hasResponded, + lastPromptDate + ) } - const hasResponded = context.globalState.get(commandKey('telemetry.hasResponded'), false) - const lastPromptDate = context.globalState.get(commandKey('telemetry.lastPromptDate'), 0) - const now = Date.now() - const thirtyDaysMs = 30 * 24 * 60 * 60 * 1000 + // Never throws because we provide a default + const telemetryEnabled = cfnTelemetrySettings.get(telemetrySettings.enabled, false) + + if (isAutomation()) { + return logAndReturnTelemetryChoice(telemetryEnabled) + } // If user has permanently responded, use their choice if (hasResponded) { - return telemetryEnabled + return logAndReturnTelemetryChoice(telemetryEnabled, hasResponded) } // Check if we should show reminder (30 days since last prompt) - const shouldPrompt = lastPromptDate === 0 || now - lastPromptDate >= thirtyDaysMs + const shouldPrompt = lastPromptDate === undefined || globals.clock.Date.now() - lastPromptDate >= thirtyDaysMs if (!shouldPrompt) { - return telemetryEnabled + return logAndReturnTelemetryChoice(telemetryEnabled, hasResponded, lastPromptDate) + } + + // Show prompt but set false if timeout + const promptPromise = promptTelemetryOptIn(context, cfnTelemetrySettings) + const timeoutPromise = new Promise((resolve) => + globals.clock.setTimeout(() => resolve(false), promptTimeoutMs) + ) + const result = await Promise.race([promptPromise, timeoutPromise]) + + // Keep prompt alive in background + void promptPromise + + return logAndReturnTelemetryChoice(result) +} +/** + * Updates the telemetry setting. In case of error, the update calls do not throw. + * They instead raise a popup and return false. + * + * @returns boolean whether the save/update was successful + */ +/* eslint-disable aws-toolkits/no-banned-usages */ +async function saveTelemetryResponse( + response: string | undefined, + cfnTelemetrySettings: CloudFormationTelemetrySettings +): Promise { + if (response === TelemetryChoice.Allow) { + return await cfnTelemetrySettings.update(telemetrySettings.enabled, true) + } else if (response === TelemetryChoice.Never) { + return await cfnTelemetrySettings.update(telemetrySettings.enabled, false) + } else if (response === TelemetryChoice.Later) { + return await cfnTelemetrySettings.update(telemetrySettings.enabled, false) } + return false +} +function logAndReturnTelemetryChoice(choice: boolean, hasResponded?: boolean, lastPromptDate?: number): boolean { + getLogger().info( + 'CloudFormation telemetry: choice=%s, hasResponded=%s, lastPromptDate=%s', + choice, + hasResponded, + lastPromptDate + ) + return choice +} + +/* eslint-disable aws-toolkits/no-banned-usages */ +async function promptTelemetryOptIn( + context: ExtensionContext, + cfnTelemetrySettings: CloudFormationTelemetrySettings +): Promise { const message = 'Help us improve the AWS CloudFormation Language Server by sharing anonymous telemetry data with AWS. You can change this preference at any time in aws.cloudformation Settings.' - const allow = 'Yes, Allow' - const later = 'Not Now' - const never = 'Never' - const learnMore = 'Learn More' - const response = await window.showInformationMessage(message, allow, later, never, learnMore) + const response = await window.showInformationMessage( + message, + TelemetryChoice.Allow, + TelemetryChoice.Later, + TelemetryChoice.Never, + TelemetryChoice.LearnMore + ) - if (response === learnMore) { - await env.openExternal( - Uri.parse('https://github.com/aws-cloudformation/cloudformation-languageserver/tree/main/src/telemetry') - ) + if (response === TelemetryChoice.LearnMore) { + await env.openExternal(Uri.parse(telemetryDocsUrl)) return promptTelemetryOptIn(context, cfnTelemetrySettings) } - if (response === allow) { - await cfnTelemetrySettings.update('enabled', true) - await context.globalState.update(commandKey('telemetry.hasResponded'), true) - } else if (response === never) { - await cfnTelemetrySettings.update('enabled', false) - await context.globalState.update(commandKey('telemetry.hasResponded'), true) - } else if (response === later) { - await cfnTelemetrySettings.update('enabled', false) - await context.globalState.update(commandKey('telemetry.lastPromptDate'), now) + const now = globals.clock.Date.now() + await context.globalState.update(telemetryKeys.lastPromptDate, now) + + // There's a chance our settings aren't registered yet from package.json, so we + // see if we can persist to settings first + try { + // Throws (with no popup) if setting is not registered + cfnTelemetrySettings.get(telemetrySettings.enabled) + } catch (err) { + getLogger().warn(err as Error) + // Save the choice in globalState and save to settings next time handleTelemetryOptIn is called + await context.globalState.update(telemetryKeys.unpersistedResponse, response) + if (response === TelemetryChoice.Allow) { + await context.globalState.update(telemetryKeys.hasResponded, true) + return true + } else if (response === TelemetryChoice.Never) { + await context.globalState.update(telemetryKeys.hasResponded, true) + return false + } else if (response === TelemetryChoice.Later) { + return false + } } - return cfnTelemetrySettings.get('enabled', false) + // At this point should be able to save and get successfully + await saveTelemetryResponse(response, cfnTelemetrySettings) + await context.globalState.update(telemetryKeys.hasResponded, response !== TelemetryChoice.Later) + return cfnTelemetrySettings.get(telemetrySettings.enabled, false) } diff --git a/packages/core/src/test/awsService/cloudformation/telemetryOptIn.test.ts b/packages/core/src/test/awsService/cloudformation/telemetryOptIn.test.ts new file mode 100644 index 00000000000..d28fde1dbf6 --- /dev/null +++ b/packages/core/src/test/awsService/cloudformation/telemetryOptIn.test.ts @@ -0,0 +1,121 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import assert from 'assert' +import * as sinon from 'sinon' +import { ExtensionContext } from 'vscode' +import { handleTelemetryOptIn } from '../../../awsService/cloudformation/telemetryOptIn' +import { CloudFormationTelemetrySettings } from '../../../awsService/cloudformation/extensionConfig' +import { commandKey } from '../../../awsService/cloudformation/utils' +import globals from '../../../shared/extensionGlobals' + +describe('telemetryOptIn', function () { + let mockContext: ExtensionContext + let mockSettings: CloudFormationTelemetrySettings + let globalState: Map + + beforeEach(function () { + globalState = new Map() + + mockContext = { + globalState: { + get: (key: string, defaultValue?: any) => globalState.get(key) ?? defaultValue, + update: async (key: string, value: any) => { + globalState.set(key, value) + }, + }, + } as any + + mockSettings = { + get: sinon.stub().returns(false), + update: sinon.stub().resolves(), + } as any + }) + + describe('promptTelemetryOptIn - automation mode', function () { + it('should return current setting without prompting in automation mode', async function () { + sinon.stub(require('../../../shared/vscode/env'), 'isAutomation').returns(true) + ;(mockSettings.get as sinon.SinonStub).returns(true) + + const result = await handleTelemetryOptIn(mockContext, mockSettings) + + assert.strictEqual(result, true) + }) + }) + + describe('promptTelemetryOptIn - user has responded', function () { + it('should return current setting if user has permanently responded', async function () { + globalState.set(commandKey('telemetry.hasResponded'), true) + ;(mockSettings.get as sinon.SinonStub).returns(true) + + const result = await handleTelemetryOptIn(mockContext, mockSettings) + + assert.strictEqual(result, true) + }) + }) + + describe('promptTelemetryOptIn - prompt timing', function () { + it('should not prompt if less than 30 days since last prompt', async function () { + const now = globals.clock.Date.now() + const twentyDaysAgo = now - 20 * 24 * 60 * 60 * 1000 + globalState.set(commandKey('telemetry.lastPromptDate'), twentyDaysAgo) + + const result = await handleTelemetryOptIn(mockContext, mockSettings) + + assert.strictEqual(result, false) + }) + }) + + describe('promptTelemetryOptIn - unpersisted response', function () { + it('should persist unpersisted Allow response', async function () { + globalState.set(commandKey('telemetry.unpersistedResponse'), 'Yes, Allow') + + const result = await handleTelemetryOptIn(mockContext, mockSettings) + + assert.strictEqual(result, true) + assert.ok((mockSettings.update as sinon.SinonStub).calledWith('enabled', true)) + assert.strictEqual(globalState.get(commandKey('telemetry.unpersistedResponse')), undefined) + }) + + it('should persist unpersisted Never response', async function () { + globalState.set(commandKey('telemetry.unpersistedResponse'), 'Never') + ;(mockSettings.update as sinon.SinonStub).resolves(true) + + const result = await handleTelemetryOptIn(mockContext, mockSettings) + + assert.strictEqual(result, false) + assert.ok((mockSettings.update as sinon.SinonStub).calledWith('enabled', false)) + assert.strictEqual(globalState.get(commandKey('telemetry.unpersistedResponse')), undefined) + }) + + it('should persist unpersisted Later response', async function () { + const lastPromptDate = globals.clock.Date.now() - 1000 + globalState.set(commandKey('telemetry.unpersistedResponse'), 'Not Now') + globalState.set(commandKey('telemetry.lastPromptDate'), lastPromptDate) + ;(mockSettings.update as sinon.SinonStub).resolves(true) + + const result = await handleTelemetryOptIn(mockContext, mockSettings) + + assert.strictEqual(result, false) + assert.ok((mockSettings.update as sinon.SinonStub).calledWith('enabled', false)) + assert.strictEqual(globalState.get(commandKey('telemetry.lastPromptDate')), lastPromptDate) + assert.strictEqual(globalState.get(commandKey('telemetry.unpersistedResponse')), undefined) + }) + + it('should clear all state if setting save fails', async function () { + globalState.set(commandKey('telemetry.unpersistedResponse'), 'Yes, Allow') + globalState.set(commandKey('telemetry.hasResponded'), true) + globalState.set(commandKey('telemetry.lastPromptDate'), globals.clock.Date.now()) + ;(mockSettings.update as sinon.SinonStub).resolves(false) + + const result = await handleTelemetryOptIn(mockContext, mockSettings) + + assert.strictEqual(result, true) + assert.strictEqual(globalState.get(commandKey('telemetry.unpersistedResponse')), undefined) + assert.strictEqual(globalState.get(commandKey('telemetry.hasResponded')), undefined) + assert.strictEqual(globalState.get(commandKey('telemetry.lastPromptDate')), undefined) + }) + }) +}) diff --git a/packages/toolkit/.changes/next-release/Bug Fix-6113ced5-5e07-4662-a641-db4588ddf8de.json b/packages/toolkit/.changes/next-release/Bug Fix-6113ced5-5e07-4662-a641-db4588ddf8de.json new file mode 100644 index 00000000000..290316e0e0b --- /dev/null +++ b/packages/toolkit/.changes/next-release/Bug Fix-6113ced5-5e07-4662-a641-db4588ddf8de.json @@ -0,0 +1,4 @@ +{ + "type": "Bug Fix", + "description": "CloudFormation: Handle telemetry setting in upgrade path case where setting is not registered" +}