Skip to content

Commit f4eac13

Browse files
authored
fix(codecatalyst): noisy getUserDetails() API calls #3333
Problem: We try to call getDevEnvironment (and by proxy getUserDetails) even though we can check if it will fail ahead of time. This happens when rendering the developer tools view. Solution: * Check environment variables before making the call. * Move error handling up so the meaning of undefined is not conflated with errors. * Lazily generate client for dev env reconnect.
1 parent 2d7e310 commit f4eac13

File tree

6 files changed

+60
-28
lines changed

6 files changed

+60
-28
lines changed

src/codecatalyst/activation.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import { CodeCatalystAuthenticationProvider } from './auth'
1414
import { registerDevfileWatcher } from './devfile'
1515
import { DevEnvClient } from '../shared/clients/devenvClient'
1616
import { watchRestartingDevEnvs } from './reconnect'
17-
import { getCodeCatalystDevEnvId } from '../shared/vscode/env'
1817
import { PromptSettings } from '../shared/settings'
1918
import { dontShow } from '../shared/localizedText'
2019
import { isCloud9 } from '../shared/extensionUtilities'
@@ -62,9 +61,12 @@ export async function activate(ctx: ExtContext): Promise<void> {
6261
ctx.extensionContext.subscriptions.push(registerDevfileWatcher(DevEnvClient.instance))
6362
}
6463

65-
const settings = PromptSettings.instance
66-
if (getCodeCatalystDevEnvId()) {
67-
const thisDevenv = await getThisDevEnv(authProvider)
64+
const thisDevenv = (await getThisDevEnv(authProvider))?.unwrapOrElse(err => {
65+
getLogger().warn('codecatalyst: failed to get current Dev Enviroment: %s', err)
66+
return undefined
67+
})
68+
69+
if (thisDevenv) {
6870
getLogger().info('codecatalyst: Dev Environment ides=%O', thisDevenv?.summary.ides)
6971
if (thisDevenv && !isDevenvVscode(thisDevenv.summary.ides)) {
7072
// Prevent Toolkit from reconnecting to a "non-vscode" devenv by actively closing it.
@@ -73,6 +75,7 @@ export async function activate(ctx: ExtContext): Promise<void> {
7375
return
7476
}
7577

78+
const settings = PromptSettings.instance
7679
if (await settings.isPromptEnabled('remoteConnected')) {
7780
const message = localize(
7881
'AWS.codecatalyst.connectedMessage',

src/codecatalyst/commands.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -184,11 +184,11 @@ interface CodeCatalystCommand<T extends any[], U> {
184184
}
185185

186186
interface ClientInjector {
187-
<T extends any[], U>(command: CodeCatalystCommand<T, U>, ...args: T): Promise<U | undefined>
187+
<T extends any[], U>(command: CodeCatalystCommand<T, U>, ...args: T): Promise<U>
188188
}
189189

190190
interface CommandDecorator {
191-
<T extends any[], U>(command: CodeCatalystCommand<T, U>): (...args: T) => Promise<U | undefined>
191+
<T extends any[], U>(command: CodeCatalystCommand<T, U>): (...args: T) => Promise<U>
192192
}
193193

194194
type Inject<T, U> = T extends (...args: infer P) => infer R
@@ -281,10 +281,6 @@ export class CodeCatalystCommands {
281281
public async openDevEnvSettings(): Promise<void> {
282282
const devenv = await this.withClient(getConnectedDevEnv)
283283

284-
if (!devenv) {
285-
throw new Error('No devenv available')
286-
}
287-
288284
return this.withClient(showConfigureDevEnv, globals.context, devenv, CodeCatalystCommands.declared)
289285
}
290286

src/codecatalyst/explorer.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { CodeCatalystAuthenticationProvider } from './auth'
1515
import { CodeCatalystCommands } from './commands'
1616
import { ConnectedDevEnv, getDevfileLocation, getThisDevEnv } from './model'
1717
import * as codecatalyst from './model'
18+
import { getLogger } from '../shared/logger'
1819

1920
const getStartedCommand = Commands.register(
2021
'aws.codecatalyst.getStarted',
@@ -125,7 +126,10 @@ export class CodeCatalystRootNode implements RootNode {
125126
}
126127

127128
public async getTreeItem() {
128-
this.devenv = await getThisDevEnv(this.authProvider)
129+
this.devenv = (await getThisDevEnv(this.authProvider))?.unwrapOrElse(err => {
130+
getLogger().warn('codecatalyst: failed to get current Dev Enviroment: %s', err)
131+
return undefined
132+
})
129133

130134
const item = new vscode.TreeItem('CodeCatalyst', vscode.TreeItemCollapsibleState.Collapsed)
131135
item.contextValue = this.authProvider.isUsingSavedConnection

src/codecatalyst/model.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
import { DevEnvClient } from '../shared/clients/devenvClient'
1818
import { getLogger } from '../shared/logger'
1919
import { AsyncCollection, toCollection } from '../shared/utilities/asyncCollection'
20-
import { getCodeCatalystSpaceName, getCodeCatalystProjectName } from '../shared/vscode/env'
20+
import { getCodeCatalystSpaceName, getCodeCatalystProjectName, getCodeCatalystDevEnvId } from '../shared/vscode/env'
2121
import { writeFile } from 'fs-extra'
2222
import { sshAgentSocketVariable, startSshAgent, startVscodeRemote } from '../shared/extensions/ssh'
2323
import { ChildProcess } from '../shared/utilities/childProcess'
@@ -28,7 +28,8 @@ import { Commands } from '../shared/vscode/commands2'
2828
import { areEqual } from '../shared/utilities/pathUtils'
2929
import { fileExists } from '../shared/filesystemUtilities'
3030
import { CodeCatalystAuthenticationProvider } from './auth'
31-
import { UnknownError } from '../shared/errors'
31+
import { ToolkitError } from '../shared/errors'
32+
import { Result } from '../shared/utilities/result'
3233

3334
export type DevEnvironmentId = Pick<DevEnvironment, 'id' | 'org' | 'project'>
3435

@@ -154,10 +155,10 @@ export interface ConnectedDevEnv {
154155
export async function getConnectedDevEnv(
155156
codeCatalystClient: CodeCatalystClient,
156157
devenvClient = DevEnvClient.instance
157-
): Promise<ConnectedDevEnv | undefined> {
158+
): Promise<ConnectedDevEnv> {
158159
const devEnvId = devenvClient.id
159-
if (!devEnvId || !devenvClient.isCodeCatalystDevEnv()) {
160-
return
160+
if (!devEnvId) {
161+
throw new ToolkitError('Not connected to a dev environment', { code: 'NotConnectedToDevEnv' })
161162
}
162163

163164
const projectName = getCodeCatalystProjectName()
@@ -180,17 +181,20 @@ export async function getConnectedDevEnv(
180181
* Gets the current devenv that Toolkit is running in, if any.
181182
*/
182183
export async function getThisDevEnv(authProvider: CodeCatalystAuthenticationProvider) {
184+
if (!getCodeCatalystDevEnvId()) {
185+
return
186+
}
187+
183188
try {
184189
await authProvider.restore()
185190
const conn = authProvider.activeConnection
186191
if (conn !== undefined && authProvider.auth.getConnectionState(conn) === 'valid') {
187192
const client = await createClient(conn)
188-
return await getConnectedDevEnv(client)
193+
return Result.ok(await getConnectedDevEnv(client))
189194
}
190195
} catch (err) {
191-
getLogger().warn(`codecatalyst: failed to get Dev Environment: ${UnknownError.cast(err).message}`)
196+
return Result.err(err)
192197
}
193-
return undefined
194198
}
195199

196200
/**

src/codecatalyst/reconnect.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { CodeCatalystAuthenticationProvider } from './auth'
1616
import { getCodeCatalystDevEnvId } from '../shared/vscode/env'
1717
import globals from '../shared/extensionGlobals'
1818
import { isDevenvVscode, recordSource } from './utils'
19+
import { SsoConnection } from '../credentials/auth'
1920

2021
const localize = nls.loadMessageBundle()
2122

@@ -30,14 +31,13 @@ export function watchRestartingDevEnvs(ctx: ExtContext, authProvider: CodeCataly
3031
}
3132
getLogger().info(`codecatalyst: reconnect: onDidChangeActiveConnection: startUrl=${conn.startUrl}`)
3233

33-
const client = await createClient(conn)
3434
const envId = getCodeCatalystDevEnvId()
35-
handleRestart(client, ctx, envId)
35+
handleRestart(conn, ctx, envId)
3636
restartHandled = true
3737
})
3838
}
3939

40-
function handleRestart(client: CodeCatalystClient, ctx: ExtContext, envId: string | undefined) {
40+
function handleRestart(conn: SsoConnection, ctx: ExtContext, envId: string | undefined) {
4141
if (envId !== undefined) {
4242
const memento = ctx.extensionContext.globalState
4343
const pendingReconnects = memento.get<Record<string, DevEnvMemento>>(codecatalystReconnectKey, {})
@@ -55,18 +55,18 @@ function handleRestart(client: CodeCatalystClient, ctx: ExtContext, envId: strin
5555
getLogger().info('codecatalyst: attempting to poll dev environments')
5656

5757
// Reconnect devenvs (if coming from a restart)
58-
reconnectDevEnvs(client, ctx).catch(err => {
58+
reconnectDevEnvs(conn, ctx).catch(err => {
5959
getLogger().error(`codecatalyst: error while resuming devenvs: ${err}`)
6060
})
6161
}
6262
}
6363

6464
/**
6565
* Attempt to poll for connection in all valid devenvs
66-
* @param client a connected client
66+
* @param conn a connection that may be used for CodeCatalyst
6767
* @param ctx the extension context
6868
*/
69-
async function reconnectDevEnvs(client: CodeCatalystClient, ctx: ExtContext): Promise<void> {
69+
async function reconnectDevEnvs(conn: SsoConnection, ctx: ExtContext): Promise<void> {
7070
const memento = ctx.extensionContext.globalState
7171
const pendingDevEnvs = memento.get<Record<string, DevEnvMemento>>(codecatalystReconnectKey, {})
7272
const validDevEnvs = filterInvalidDevEnvs(pendingDevEnvs)
@@ -85,12 +85,14 @@ async function reconnectDevEnvs(client: CodeCatalystClient, ctx: ExtContext): Pr
8585
'Dev Environments restarting: {0}',
8686
polledDevEnvs
8787
)
88-
vscode.window.withProgress(
88+
await vscode.window.withProgress(
8989
{
9090
location: vscode.ProgressLocation.Notification,
9191
},
92-
(progress, token) => {
92+
async (progress, token) => {
9393
progress.report({ message: progressTitle })
94+
const client = await createClient(conn)
95+
9496
return pollDevEnvs(client, progress, token, memento, validDevEnvs)
9597
}
9698
)

src/testE2E/codecatalyst/client.test.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
createClient as createCodeCatalystClient,
1313
DevEnvironment,
1414
} from '../../shared/clients/codecatalystClient'
15-
import { prepareDevEnvConnection } from '../../codecatalyst/model'
15+
import { getThisDevEnv, prepareDevEnvConnection } from '../../codecatalyst/model'
1616
import { Auth, createBuilderIdProfile, SsoConnection } from '../../credentials/auth'
1717
import { CodeCatalystAuthenticationProvider, isValidCodeCatalystConnection } from '../../codecatalyst/auth'
1818
import { CodeCatalystCommands, DevEnvironmentSettings } from '../../codecatalyst/commands'
@@ -131,6 +131,29 @@ describe('Test how this codebase uses the CodeCatalyst API', function () {
131131
})
132132
})
133133

134+
describe('getThisDevEnv', function () {
135+
const ccAuth = CodeCatalystAuthenticationProvider.fromContext(globals.context)
136+
137+
it('returns `undefined` if not in a dev environment', async function () {
138+
const result = await getThisDevEnv(ccAuth)
139+
assert.strictEqual(result, undefined)
140+
})
141+
142+
it('returns an error if in a dev environment but the API calls fail', async function () {
143+
const oldEnv = { ...process.env }
144+
process.env['__DEV_ENVIRONMENT_ID'] = defaultDevEnv.id
145+
process.env['__DEV_ENVIRONMENT_SPACE_NAME'] = defaultDevEnv.org.name
146+
process.env['__DEV_ENVIRONMENT_PROJECT_NAME'] = 'wrong-project-name'
147+
148+
try {
149+
const result = await getThisDevEnv(ccAuth)
150+
assert.ok(result?.isErr(), 'Expected an error to be returned')
151+
} finally {
152+
process.env = oldEnv
153+
}
154+
})
155+
})
156+
134157
it('creates an empty Dev Environment', async function () {
135158
const emptyDevEnvSettings = buildDevEnvSettings()
136159
const emptyDevEnv = await createDevEnvFromWebview(emptyDevEnvSettings, {

0 commit comments

Comments
 (0)