Skip to content

Commit e5fa306

Browse files
committed
fix(lsp): Ambiguous or misleading log messages.
Problem: Ambiguous or misleading log messages. Solution: Refine the logging logic.
1 parent a264377 commit e5fa306

File tree

3 files changed

+46
-53
lines changed

3 files changed

+46
-53
lines changed

packages/core/src/amazonq/lsp/lspClient.ts

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import * as jose from 'jose'
1515

1616
import { Disposable, ExtensionContext } from 'vscode'
1717

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

249249
const ok2 =
250250
!lspProc.stopped &&
@@ -264,7 +264,9 @@ async function validateNodeExe(nodePath: string, lsp: string, args: string[]) {
264264
truthy: true,
265265
})
266266
if (!ok2 || selfExit) {
267-
throw new ToolkitError(`amazonqLsp: failed to run (exitcode=${lspProc.exitCode()}): ${lspProc}`)
267+
throw new ToolkitError(
268+
`amazonqLsp: failed to run (exitcode=${lspProc.exitCode()}): ${lspProc.toString(false, true)}`
269+
)
268270
}
269271
} finally {
270272
lspProc.stop(true)
@@ -300,18 +302,11 @@ export async function activate(extensionContext: ExtensionContext, resourcePaths
300302

301303
const serverModule = resourcePaths.lsp
302304

303-
// If the extension is launch in debug mode the debug server options are use
304-
// Otherwise the run options are used
305-
let serverOptions: ServerOptions = {
306-
run: { module: serverModule, transport: TransportKind.ipc },
307-
debug: { module: serverModule, transport: TransportKind.ipc, options: debugOptions },
308-
}
309-
310-
// TODO(jmkeyes): this overwrites the above...?
311-
serverOptions = createServerOptions({
305+
const serverOptions = createServerOptions({
312306
encryptionKey: key,
313307
executable: resourcePaths.node,
314308
serverModule,
309+
// TODO(jmkeyes): we always use the debug options...?
315310
execArgv: debugOptions.execArgv,
316311
})
317312

packages/core/src/shared/lsp/lspResolver.ts

Lines changed: 35 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -40,44 +40,41 @@ export class LanguageServerResolver {
4040
languageServerVersion: result.version,
4141
}
4242
}
43-
try {
44-
const latestVersion = this.latestCompatibleLspVersion()
45-
const targetContents = this.getLSPTargetContents(latestVersion)
46-
const cacheDirectory = this.getDownloadDirectory(latestVersion.serverVersion)
47-
48-
const serverResolvers: StageResolver<LspResult>[] = [
49-
{
50-
// 1: Use the current local ("cached") LSP server bundle, if any.
51-
resolve: async () => await this.getLocalServer(cacheDirectory, latestVersion, targetContents),
52-
telemetryMetadata: { id: this.lsName, languageServerLocation: 'cache' },
53-
},
54-
{
55-
// 2: Download the latest LSP server bundle.
56-
resolve: async () => await this.fetchRemoteServer(cacheDirectory, latestVersion, targetContents),
57-
telemetryMetadata: { id: this.lsName, languageServerLocation: 'remote' },
58-
},
59-
{
60-
// 3: If the download fails, try an older, cached version.
61-
resolve: async () => await this.getFallbackServer(latestVersion),
62-
telemetryMetadata: { id: this.lsName, languageServerLocation: 'fallback' },
63-
},
64-
]
65-
66-
/**
67-
* Example:
68-
* ```
69-
* LspResult {
70-
* assetDirectory = "<cachedir>/aws/toolkits/language-servers/AmazonQ/3.3.0"
71-
* location = 'cache'
72-
* version = '3.3.0'
73-
* }
74-
* ```
75-
*/
76-
const resolved = await tryStageResolvers('getServer', serverResolvers, getServerVersion)
77-
return resolved
78-
} finally {
79-
logger.info(`Finished setting up LSP server`)
80-
}
43+
const latestVersion = this.latestCompatibleLspVersion()
44+
const targetContents = this.getLSPTargetContents(latestVersion)
45+
const cacheDirectory = this.getDownloadDirectory(latestVersion.serverVersion)
46+
47+
const serverResolvers: StageResolver<LspResult>[] = [
48+
{
49+
// 1: Use the current local ("cached") LSP server bundle, if any.
50+
resolve: async () => await this.getLocalServer(cacheDirectory, latestVersion, targetContents),
51+
telemetryMetadata: { id: this.lsName, languageServerLocation: 'cache' },
52+
},
53+
{
54+
// 2: Download the latest LSP server bundle.
55+
resolve: async () => await this.fetchRemoteServer(cacheDirectory, latestVersion, targetContents),
56+
telemetryMetadata: { id: this.lsName, languageServerLocation: 'remote' },
57+
},
58+
{
59+
// 3: If the download fails, try an older, cached version.
60+
resolve: async () => await this.getFallbackServer(latestVersion),
61+
telemetryMetadata: { id: this.lsName, languageServerLocation: 'fallback' },
62+
},
63+
]
64+
65+
/**
66+
* Example:
67+
* ```
68+
* LspResult {
69+
* assetDirectory = "<cachedir>/aws/toolkits/language-servers/AmazonQ/3.3.0"
70+
* location = 'cache'
71+
* version = '3.3.0'
72+
* }
73+
* ```
74+
*/
75+
const resolved = await tryStageResolvers('getServer', serverResolvers, getServerVersion)
76+
logger.info('Finished updating "%s" LSP server: %O', this.lsName, resolved.assetDirectory)
77+
return resolved
8178
}
8279

8380
/** Finds an older, cached version of the LSP server bundle. */

packages/core/src/shared/utilities/processUtils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -513,9 +513,10 @@ export class ChildProcess {
513513
* Gets a string representation of the process invocation.
514514
*
515515
* @param noparams Omit parameters in the result (to protect sensitive info).
516+
* @param nostatus Omit "(not started)" note.
516517
*/
517-
public toString(noparams = false): string {
518-
const pid = this.pid() > 0 ? `PID ${this.pid()}:` : '(not started)'
518+
public toString(noparams = false, nostatus = false): string {
519+
const pid = this.pid() > 0 ? `PID ${this.pid()}:` : nostatus ? '' : '(not started)'
519520
return `${pid} [${this.#command} ${noparams ? '...' : this.#args.join(' ')}]`
520521
}
521522
}

0 commit comments

Comments
 (0)