From 8ece808e38b839e27df4ca76efecca86ed7561c0 Mon Sep 17 00:00:00 2001 From: Nikolas Komonen <118216176+nkomonen-amazon@users.noreply.github.com> Date: Fri, 25 Apr 2025 12:14:49 -0400 Subject: [PATCH 1/3] fix(amazonq): Warn user Developer Profile not selected (#7160) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem: If a user has not selected Q Developer Profile after signing in, their features will not work. Some existing users, before the introduction of Q Developer Profiles, who were already signed in had their features stop working because they didn't select a profile. ## Solution: On startup if we detect the user did not select a profile, then send a warning message that their features will not work until they select one. The message will have a button to allow them to select a profile through quickpick, or entirly ignore the message and not show it again. Screenshot 2025-04-24 at 5 42 51 PM --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: nkomonen-amazon --- ...-489e72aa-bb0d-4964-bd84-002f25db6b5f.json | 4 ++ packages/amazonq/package.json | 4 ++ packages/core/src/codewhisperer/activation.ts | 5 ++ .../core/src/codewhisperer/commands/types.ts | 3 ++ .../core/src/codewhisperer/region/utils.ts | 49 +++++++++++++++++++ .../core/src/codewhisperer/util/authUtil.ts | 1 + .../core/src/shared/settings-amazonq.gen.ts | 3 +- 7 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 packages/amazonq/.changes/next-release/Bug Fix-489e72aa-bb0d-4964-bd84-002f25db6b5f.json create mode 100644 packages/core/src/codewhisperer/region/utils.ts diff --git a/packages/amazonq/.changes/next-release/Bug Fix-489e72aa-bb0d-4964-bd84-002f25db6b5f.json b/packages/amazonq/.changes/next-release/Bug Fix-489e72aa-bb0d-4964-bd84-002f25db6b5f.json new file mode 100644 index 00000000000..0cd71188f81 --- /dev/null +++ b/packages/amazonq/.changes/next-release/Bug Fix-489e72aa-bb0d-4964-bd84-002f25db6b5f.json @@ -0,0 +1,4 @@ +{ + "type": "Bug Fix", + "description": "Toast message to warn users if Developer Profile is not selected" +} diff --git a/packages/amazonq/package.json b/packages/amazonq/package.json index 20fdc3610ef..cb74db00761 100644 --- a/packages/amazonq/package.json +++ b/packages/amazonq/package.json @@ -131,6 +131,10 @@ "amazonQChatDisclaimer": { "type": "boolean", "default": false + }, + "amazonQSelectDeveloperProfile": { + "type": "boolean", + "default": false } }, "additionalProperties": false diff --git a/packages/core/src/codewhisperer/activation.ts b/packages/core/src/codewhisperer/activation.ts index efebb01e179..b0aa54e17a0 100644 --- a/packages/core/src/codewhisperer/activation.ts +++ b/packages/core/src/codewhisperer/activation.ts @@ -95,6 +95,7 @@ import { SecurityIssueTreeViewProvider } from './service/securityIssueTreeViewPr import { setContext } from '../shared/vscode/setContext' import { syncSecurityIssueWebview } from './views/securityIssue/securityIssueWebview' import { detectCommentAboveLine } from '../shared/utilities/commentUtils' +import { notifySelectDeveloperProfile } from './region/utils' let localize: nls.LocalizeFunc @@ -380,6 +381,10 @@ export async function activate(context: ExtContext): Promise { await auth.notifySessionConfiguration() } } + + if (auth.requireProfileSelection()) { + await notifySelectDeveloperProfile() + } }, { emit: false, functionId: { name: 'activateCwCore' } } ) diff --git a/packages/core/src/codewhisperer/commands/types.ts b/packages/core/src/codewhisperer/commands/types.ts index e211ae76f9a..cec28829507 100644 --- a/packages/core/src/codewhisperer/commands/types.ts +++ b/packages/core/src/codewhisperer/commands/types.ts @@ -18,6 +18,8 @@ export const firstStartUpSource = ExtStartUpSources.firstStartUp export const cwEllipsesMenu = 'ellipsesMenu' /** Indicates a CodeWhisperer command was executed from the command palette */ export const commandPalette = 'commandPalette' +/** Indicates a CodeWhisperer command was executed as a result of a toast message interaction */ +export const toastMessage = 'toastMessage' /** * Indicates what caused the CodeWhisperer command to be executed, since a command can be executed from different "sources" @@ -35,3 +37,4 @@ export type CodeWhispererSource = | typeof firstStartUpSource | typeof cwEllipsesMenu | typeof commandPalette + | typeof toastMessage diff --git a/packages/core/src/codewhisperer/region/utils.ts b/packages/core/src/codewhisperer/region/utils.ts new file mode 100644 index 00000000000..dd988f74a30 --- /dev/null +++ b/packages/core/src/codewhisperer/region/utils.ts @@ -0,0 +1,49 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ +import * as nls from 'vscode-nls' +const localize = nls.loadMessageBundle() +import { AmazonQPromptSettings } from '../../shared/settings' +import { telemetry } from '../../shared/telemetry/telemetry' +import vscode from 'vscode' +import { selectRegionProfileCommand } from '../commands/basicCommands' +import { placeholder } from '../../shared/vscode/commands2' +import { toastMessage } from '../commands/types' + +/** + * Creates a toast message telling the user they need to select a Developer Profile + */ +export async function notifySelectDeveloperProfile() { + const suppressId = 'amazonQSelectDeveloperProfile' + const settings = AmazonQPromptSettings.instance + const shouldShow = settings.isPromptEnabled(suppressId) + if (!shouldShow) { + return + } + + const message = localize( + 'aws.amazonq.profile.mustSelectMessage', + 'You must select a Q Developer Profile for Amazon Q features to work.' + ) + const selectProfile = 'Select Profile' + const dontShowAgain = 'Dont Show Again' + + await telemetry.toolkit_showNotification.run(async () => { + telemetry.record({ id: 'mustSelectDeveloperProfileMessage' }) + void vscode.window.showWarningMessage(message, selectProfile, dontShowAgain).then(async (resp) => { + await telemetry.toolkit_invokeAction.run(async () => { + if (resp === selectProfile) { + // Show Profile + telemetry.record({ action: 'select' }) + void selectRegionProfileCommand.execute(placeholder, toastMessage) + } else if (resp === dontShowAgain) { + telemetry.record({ action: 'dontShowAgain' }) + await settings.disablePrompt(suppressId) + } else { + telemetry.record({ action: 'ignore' }) + } + }) + }) + }) +} diff --git a/packages/core/src/codewhisperer/util/authUtil.ts b/packages/core/src/codewhisperer/util/authUtil.ts index 0898493b6db..f4cc90a5293 100644 --- a/packages/core/src/codewhisperer/util/authUtil.ts +++ b/packages/core/src/codewhisperer/util/authUtil.ts @@ -46,6 +46,7 @@ import { withTelemetryContext } from '../../shared/telemetry/util' import { focusAmazonQPanel } from '../../codewhispererChat/commands/registerCommands' import { throttle } from 'lodash' import { RegionProfileManager } from '../region/regionProfileManager' + /** Backwards compatibility for connections w pre-chat scopes */ export const codeWhispererCoreScopes = [...scopesCodeWhispererCore] export const codeWhispererChatScopes = [...codeWhispererCoreScopes, ...scopesCodeWhispererChat] diff --git a/packages/core/src/shared/settings-amazonq.gen.ts b/packages/core/src/shared/settings-amazonq.gen.ts index f0a3d47f989..bee57f9aa82 100644 --- a/packages/core/src/shared/settings-amazonq.gen.ts +++ b/packages/core/src/shared/settings-amazonq.gen.ts @@ -21,7 +21,8 @@ export const amazonqSettings = { "ssoCacheError": {}, "amazonQLspManifestMessage": {}, "amazonQWorkspaceLspManifestMessage": {}, - "amazonQChatDisclaimer": {} + "amazonQChatDisclaimer": {}, + "amazonQSelectDeveloperProfile": {} }, "amazonQ.showCodeWithReferences": {}, "amazonQ.allowFeatureDevelopmentToRunCodeAndTests": {}, From 638778beee1a88783753a93c70766c2ee55b789d Mon Sep 17 00:00:00 2001 From: Hweinstock <42325418+Hweinstock@users.noreply.github.com> Date: Fri, 25 Apr 2025 12:48:47 -0400 Subject: [PATCH 2/3] feat(core): add value length cap to partialClone (#7150) ## Problem If we want to log a large json object with giant strings (think whole files), it can make the logs difficult to read. This is especially relevant when the underlying string values are not very important. ## Solution - allow the string values to be truncated with `maxStringLength` option. ## Notes I am planning to port this utility to Flare to improve the logging experience there, and want this functionality to exist there however I thought this could be useful here too. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --- .../amazonq/test/e2e/inline/inline.test.ts | 2 +- packages/core/src/auth/sso/clients.ts | 4 +- .../src/shared/utilities/collectionUtils.ts | 21 ++++- .../src/shared/utilities/textUtilities.ts | 2 +- packages/core/src/shared/vscode/commands2.ts | 2 +- .../shared/utilities/collectionUtils.test.ts | 91 ++++++++++++------- 6 files changed, 80 insertions(+), 42 deletions(-) diff --git a/packages/amazonq/test/e2e/inline/inline.test.ts b/packages/amazonq/test/e2e/inline/inline.test.ts index 57c6e1c4996..43a9f67ab73 100644 --- a/packages/amazonq/test/e2e/inline/inline.test.ts +++ b/packages/amazonq/test/e2e/inline/inline.test.ts @@ -122,7 +122,7 @@ describe('Amazon Q Inline', async function () { .query({ metricName: 'codewhisperer_userTriggerDecision', }) - .map((e) => collectionUtil.partialClone(e, 3, ['credentialStartUrl'], '[omitted]')) + .map((e) => collectionUtil.partialClone(e, 3, ['credentialStartUrl'], { replacement: '[omitted]' })) } for (const [name, invokeCompletion] of [ diff --git a/packages/core/src/auth/sso/clients.ts b/packages/core/src/auth/sso/clients.ts index 01d0e031d04..e050bdc793e 100644 --- a/packages/core/src/auth/sso/clients.ts +++ b/packages/core/src/auth/sso/clients.ts @@ -258,7 +258,7 @@ function addLoggingMiddleware(client: SSOOIDCClient) { args.input as unknown as Record, 3, ['clientSecret', 'accessToken', 'refreshToken'], - '[omitted]' + { replacement: '[omitted]' } ) getLogger().debug('API request (%s %s): %O', hostname, path, input) } @@ -288,7 +288,7 @@ function addLoggingMiddleware(client: SSOOIDCClient) { result.output as unknown as Record, 3, ['clientSecret', 'accessToken', 'refreshToken'], - '[omitted]' + { replacement: '[omitted]' } ) getLogger().debug('API response (%s %s): %O', hostname, path, output) } diff --git a/packages/core/src/shared/utilities/collectionUtils.ts b/packages/core/src/shared/utilities/collectionUtils.ts index 9f9fe9875b9..8a428b8e8b7 100644 --- a/packages/core/src/shared/utilities/collectionUtils.ts +++ b/packages/core/src/shared/utilities/collectionUtils.ts @@ -7,6 +7,7 @@ import { isWeb } from '../extensionGlobals' import { inspect as nodeInspect } from 'util' import { AsyncCollection, toCollection } from './asyncCollection' import { SharedProp, AccumulableKeys, Coalesce, isNonNullable } from './tsUtils' +import { truncate } from './textUtilities' export function union(a: Iterable, b: Iterable): Set { const result = new Set() @@ -304,10 +305,22 @@ export function assign, U extends Partial>(data: T * @param depth * @param omitKeys Omit properties matching these names (at any depth). * @param replacement Replacement for object whose fields extend beyond `depth`, and properties matching `omitKeys`. + * @param maxStringLength truncates string values that exceed this threshold (includes values in nested arrays) */ -export function partialClone(obj: any, depth: number = 3, omitKeys: string[] = [], replacement?: any): any { +export function partialClone( + obj: any, + depth: number = 3, + omitKeys: string[] = [], + options?: { + replacement?: any + maxStringLength?: number + } +): any { // Base case: If input is not an object or has no children, return it. if (typeof obj !== 'object' || obj === null || 0 === Object.getOwnPropertyNames(obj).length) { + if (typeof obj === 'string' && options?.maxStringLength) { + return truncate(obj, options?.maxStringLength, '...') + } return obj } @@ -315,15 +328,15 @@ export function partialClone(obj: any, depth: number = 3, omitKeys: string[] = [ const clonedObj = Array.isArray(obj) ? [] : {} if (depth === 0) { - return replacement ? replacement : clonedObj + return options?.replacement ? options.replacement : clonedObj } // Recursively clone properties of the input object for (const key in obj) { if (omitKeys.includes(key)) { - ;(clonedObj as any)[key] = replacement ? replacement : Array.isArray(obj) ? [] : {} + ;(clonedObj as any)[key] = options?.replacement ? options.replacement : Array.isArray(obj) ? [] : {} } else if (Object.prototype.hasOwnProperty.call(obj, key)) { - ;(clonedObj as any)[key] = partialClone(obj[key], depth - 1, omitKeys, replacement) + ;(clonedObj as any)[key] = partialClone(obj[key], depth - 1, omitKeys, options) } } diff --git a/packages/core/src/shared/utilities/textUtilities.ts b/packages/core/src/shared/utilities/textUtilities.ts index 53c2e2be32c..ed1e1619122 100644 --- a/packages/core/src/shared/utilities/textUtilities.ts +++ b/packages/core/src/shared/utilities/textUtilities.ts @@ -10,7 +10,7 @@ import { default as stripAnsi } from 'strip-ansi' import { getLogger } from '../logger/logger' /** - * Truncates string `s` if it exceeds `n` chars. + * Truncates string `s` if it has or exceeds `n` chars. * * If `n` is negative, truncates at start instead of end. * diff --git a/packages/core/src/shared/vscode/commands2.ts b/packages/core/src/shared/vscode/commands2.ts index c55cd66cc7a..b40134c2afa 100644 --- a/packages/core/src/shared/vscode/commands2.ts +++ b/packages/core/src/shared/vscode/commands2.ts @@ -653,7 +653,7 @@ async function runCommand(fn: T, info: CommandInfo): Prom logger.debug( `command: running ${label} with arguments: %O`, - partialClone(args, 3, ['clientSecret', 'accessToken', 'refreshToken', 'tooltip'], '[omitted]') + partialClone(args, 3, ['clientSecret', 'accessToken', 'refreshToken', 'tooltip'], { replacement: '[omitted]' }) ) try { diff --git a/packages/core/src/test/shared/utilities/collectionUtils.test.ts b/packages/core/src/test/shared/utilities/collectionUtils.test.ts index 53ddc39eff8..34aacb9f28e 100644 --- a/packages/core/src/test/shared/utilities/collectionUtils.test.ts +++ b/packages/core/src/test/shared/utilities/collectionUtils.test.ts @@ -710,8 +710,10 @@ describe('CollectionUtils', async function () { }) describe('partialClone', function () { - it('omits properties by depth', function () { - const testObj = { + let multipleTypedObj: object + + before(async function () { + multipleTypedObj = { a: 34234234234, b: '123456789', c: new Date(2023, 1, 1), @@ -724,57 +726,80 @@ describe('CollectionUtils', async function () { throw Error() }, } + }) - assert.deepStrictEqual(partialClone(testObj, 1), { - ...testObj, + it('omits properties by depth', function () { + assert.deepStrictEqual(partialClone(multipleTypedObj, 1), { + ...multipleTypedObj, d: {}, e: {}, }) - assert.deepStrictEqual(partialClone(testObj, 0, [], '[replaced]'), '[replaced]') - assert.deepStrictEqual(partialClone(testObj, 1, [], '[replaced]'), { - ...testObj, + assert.deepStrictEqual(partialClone(multipleTypedObj, 0, [], { replacement: '[replaced]' }), '[replaced]') + assert.deepStrictEqual(partialClone(multipleTypedObj, 1, [], { replacement: '[replaced]' }), { + ...multipleTypedObj, d: '[replaced]', e: '[replaced]', }) - assert.deepStrictEqual(partialClone(testObj, 3), { - ...testObj, + assert.deepStrictEqual(partialClone(multipleTypedObj, 3), { + ...multipleTypedObj, d: { d1: { d2: {} } }, }) - assert.deepStrictEqual(partialClone(testObj, 4), testObj) + assert.deepStrictEqual(partialClone(multipleTypedObj, 4), multipleTypedObj) }) it('omits properties by name', function () { - const testObj = { - a: 34234234234, - b: '123456789', - c: new Date(2023, 1, 1), - d: { d1: { d2: { d3: 'deep' } } }, + assert.deepStrictEqual(partialClone(multipleTypedObj, 2, ['c', 'e2'], { replacement: '[replaced]' }), { + ...multipleTypedObj, + c: '[replaced]', + d: { d1: '[replaced]' }, + e: { + e1: '[replaced]', + e2: '[replaced]', + }, + }) + assert.deepStrictEqual(partialClone(multipleTypedObj, 3, ['c', 'e2'], { replacement: '[replaced]' }), { + ...multipleTypedObj, + c: '[replaced]', + d: { d1: { d2: '[replaced]' } }, e: { e1: [4, 3, 7], - e2: 'loooooooooo \n nnnnnnnnnnn \n gggggggg \n string', + e2: '[replaced]', }, - f: () => { - throw Error() + }) + }) + + it('truncates properties by maxLength', function () { + const testObj = { + strValue: '1', + boolValue: true, + longString: '11111', + nestedObj: { + nestedObjAgain: { + longNestedStr: '11111', + shortNestedStr: '11', + }, + }, + nestedObj2: { + functionValue: (_: unknown) => {}, }, + nestedObj3: { + myArray: ['1', '11111', '1'], + }, + objInArray: [{ shortString: '11', longString: '11111' }], } - - assert.deepStrictEqual(partialClone(testObj, 2, ['c', 'e2'], '[omitted]'), { + assert.deepStrictEqual(partialClone(testObj, 5, [], { maxStringLength: 2 }), { ...testObj, - c: '[omitted]', - d: { d1: '[omitted]' }, - e: { - e1: '[omitted]', - e2: '[omitted]', + longString: '11...', + nestedObj: { + nestedObjAgain: { + longNestedStr: '11...', + shortNestedStr: '11', + }, }, - }) - assert.deepStrictEqual(partialClone(testObj, 3, ['c', 'e2'], '[omitted]'), { - ...testObj, - c: '[omitted]', - d: { d1: { d2: '[omitted]' } }, - e: { - e1: [4, 3, 7], - e2: '[omitted]', + nestedObj3: { + myArray: ['1', '11...', '1'], }, + objInArray: [{ shortString: '11', longString: '11...' }], }) }) }) From 3549e338d8209cdcf4d2e24fe9c6796dcf828e64 Mon Sep 17 00:00:00 2001 From: Will Lo <96078566+Will-ShaoHua@users.noreply.github.com> Date: Fri, 25 Apr 2025 12:00:27 -0700 Subject: [PATCH 3/3] fix(amazonq): fix enterprise users not able to sign in correctly if they have 2+ vscode instances open (#7151) ## Problem revision of #7134, this pr aims to address the comment from the previous PR https://github.com/aws/aws-toolkit-vscode/pull/7134#discussion_r2056518840 to extract the logic to a shared module so the diff against 7134 is expected to be large. after deployment 4/21, service has a strict 1 tps throttling policy and there was no caching for the API call previously. It will impact users as long as they have multiple ide instances opened as all of them will make list profile call while users attempt to sign in. ## Solution 1. cache the api result for 60 seconds for reuse 2. use lock to prevent multiple instances trying to call at the same time. Only 1 will make the service call and the rest will wait until it's done and read from the cache. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --- ...-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json | 4 + .../region/regionProfileManager.ts | 35 +++- .../core/src/codewhisperer/util/authUtil.ts | 2 + .../webview/vue/amazonq/backend_amazonq.ts | 2 +- packages/core/src/shared/globalState.ts | 1 + packages/core/src/shared/logger/logger.ts | 1 + .../src/shared/utilities/resourceCache.ts | 190 ++++++++++++++++++ 7 files changed, 232 insertions(+), 3 deletions(-) create mode 100644 packages/amazonq/.changes/next-release/Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json create mode 100644 packages/core/src/shared/utilities/resourceCache.ts diff --git a/packages/amazonq/.changes/next-release/Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json b/packages/amazonq/.changes/next-release/Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json new file mode 100644 index 00000000000..087dd85d88d --- /dev/null +++ b/packages/amazonq/.changes/next-release/Bug Fix-8290a06f-ee3f-4235-b8af-faedb1bdfbe4.json @@ -0,0 +1,4 @@ +{ + "type": "Bug Fix", + "description": "Fix users can not log in successfully with 2+ IDE instnaces open due to throttle error throw by the service" +} diff --git a/packages/core/src/codewhisperer/region/regionProfileManager.ts b/packages/core/src/codewhisperer/region/regionProfileManager.ts index 039d5cefb59..a28ac46ee1c 100644 --- a/packages/core/src/codewhisperer/region/regionProfileManager.ts +++ b/packages/core/src/codewhisperer/region/regionProfileManager.ts @@ -29,6 +29,7 @@ import { isAwsError, ToolkitError } from '../../shared/errors' import { telemetry } from '../../shared/telemetry/telemetry' import { localize } from '../../shared/utilities/vsCodeUtils' import { Commands } from '../../shared/vscode/commands2' +import { CachedResource } from '../../shared/utilities/resourceCache' // TODO: is there a better way to manage all endpoint strings in one place? export const defaultServiceConfig: CodeWhispererConfig = { @@ -59,6 +60,27 @@ export class RegionProfileManager { // Store the last API results (for UI propuse) so we don't need to call service again if doesn't require "latest" result private _profiles: RegionProfile[] = [] + private readonly cache = new (class extends CachedResource { + constructor(private readonly profileProvider: () => Promise) { + super( + 'aws.amazonq.regionProfiles.cache', + 60000, + { + resource: { + locked: false, + timestamp: 0, + result: undefined, + }, + }, + { timeout: 15000, interval: 1500, truthy: true } + ) + } + + override resourceProvider(): Promise { + return this.profileProvider() + } + })(this.listRegionProfile.bind(this)) + get activeRegionProfile() { const conn = this.connectionProvider() if (isBuilderIdConnection(conn)) { @@ -104,6 +126,10 @@ export class RegionProfileManager { constructor(private readonly connectionProvider: () => Connection | undefined) {} + async getProfiles(): Promise { + return this.cache.getResource() + } + async listRegionProfile(): Promise { this._profiles = [] @@ -238,7 +264,7 @@ export class RegionProfileManager { return } // cross-validation - this.listRegionProfile() + this.getProfiles() .then(async (profiles) => { const r = profiles.find((it) => it.arn === previousSelected.arn) if (!r) { @@ -300,7 +326,7 @@ export class RegionProfileManager { const selected = this.activeRegionProfile let profiles: RegionProfile[] = [] try { - profiles = await this.listRegionProfile() + profiles = await this.getProfiles() } catch (e) { return [ { @@ -347,6 +373,11 @@ export class RegionProfileManager { } } + // Should be called on connection changed in case users change to a differnet connection and use the wrong resultset. + async clearCache() { + await this.cache.clearCache() + } + async createQClient(region: string, endpoint: string, conn: SsoConnection): Promise { const token = (await conn.getToken()).accessToken const serviceOption: ServiceOptions = { diff --git a/packages/core/src/codewhisperer/util/authUtil.ts b/packages/core/src/codewhisperer/util/authUtil.ts index f4cc90a5293..2f8843d754b 100644 --- a/packages/core/src/codewhisperer/util/authUtil.ts +++ b/packages/core/src/codewhisperer/util/authUtil.ts @@ -144,6 +144,8 @@ export class AuthUtil { if (!this.isConnected()) { await this.regionProfileManager.invalidateProfile(this.regionProfileManager.activeRegionProfile?.arn) } + + await this.regionProfileManager.clearCache() }) this.regionProfileManager.onDidChangeRegionProfile(async () => { diff --git a/packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts b/packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts index 0853f80e952..6ed152ab4ea 100644 --- a/packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts +++ b/packages/core/src/login/webview/vue/amazonq/backend_amazonq.ts @@ -223,7 +223,7 @@ export class AmazonQLoginWebview extends CommonAuthWebview { */ override async listRegionProfiles(): Promise { try { - return await AuthUtil.instance.regionProfileManager.listRegionProfile() + return await AuthUtil.instance.regionProfileManager.getProfiles() } catch (e) { const conn = AuthUtil.instance.conn as SsoConnection | undefined telemetry.amazonq_didSelectProfile.emit({ diff --git a/packages/core/src/shared/globalState.ts b/packages/core/src/shared/globalState.ts index 44d848ec69d..6ecca56f90a 100644 --- a/packages/core/src/shared/globalState.ts +++ b/packages/core/src/shared/globalState.ts @@ -49,6 +49,7 @@ export type globalKey = | 'aws.toolkit.lsp.manifest' | 'aws.amazonq.customization.overrideV2' | 'aws.amazonq.regionProfiles' + | 'aws.amazonq.regionProfiles.cache' // Deprecated/legacy names. New keys should start with "aws.". | '#sessionCreationDates' // Legacy name from `ssoAccessTokenProvider.ts`. | 'CODECATALYST_RECONNECT' diff --git a/packages/core/src/shared/logger/logger.ts b/packages/core/src/shared/logger/logger.ts index 9b4bead6a37..eac564b9c35 100644 --- a/packages/core/src/shared/logger/logger.ts +++ b/packages/core/src/shared/logger/logger.ts @@ -18,6 +18,7 @@ export type LogTopic = | 'chat' | 'stepfunctions' | 'unknown' + | 'resourceCache' class ErrorLog { constructor( diff --git a/packages/core/src/shared/utilities/resourceCache.ts b/packages/core/src/shared/utilities/resourceCache.ts new file mode 100644 index 00000000000..8189f40a4c5 --- /dev/null +++ b/packages/core/src/shared/utilities/resourceCache.ts @@ -0,0 +1,190 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import globals from '../extensionGlobals' +import { globalKey } from '../globalState' +import { getLogger } from '../logger/logger' +import { waitUntil } from '../utilities/timeoutUtils' + +/** + * args: + * @member result: the actual resource type callers want to use + * @member locked: readWriteLock, while the lock is acquired by one process, the other can't access to it until it's released by the previous + * @member timestamp: used for determining the resource is stale or not + */ +interface Resource { + result: V | undefined + locked: boolean + timestamp: number +} + +/** + * GlobalStates schema, which is used for vscode global states deserialization, [globals.globalState#tryGet] etc. + * The purpose of it is to allow devs to overload the resource into existing global key and no need to create a specific key for only this purpose. + */ +export interface GlobalStateSchema { + resource: Resource +} + +const logger = getLogger('resourceCache') + +function now() { + return globals.clock.Date.now() +} + +/** + * CacheResource utilizes VSCode global states API to cache resources which are expensive to get so that the result can be shared across multiple VSCode instances. + * The first VSCode instance invoking #getResource will hold a lock and make the actual network call/FS read to pull the real response. + * When the pull is done, the lock will be released and it then caches the result in the global states. Then the rest of instances can now acquire the lock 1 by 1 and read the resource from the cache. + * + * constructor: + * @param key: global state key, which is used for globals.globalState#update, #tryGet etc. + * @param expirationInMilli: cache expiration time in milli seconds + * @param defaultValue: default value for the cache if the cache doesn't pre-exist in users' FS + * @param waitUntilOption: waitUntil option for acquire lock + * + * methods: + * @method resourceProvider: implementation needs to implement this method to obtain the latest resource either via network calls or FS read + * @method getResource: obtain the resource from cache or pull the latest from the service if the cache either expires or doesn't exist + */ +export abstract class CachedResource { + constructor( + private readonly key: globalKey, + private readonly expirationInMilli: number, + private readonly defaultValue: GlobalStateSchema, + private readonly waitUntilOption: { timeout: number; interval: number; truthy: boolean } + ) {} + + abstract resourceProvider(): Promise + + async getResource(): Promise { + const cachedValue = await this.tryLoadResourceAndLock() + const resource = cachedValue?.resource + + // If cache is still fresh, return cached result, otherwise pull latest from the service + if (cachedValue && resource && resource.result) { + const duration = now() - resource.timestamp + if (duration < this.expirationInMilli) { + logger.debug( + `cache hit, duration(%sms) is less than expiration(%sms), returning cached value %s`, + duration, + this.expirationInMilli, + this.key + ) + // release the lock + await this.releaseLock(resource, cachedValue) + return resource.result + } else { + logger.debug( + `cache is stale, duration(%sms) is older than expiration(%sms), pulling latest resource %s`, + duration, + this.expirationInMilli, + this.key + ) + } + } else { + logger.info(`cache miss, pulling latest resource %s`, this.key) + } + + /** + * Possible paths here + * 1. cache doesn't exist. + * 2. cache exists but expired. + * 3. lock is held by other process and the waiting time is greater than the specified waiting time + */ + try { + // Make the real network call / FS read to pull the resource + const latest = await this.resourceProvider() + + // Update resource cache and release the lock + const r: Resource = { + locked: false, + timestamp: now(), + result: latest, + } + logger.info(`doen loading latest resource, updating resource cache: %s`, this.key) + await this.releaseLock(r) + return latest + } catch (e) { + logger.error(`failed to load latest resource, releasing lock: %s`, this.key) + await this.releaseLock() + throw e + } + } + + // This method will lock the resource so other callers have to wait until the lock is released, otherwise will return undefined if it times out + private async tryLoadResourceAndLock(): Promise | undefined> { + const _acquireLock = async () => { + const cachedValue = this.readCacheOrDefault() + + if (!cachedValue.resource.locked) { + await this.lockResource(cachedValue) + return cachedValue + } + + return undefined + } + + const lock = await waitUntil(async () => { + const lock = await _acquireLock() + logger.debug(`try obtain resource cache read write lock for resource %s`, this.key) + if (lock) { + return lock + } + }, this.waitUntilOption) + + return lock + } + + async lockResource(baseCache: GlobalStateSchema): Promise { + await this.updateResourceCache({ locked: true }, baseCache) + } + + async releaseLock(): Promise + async releaseLock(resource: Partial>): Promise + async releaseLock(resource: Partial>, baseCache: GlobalStateSchema): Promise + async releaseLock(resource?: Partial>, baseCache?: GlobalStateSchema): Promise { + if (!resource) { + await this.updateResourceCache({ locked: false }, undefined) + } else if (baseCache) { + await this.updateResourceCache(resource, baseCache) + } else { + await this.updateResourceCache(resource, undefined) + } + } + + async clearCache() { + const baseCache = this.readCacheOrDefault() + await this.updateResourceCache({ result: undefined, timestamp: 0, locked: false }, baseCache) + } + + private async updateResourceCache(resource: Partial>, cache: GlobalStateSchema | undefined) { + const baseCache = cache ?? this.readCacheOrDefault() + + const toUpdate: GlobalStateSchema = { + ...baseCache, + resource: { + ...baseCache.resource, + ...resource, + }, + } + + await globals.globalState.update(this.key, toUpdate) + } + + private readCacheOrDefault(): GlobalStateSchema { + const cachedValue = globals.globalState.tryGet>(this.key, Object, { + ...this.defaultValue, + resource: { + ...this.defaultValue.resource, + locked: false, + result: undefined, + timestamp: 0, + }, + }) + + return cachedValue + } +}