Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/amazonq/src/lsp/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@ import vscode from 'vscode'
import { startLanguageServer } from './client'
import { AmazonQLspInstaller } from './lspInstaller'
import { lspSetupStage, ToolkitError, messages } from 'aws-core-vscode/shared'
import { AuthUtil } from 'aws-core-vscode/codewhisperer'

export async function activate(ctx: vscode.ExtensionContext) {
try {
await lspSetupStage('all', async () => {
const installResult = await new AmazonQLspInstaller().resolve()
return await lspSetupStage('launch', () => startLanguageServer(ctx, installResult.resourcePaths))
})
await AuthUtil.instance.restore()
} catch (err) {
const e = err as ToolkitError
void messages.showViewLogsMessage(`Failed to launch Amazon Q language server: ${e.message}`)
Expand Down
17 changes: 15 additions & 2 deletions packages/amazonq/src/lsp/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,23 @@ export async function startLanguageServer(
toDispose.push(disposable)

await client.onReady()

// IMPORTANT: This sets up Auth and must be called before anything attempts to use it
AuthUtil.create(new auth2.LanguageClientAuth(client, clientId, encryptionKey))
// Ideally this would be part of AuthUtil.create() as it restores the existing Auth connection, but we have some
// future work which will need to delay this call
await AuthUtil.instance.restore()

await postStartLanguageServer(client, resourcePaths, toDispose)

return client
}

async function postStartLanguageServer(
client: LanguageClient,
resourcePaths: AmazonQResourcePaths,
toDispose: vscode.Disposable[]
) {
// Request handler for when the server wants to know about the clients auth connnection. Must be registered before the initial auth init call
client.onRequest<ConnectionMetadata, Error>(auth2.notificationTypes.getConnectionMetadata.method, () => {
return {
Expand Down Expand Up @@ -347,8 +362,6 @@ export async function startLanguageServer(
// Set this inside onReady so that it only triggers on subsequent language server starts (not the first)
onServerRestartHandler(client)
)

return client
}

/**
Expand Down
10 changes: 7 additions & 3 deletions packages/core/src/auth/auth2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,14 @@ export class SsoLogin implements BaseLogin {
}

private updateConnectionState(state: AuthState) {
if (this.connectionState !== state) {
this.eventEmitter.fire({ id: this.profileName, state })
const oldState = this.connectionState
const newState = state

this.connectionState = newState

if (oldState !== newState) {
this.eventEmitter.fire({ id: this.profileName, state: this.connectionState })
Comment on lines +331 to +334
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there no race conditions in swapping the event emitter with the status update?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is unlikely (maybe impossible in this case), but when we fire the event emitter and have not updated this.connectionState to the correct value yet, something may attempt to read it after getting the event and it will be a stale value.

I made this change initially though with the assumption that it may have had no actual impact.

}
this.connectionState = state
}

private ssoTokenChangedHandler(params: SsoTokenChangedParams) {
Expand Down
5 changes: 0 additions & 5 deletions packages/core/src/codewhisperer/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ import { setContext } from '../shared/vscode/setContext'
import { syncSecurityIssueWebview } from './views/securityIssue/securityIssueWebview'
import { detectCommentAboveLine } from '../shared/utilities/commentUtils'
import { activateEditTracking } from './nextEditPrediction/activation'
import { notifySelectDeveloperProfile } from './region/utils'

let localize: nls.LocalizeFunc

Expand Down Expand Up @@ -351,10 +350,6 @@ export async function activate(context: ExtContext): Promise<void> {
await AuthUtil.instance.notifySessionConfiguration()
}
}

if (AuthUtil.instance.regionProfileManager.requireProfileSelection()) {
await notifySelectDeveloperProfile()
}
},
{ emit: false, functionId: { name: 'activateCwCore' } }
)
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/codewhisperer/region/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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'

Expand Down Expand Up @@ -36,7 +35,7 @@ export async function notifySelectDeveloperProfile() {
if (resp === selectProfile) {
// Show Profile
telemetry.record({ action: 'select' })
void selectRegionProfileCommand.execute(placeholder, toastMessage)
void vscode.commands.executeCommand('aws.amazonq.selectRegionProfile', placeholder, toastMessage)
} else if (resp === dontShowAgain) {
telemetry.record({ action: 'dontShowAgain' })
await settings.disablePrompt(suppressId)
Expand Down
42 changes: 38 additions & 4 deletions packages/core/src/codewhisperer/util/authUtil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import { builderIdStartUrl, internalStartUrl } from '../../auth/sso/constants'
import { VSCODE_EXTENSION_ID } from '../../shared/extensions'
import { RegionProfileManager } from '../region/regionProfileManager'
import { AuthFormId } from '../../login/webview/vue/types'
import { notifySelectDeveloperProfile } from '../region/utils'
import { once } from '../../shared/utilities/functionUtils'

const localize = nls.loadMessageBundle()

Expand Down Expand Up @@ -91,9 +93,31 @@ export class AuthUtil implements IAuthProvider {
return this.session.loginType === LoginTypes.SSO
}

/**
* HACK: Ideally we'd put {@link notifySelectDeveloperProfile} in to {@link restore}.
* But because {@link refreshState} is only called if !isConnected, we cannot do it since
* {@link notifySelectDeveloperProfile} needs {@link refreshState} to run so it can set
* the Bearer Token in the LSP first.
*/
didStartSignedIn = false

async restore() {
await this.session.restore()
if (!this.isConnected()) {
this.didStartSignedIn = this.isConnected()

// HACK: We noticed that if calling `refreshState()` here when the user was already signed in, something broke.
// So as a solution we only call it if they were not already signed in.
//
// But in the case where a user was already signed in, we allow `session.restore()` to trigger `refreshState()` through
// event emitters.
// This is unoptimal since `refreshState()` should be able to be called multiple times and still work.
//
// Because of this edge case, when `restore()` is called we cannot assume all Auth is setup when this function returns,
// since we may still be waiting on the event emitter to trigger the expected functions.
//
// TODO: Figure out why removing the if statement below causes things to break. Maybe we just need to
// promisify the call and any subsequent callers will not make a redundant call.
if (!this.didStartSignedIn) {
await this.refreshState()
}
}
Expand Down Expand Up @@ -257,20 +281,24 @@ export class AuthUtil implements IAuthProvider {

private async refreshState(state = this.getAuthState()) {
if (state === 'expired' || state === 'notConnected') {
this.lspAuth.deleteBearerToken()
if (this.isIdcConnection()) {
await this.regionProfileManager.invalidateProfile(this.regionProfileManager.activeRegionProfile?.arn)
await this.regionProfileManager.clearCache()
}
this.lspAuth.deleteBearerToken()
}
if (state === 'connected') {
const bearerTokenParams = (await this.session.getToken()).updateCredentialsParams
await this.lspAuth.updateBearerToken(bearerTokenParams)

if (this.isIdcConnection()) {
await this.regionProfileManager.restoreProfileSelection()
}
const bearerTokenParams = (await this.session.getToken()).updateCredentialsParams
await this.lspAuth.updateBearerToken(bearerTokenParams)
}

// regardless of state, send message at startup if user needs to select a Developer Profile
void this.tryNotifySelectDeveloperProfile()

Comment on lines +299 to +301
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This refreshState function triggers not only upon startup, but on every auth state change. Do we need to check here whether it's the first use or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function, see below, only triggers once on startup, and any subsequent times it will not because of the use of once

vsCodeState.isFreeTierLimitReached = false
await this.setVscodeContextProps(state)
await Promise.all([
Expand All @@ -283,6 +311,12 @@ export class AuthUtil implements IAuthProvider {
}
}

private tryNotifySelectDeveloperProfile = once(async () => {
if (this.regionProfileManager.requireProfileSelection() && this.didStartSignedIn) {
await notifySelectDeveloperProfile()
}
})

async getTelemetryMetadata(): Promise<TelemetryMetadata> {
if (!this.isConnected()) {
return {
Expand Down
Loading