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
4 changes: 2 additions & 2 deletions packages/amazonq/src/lsp/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import vscode from 'vscode'
import { startLanguageServer } from './client'
import { AmazonQLspInstaller } from './lspInstaller'
import { lspSetupStage, ToolkitError } from 'aws-core-vscode/shared'
import { lspSetupStage, ToolkitError, messages } from 'aws-core-vscode/shared'

export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
try {
Expand All @@ -16,6 +16,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
})
} catch (err) {
const e = err as ToolkitError
void vscode.window.showInformationMessage(`Unable to launch amazonq language server: ${e.message}`)
void messages.showViewLogsMessage(`Failed to launch Amazon Q language server: ${e.message}`)
}
}
21 changes: 8 additions & 13 deletions packages/core/src/amazonq/lsp/lspClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import * as jose from 'jose'

import { Disposable, ExtensionContext } from 'vscode'

import { LanguageClient, LanguageClientOptions, ServerOptions, TransportKind } from 'vscode-languageclient'
import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient'
import {
BuildIndexRequestPayload,
BuildIndexRequestType,
Expand Down Expand Up @@ -235,7 +235,7 @@ async function validateNodeExe(nodePath: string, lsp: string, args: string[]) {
const r = await proc.run()
const ok = r.exitCode === 0 && r.stdout.includes('ok')
if (!ok) {
const msg = `failed to run basic "node -e" test (exitcode=${r.exitCode}): ${proc}`
const msg = `failed to run basic "node -e" test (exitcode=${r.exitCode}): ${proc.toString(false, true)}`
logger.error(msg)
throw new ToolkitError(`amazonqLsp: ${msg}`)
}
Expand All @@ -244,7 +244,7 @@ async function validateNodeExe(nodePath: string, lsp: string, args: string[]) {
const lspProc = new ChildProcess(nodePath, [lsp, ...args], { logging: 'no' })
try {
// Start asynchronously (it never stops; we need to stop it below).
lspProc.run().catch((e) => logger.error('failed to run: %s', lspProc))
lspProc.run().catch((e) => logger.error('failed to run: %s', lspProc.toString(false, true)))

const ok2 =
!lspProc.stopped &&
Expand All @@ -264,7 +264,9 @@ async function validateNodeExe(nodePath: string, lsp: string, args: string[]) {
truthy: true,
})
if (!ok2 || selfExit) {
throw new ToolkitError(`amazonqLsp: failed to run (exitcode=${lspProc.exitCode()}): ${lspProc}`)
throw new ToolkitError(
`amazonqLsp: failed to run (exitcode=${lspProc.exitCode()}): ${lspProc.toString(false, true)}`
)
}
} finally {
lspProc.stop(true)
Expand Down Expand Up @@ -300,18 +302,11 @@ export async function activate(extensionContext: ExtensionContext, resourcePaths

const serverModule = resourcePaths.lsp

// If the extension is launch in debug mode the debug server options are use
// Otherwise the run options are used
let serverOptions: ServerOptions = {
run: { module: serverModule, transport: TransportKind.ipc },
debug: { module: serverModule, transport: TransportKind.ipc, options: debugOptions },
}

// TODO(jmkeyes): this overwrites the above...?
serverOptions = createServerOptions({
const serverOptions = createServerOptions({
encryptionKey: key,
executable: resourcePaths.node,
serverModule,
// TODO(jmkeyes): we always use the debug options...?
execArgv: debugOptions.execArgv,
})

Expand Down
24 changes: 12 additions & 12 deletions packages/core/src/amazonq/lsp/lspController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,11 @@ export class LspController {
}

async buildIndex(buildIndexConfig: BuildIndexConfig) {
this.logger.info(`LspController: Starting to build index of project`)
this.logger.info(`Starting to build index of project`)
const start = performance.now()
const projPaths = (vscode.workspace.workspaceFolders ?? []).map((folder) => folder.uri.fsPath)
if (projPaths.length === 0) {
this.logger.info(`LspController: Skipping building index. No projects found in workspace`)
this.logger.info(`Skipping building index. No projects found in workspace`)
return
}
projPaths.sort()
Expand All @@ -119,12 +119,12 @@ export class LspController {
(accumulator, currentFile) => accumulator + currentFile.fileSizeBytes,
0
)
this.logger.info(`LspController: Found ${files.length} files in current project ${projPaths}`)
this.logger.info(`Found ${files.length} files in current project ${projPaths}`)
const config = buildIndexConfig.isVectorIndexEnabled ? 'all' : 'default'
const r = files.map((f) => f.fileUri.fsPath)
const resp = await LspClient.instance.buildIndex(r, projRoot, config)
if (resp) {
this.logger.debug(`LspController: Finish building index of project`)
this.logger.debug(`Finish building index of project`)
const usage = await LspClient.instance.getLspServerUsage()
telemetry.amazonq_indexWorkspace.emit({
duration: performance.now() - start,
Expand All @@ -137,7 +137,7 @@ export class LspController {
credentialStartUrl: buildIndexConfig.startUrl,
})
} else {
this.logger.error(`LspController: Failed to build index of project`)
this.logger.error(`Failed to build index of project`)
telemetry.amazonq_indexWorkspace.emit({
duration: performance.now() - start,
result: 'Failed',
Expand All @@ -149,7 +149,7 @@ export class LspController {
}
} catch (error) {
// TODO: use telemetry.run()
this.logger.error(`LspController: Failed to build index of project`)
this.logger.error(`Failed to build index of project`)
telemetry.amazonq_indexWorkspace.emit({
duration: performance.now() - start,
result: 'Failed',
Expand All @@ -166,7 +166,7 @@ export class LspController {

async trySetupLsp(context: vscode.ExtensionContext, buildIndexConfig: BuildIndexConfig) {
if (isCloud9() || isWeb() || isAmazonInternalOs()) {
this.logger.warn('LspController: Skipping LSP setup. LSP is not compatible with the current environment. ')
this.logger.warn('Skipping LSP setup. LSP is not compatible with the current environment. ')
// do not do anything if in Cloud9 or Web mode or in AL2 (AL2 does not support node v18+)
return
}
Expand All @@ -181,7 +181,7 @@ export class LspController {
const usage = await LspClient.instance.getLspServerUsage()
if (usage) {
this.logger.info(
`LspController: LSP server CPU ${usage.cpuUsage}%, LSP server Memory ${
`LSP server CPU ${usage.cpuUsage}%, LSP server Memory ${
usage.memoryUsage / (1024 * 1024)
}MB `
)
Expand All @@ -190,7 +190,7 @@ export class LspController {
30 * 60 * 1000
)
} catch (e) {
this.logger.error(`LspController: LSP failed to activate ${e}`)
this.logger.error(`LSP failed to activate ${e}`)
}
})
}
Expand All @@ -207,7 +207,7 @@ export class LspController {
return
}
this._contextCommandSymbolsUpdated = true
getLogger().debug(`LspController: Start adding symbols to context picker menu`)
getLogger().debug(`Start adding symbols to context picker menu`)
try {
const indexSeqNum = await LspClient.instance.getIndexSequenceNumber()
await LspClient.instance.updateIndex([], 'context_command_symbol_update')
Expand All @@ -223,15 +223,15 @@ export class LspController {
{ interval: 1000, timeout: 60_000, truthy: true }
)
} catch (err) {
getLogger().error(`LspController: Failed to find symbols`)
this.logger.error(`Failed to find symbols`)
}
}

private async setupLsp(context: vscode.ExtensionContext) {
await lspSetupStage('all', async () => {
const installResult = await new WorkspaceLspInstaller().resolve()
await lspSetupStage('launch', async () => activateLsp(context, installResult.resourcePaths))
this.logger.info('LspController: LSP activated')
this.logger.info('LSP activated')
})
}
}
4 changes: 3 additions & 1 deletion packages/core/src/shared/lsp/baseLspInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ export abstract class BaseLspInstaller<T extends ResourcePaths = ResourcePaths,
await this.postInstall(assetDirectory)

const deletedVersions = await cleanLspDownloads(manifest.versions, nodePath.dirname(assetDirectory))
this.logger.debug(`cleaning old LSP versions deleted ${deletedVersions.length} versions`)
if (deletedVersions.length > 0) {
this.logger.debug(`cleaning old LSP versions: deleted ${deletedVersions.length} versions`)
}

const r = {
...installationResult,
Expand Down
79 changes: 36 additions & 43 deletions packages/core/src/shared/lsp/lspResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,44 +40,41 @@ export class LanguageServerResolver {
languageServerVersion: result.version,
}
}
try {
const latestVersion = this.latestCompatibleLspVersion()
const targetContents = this.getLSPTargetContents(latestVersion)
const cacheDirectory = this.getDownloadDirectory(latestVersion.serverVersion)

const serverResolvers: StageResolver<LspResult>[] = [
{
// 1: Use the current local ("cached") LSP server bundle, if any.
resolve: async () => await this.getLocalServer(cacheDirectory, latestVersion, targetContents),
telemetryMetadata: { id: this.lsName, languageServerLocation: 'cache' },
},
{
// 2: Download the latest LSP server bundle.
resolve: async () => await this.fetchRemoteServer(cacheDirectory, latestVersion, targetContents),
telemetryMetadata: { id: this.lsName, languageServerLocation: 'remote' },
},
{
// 3: If the download fails, try an older, cached version.
resolve: async () => await this.getFallbackServer(latestVersion),
telemetryMetadata: { id: this.lsName, languageServerLocation: 'fallback' },
},
]

/**
* Example:
* ```
* LspResult {
* assetDirectory = "<cachedir>/aws/toolkits/language-servers/AmazonQ/3.3.0"
* location = 'cache'
* version = '3.3.0'
* }
* ```
*/
const resolved = await tryStageResolvers('getServer', serverResolvers, getServerVersion)
return resolved
} finally {
logger.info(`Finished setting up LSP server`)
Copy link
Contributor Author

@justinmk3 justinmk3 Apr 16, 2025

Choose a reason for hiding this comment

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

Removed this finally. If an exception was thrown, it didn't really "finish" setup?

}
const latestVersion = this.latestCompatibleLspVersion()
const targetContents = this.getLSPTargetContents(latestVersion)
const cacheDirectory = this.getDownloadDirectory(latestVersion.serverVersion)

const serverResolvers: StageResolver<LspResult>[] = [
{
// 1: Use the current local ("cached") LSP server bundle, if any.
resolve: async () => await this.getLocalServer(cacheDirectory, latestVersion, targetContents),
telemetryMetadata: { id: this.lsName, languageServerLocation: 'cache' },
},
{
// 2: Download the latest LSP server bundle.
resolve: async () => await this.fetchRemoteServer(cacheDirectory, latestVersion, targetContents),
telemetryMetadata: { id: this.lsName, languageServerLocation: 'remote' },
},
{
// 3: If the download fails, try an older, cached version.
resolve: async () => await this.getFallbackServer(latestVersion),
telemetryMetadata: { id: this.lsName, languageServerLocation: 'fallback' },
},
]

/**
* Example:
* ```
* LspResult {
* assetDirectory = "<cachedir>/aws/toolkits/language-servers/AmazonQ/3.3.0"
* location = 'cache'
* version = '3.3.0'
* }
* ```
*/
const resolved = await tryStageResolvers('getServer', serverResolvers, getServerVersion)
logger.info('Finished preparing "%s" LSP server: %O', this.lsName, resolved.assetDirectory)
return resolved
}

/** Finds an older, cached version of the LSP server bundle. */
Expand Down Expand Up @@ -147,11 +144,7 @@ export class LanguageServerResolver {
}
} else {
// Delete the cached directory since it's invalid
if (await fs.existsDir(cacheDirectory)) {
await fs.delete(cacheDirectory, {
recursive: true,
})
}
await fs.delete(cacheDirectory, { force: true, recursive: true })
throw new ToolkitError('Failed to retrieve server from cache', { code: 'InvalidCache' })
}
}
Expand Down
26 changes: 20 additions & 6 deletions packages/core/src/shared/lsp/manifestResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,25 +64,38 @@ export class ManifestResolver {
}).getNewETagContent(this.getEtag())

if (!resp.content) {
throw new ToolkitError(
`New content was not downloaded; fallback to the locally stored "${this.lsName}" manifest`
)
const msg = `"${this.lsName}" manifest unchanged, skipping download: ${this.manifestURL}`
Comment on lines 66 to +67
Copy link
Contributor Author

@justinmk3 justinmk3 Apr 16, 2025

Choose a reason for hiding this comment

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

!resp.content means "etag didnt change". It doesn't mean "failed to download".

The old code would always report a failed languageServer_setup in the common case (where etag didn't change).

logger.debug(msg)

const localManifest = await this.getLocalManifest(true).catch(() => undefined)
if (localManifest) {
localManifest.location = 'remote'
Comment on lines +70 to +72
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 common case is that etag didnt change. So that's handled here by re-using getLocalManifest. ManifestResolver doesn't have a way to tell its caller "I didnt fail but please continue to the next step".

return localManifest
} else {
// Will emit a `languageServer_setup` result=failed metric...
throw new ToolkitError(msg)
}
}

logger.debug(`fetched "${this.lsName}" manifest: ${this.manifestURL}`)
const manifest = this.parseManifest(resp.content)
await this.saveManifest(resp.eTag, resp.content)
await this.checkDeprecation(manifest)
manifest.location = 'remote'
return manifest
}

private async getLocalManifest(): Promise<Manifest> {
logger.info(`Failed to download latest "${this.lsName}" manifest. Falling back to local manifest.`)
private async getLocalManifest(silent: boolean = false): Promise<Manifest> {
Comment on lines -80 to +88
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Always reporting "failed to download" here is misleading because this will happen when "etag didnt change".

if (!silent) {
logger.info(`trying local "${this.lsName}" manifest...`)
}
const storage = this.getStorage()
const manifestData = storage[this.lsName]

if (!manifestData?.content) {
throw new ToolkitError(`Failed to download "${this.lsName}" manifest and no local manifest found.`)
const msg = `local "${this.lsName}" manifest not found`
logger.warn(msg)
throw new ToolkitError(msg)
}

const manifest = this.parseManifest(manifestData.content)
Expand Down Expand Up @@ -145,6 +158,7 @@ export class ManifestResolver {
...storage,
[this.lsName]: {
etag,
// XXX: this stores the entire manifest. vscode warns about our globalStorage size...
content,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are storing the entire manifest in globalStorage :/

},
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ export class HttpResourceFetcher implements ResourceFetcher<Response> {
if (response.status === 304) {
// Explanation: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match
contents = undefined
this.logger.verbose(`E-Tag, ${eTagResponse}, matched. No content downloaded from: ${this.url}`)
this.logger.verbose(`E-Tag matched (${eTagResponse}). Download skipped: ${this.url}`)
} else {
this.logger.verbose(`No E-Tag match. Downloaded content from: ${this.logText()}`)
this.logger.verbose(`E-Tag not matched. Downloaded: ${this.logText()}`)
}

return { content: contents, eTag: eTagResponse }
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/shared/utilities/processUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,10 @@ export class ChildProcess {
* Gets a string representation of the process invocation.
*
* @param noparams Omit parameters in the result (to protect sensitive info).
* @param nostatus Omit "(not started)" note.
*/
public toString(noparams = false): string {
const pid = this.pid() > 0 ? `PID ${this.pid()}:` : '(not started)'
public toString(noparams = false, nostatus = false): string {
const pid = this.pid() > 0 ? `PID ${this.pid()}:` : nostatus ? '' : '(not started)'
return `${pid} [${this.#command} ${noparams ? '...' : this.#args.join(' ')}]`
}
}
Loading