Skip to content

Commit e303e1a

Browse files
satyakighsatyakigh
committed
Misc fixes
1 parent 707f8be commit e303e1a

13 files changed

Lines changed: 746 additions & 299 deletions

File tree

‎.review-pr-8856.md‎

Lines changed: 0 additions & 195 deletions
This file was deleted.

‎packages/core/src/awsService/cloudformation/extension.ts‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,7 @@
44
*/
55

66
import { ExtensionContext, window, languages, commands, Disposable } from 'vscode'
7-
import {
8-
LanguageClient,
9-
LanguageClientOptions,
10-
ServerOptions,
11-
TransportKind,
12-
ErrorHandlerResult,
13-
CloseHandlerResult,
14-
} from 'vscode-languageclient/node'
15-
import { CloseAction, ErrorAction, Message } from 'vscode-languageclient/node'
7+
import { LanguageClient, LanguageClientOptions, ServerOptions, TransportKind } from 'vscode-languageclient/node'
168
import { formatMessage, toString, startupFailureMessage, clientIdForInitialization } from './utils'
179
import globals from '../../shared/extensionGlobals'
1810
import { extensionVersion, getServiceEnvVarConfig } from '../../shared/vscode/env'
@@ -95,7 +87,7 @@ function createClientFactory(
9587
clientId: string,
9688
cfnLspConfig: Record<string, string | undefined>
9789
): LanguageClientFactory {
98-
return async (serverPath: string, _serverRootDir: string): Promise<LanguageClient> => {
90+
return async ({ serverPath, errorHandler }): Promise<LanguageClient> => {
9991
if (!(await fs.existsFile(serverPath))) {
10092
throw new Error(`CloudFormation LSP ${serverPath} not found`)
10193
}
@@ -158,15 +150,8 @@ function createClientFactory(
158150
},
159151
},
160152
},
161-
errorHandler: {
162-
error: (error: Error, message: Message | undefined, _count: number | undefined): ErrorHandlerResult => {
163-
void window.showErrorMessage(formatMessage(`${toString(message)} - ${toString(error)}`))
164-
return { action: ErrorAction.Continue }
165-
},
166-
closed: (): CloseHandlerResult => {
167-
return { action: CloseAction.DoNotRestart }
168-
},
169-
},
153+
// Close/error policy is shared across toolkit language servers (LspServerLifecycleController).
154+
errorHandler,
170155
}
171156

172157
return new LanguageClient(ExtensionId, ExtensionName, serverOptions, clientOptions)
@@ -197,6 +182,9 @@ async function startClient(context: ExtensionContext): Promise<void> {
197182
resolver: serverProvider,
198183
invalidator: serverProvider,
199184
clientFactory,
185+
onError: (error, message) => {
186+
void window.showErrorMessage(formatMessage(`${toString(message)} - ${toString(error)}`))
187+
},
200188
})
201189
launcher = sessionLauncher
202190

@@ -368,7 +356,10 @@ export async function activate(context: ExtensionContext): Promise<void> {
368356
formatMessage(`Failed to restart CloudFormation language server: ${toString(error)}`)
369357
)
370358
}
371-
})
359+
}),
360+
// The client and its UI are owned by the session (so "Restart Server" can replace them), but the
361+
// extension lifetime must still shut the server down gracefully on deactivation.
362+
{ dispose: () => void disposeClientSession() }
372363
)
373364

374365
try {
@@ -383,5 +374,9 @@ export async function activate(context: ExtensionContext): Promise<void> {
383374
}
384375

385376
export async function deactivate(): Promise<void> {
386-
await disposeClientSession()
377+
try {
378+
await disposeClientSession()
379+
} catch (err) {
380+
getLogger('awsCfnLsp').warn(`Failed to stop CloudFormation language server on deactivate: ${err}`)
381+
}
387382
}

‎packages/core/src/awsService/cloudformation/utils.ts‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,36 +27,45 @@ export function clientIdForInitialization(telemetryEnabled: boolean, clientId: s
2727
return telemetryEnabled && !isAnonymousClientId(clientId) ? clientId : undefined
2828
}
2929

30+
const installFailureMessages: Record<string, string> = {
31+
ManifestFetchFailed: 'Failed to fetch CloudFormation LSP manifest. Check your network connection.',
32+
NoCompatibleVersion: 'No compatible CloudFormation LSP version found for your platform.',
33+
RemoteDownloadFailed: 'Failed to download CloudFormation LSP. Check your network connection.',
34+
ExtractionFailed: 'Failed to extract CloudFormation LSP.',
35+
HashIntegrityFailed: 'Downloaded file integrity check failed. The file may be corrupted.',
36+
}
37+
38+
/** Emitted by `LspLauncher` when the server process could not be started even after a reinstall. */
39+
const startFailedCode = 'LspStartFailed'
40+
const startFailedMessage = 'CloudFormation language server failed to start. See the AWS Toolkit logs for details.'
41+
42+
/**
43+
* Maps a startup error to a user-facing message. Install errors (which identify a cause the user can
44+
* act on) take precedence over the generic process-start failure anywhere in the `cause` chain.
45+
*/
3046
export function startupFailureMessage(error: unknown): string | undefined {
31-
const messages: Record<string, string> = {
32-
ManifestFetchFailed: 'Failed to fetch CloudFormation LSP manifest. Check your network connection.',
33-
NoCompatibleVersion: 'No compatible CloudFormation LSP version found for your platform.',
34-
RemoteDownloadFailed: 'Failed to download CloudFormation LSP. Check your network connection.',
35-
ExtractionFailed: 'Failed to extract CloudFormation LSP.',
36-
HashIntegrityFailed: 'Downloaded file integrity check failed. The file may be corrupted.',
47+
const codes = collectErrorCodes(error)
48+
const installCode = codes.find((code) => code in installFailureMessages)
49+
if (installCode) {
50+
return formatMessage(installFailureMessages[installCode])
3751
}
38-
const code = findErrorCode(error)
39-
return code && messages[code] ? formatMessage(messages[code]) : undefined
52+
if (codes.includes(startFailedCode)) {
53+
return formatMessage(startFailedMessage)
54+
}
55+
return undefined
4056
}
4157

42-
function findErrorCode(error: unknown): string | undefined {
58+
function collectErrorCodes(error: unknown): string[] {
59+
const codes: string[] = []
4360
let current = error
4461
while (current instanceof Error) {
4562
const code = (current as Error & { code?: unknown }).code
4663
if (typeof code === 'string') {
47-
if (
48-
code === 'ManifestFetchFailed' ||
49-
code === 'NoCompatibleVersion' ||
50-
code === 'RemoteDownloadFailed' ||
51-
code === 'ExtractionFailed' ||
52-
code === 'HashIntegrityFailed'
53-
) {
54-
return code
55-
}
64+
codes.push(code)
5665
}
5766
current = (current as Error & { cause?: unknown }).cause
5867
}
59-
return undefined
68+
return codes
6069
}
6170

6271
export function commandKey(key: string): string {

‎packages/core/src/extensionNode.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import * as nls from 'vscode-nls'
88

99
import * as codecatalyst from './codecatalyst/activation'
1010
import { activate as activateAppBuilder } from './awsService/appBuilder/activation'
11-
import { activate as activateCloudFormation } from './awsService/cloudformation/extension'
11+
import {
12+
activate as activateCloudFormation,
13+
deactivate as deactivateCloudFormation,
14+
} from './awsService/cloudformation/extension'
1215
import { activate as activateAwsExplorer } from './awsexplorer/activation'
1316
import { activate as activateCloudWatchLogs } from './awsService/cloudWatchLogs/activation'
1417
import { activate as activateSchemas } from './eventSchemas/activation'
@@ -272,7 +275,12 @@ export async function activate(context: vscode.ExtensionContext) {
272275

273276
export async function deactivate() {
274277
// Run concurrently to speed up execution. stop() does not throw so it is safe
275-
await Promise.all([await (await CrashMonitoring.instance())?.shutdown(), deactivateCommon(), deactivateEc2()])
278+
await Promise.all([
279+
await (await CrashMonitoring.instance())?.shutdown(),
280+
deactivateCommon(),
281+
deactivateEc2(),
282+
deactivateCloudFormation(),
283+
])
276284
globals.sdkClientBuilderV3.clearServiceCache()
277285
await globals.resourceManager.dispose()
278286
}

‎packages/core/src/shared/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ export { default as request } from './request'
7777
export * from './lsp/utils/platform'
7878
export * from './lsp/utils/targetResolver'
7979
export * from './lsp/lspLauncher'
80+
export * from './lsp/lspServerLifecycle'
8081
export * as processUtils from './utilities/processUtils'
8182
export * as BaseLspInstaller from './lsp/baseLspInstaller'
8283
export * as collectionUtil from './utilities/collectionUtils'

‎packages/core/src/shared/lsp/lspLauncher.ts‎

Lines changed: 87 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,10 @@
44
*/
55

66
import { Disposable } from 'vscode'
7-
import { LanguageClient } from 'vscode-languageclient/node'
7+
import { ErrorHandler, LanguageClient } from 'vscode-languageclient/node'
88
import { getLogger } from '../logger/logger'
99
import { ToolkitError } from '../errors'
10+
import { LspServerLifecycleController, LspServerLifecycleHooks } from './lspServerLifecycle'
1011

1112
const logger = getLogger('lsp')
1213

@@ -19,31 +20,57 @@ export interface LspInstallationInvalidator {
1920
invalidateResolvedInstallation(): void | Promise<void>
2021
}
2122

22-
export type LanguageClientFactory = (serverPath: string, serverRootDir: string) => Promise<LanguageClient>
23+
/** What the launcher hands to the client factory; `errorHandler` must be set on `LanguageClientOptions`. */
24+
export interface LanguageClientFactoryContext {
25+
serverPath: string
26+
serverRootDir: string
27+
/** Shared close/error policy from {@link LspServerLifecycleController}; do not implement your own. */
28+
errorHandler: ErrorHandler
29+
}
30+
31+
export type LanguageClientFactory = (context: LanguageClientFactoryContext) => Promise<LanguageClient>
2332

24-
export interface LspLauncherConfig {
33+
export interface LspLauncherConfig extends LspServerLifecycleHooks {
2534
name: string
2635
resolver: LspServerResolver
2736
invalidator: LspInstallationInvalidator
2837
clientFactory: LanguageClientFactory
2938
onStarted?: (client: LanguageClient) => Promise<void>
3039
}
3140

41+
interface ResolvedServer {
42+
serverPath: string
43+
serverRootDir: string
44+
}
45+
46+
/**
47+
* Starts a managed language server: resolves (installing if needed), creates the client, and runs the
48+
* shared startup-recovery policy from {@link LspServerLifecycleController}. Owns the running client for
49+
* the session; `stop()`/`dispose()` shut it down.
50+
*/
3251
export class LspLauncher implements Disposable {
3352
private client?: LanguageClient
3453
private startPromise?: Promise<LanguageClient>
3554
private disposed = false
3655
private readonly config: LspLauncherConfig
56+
private readonly lifecycle: LspServerLifecycleController<LanguageClient, ResolvedServer>
3757

3858
constructor(config: LspLauncherConfig) {
3959
this.config = config
60+
this.lifecycle = new LspServerLifecycleController<LanguageClient, ResolvedServer>({
61+
name: config.name,
62+
resolveServer: () => this.resolveServer(),
63+
startProcess: (server) => this.startProcess(server),
64+
invalidateAndReinstall: () => config.invalidator.invalidateResolvedInstallation(),
65+
shouldRepair: config.shouldRepair,
66+
onServerStopped: config.onServerStopped,
67+
onError: config.onError,
68+
})
4069
}
4170

4271
async start(): Promise<LanguageClient> {
4372
if (this.disposed) {
44-
throw new ToolkitError(`${this.config.name}: cannot start a disposed launcher`, {
45-
code: 'LspLauncherDisposed',
46-
})
73+
throw this.disposedError('cannot start a disposed launcher')
4774
}
4875

4976
if (this.client) {
@@ -64,69 +91,70 @@ export class LspLauncher implements Disposable {
6491
}
6592
}
6693

67-
private async doStart(): Promise<LanguageClient> {
68-
for (let attempt = 1; attempt <= 2; attempt++) {
69-
const isFinalAttempt = attempt === 2
70-
71-
const serverPath = await this.config.resolver.serverExecutable()
72-
const serverRootDir = await this.config.resolver.serverRootDir()
73-
74-
logger.info(`${this.config.name}: creating client for server at ${serverPath}`)
94+
private async resolveServer(): Promise<ResolvedServer> {
95+
const serverPath = await this.config.resolver.serverExecutable()
96+
const serverRootDir = await this.config.resolver.serverRootDir()
97+
return { serverPath, serverRootDir }
98+
}
7599

76-
let candidate: LanguageClient | undefined
77-
try {
78-
candidate = await this.config.clientFactory(serverPath, serverRootDir)
79-
await candidate.start()
80-
} catch (startErr) {
81-
if (candidate) {
82-
await bestEffortStopDispose(candidate, this.config.name)
83-
}
84-
85-
if (isFinalAttempt) {
86-
throw new ToolkitError(
87-
`${this.config.name}: failed to start language server after retry: ${startErr}`,
88-
{ code: 'LspStartFailed', cause: startErr as Error }
89-
)
90-
}
91-
92-
logger.warn(`${this.config.name}: process start failed, invalidating and retrying once: ${startErr}`)
93-
await this.config.invalidator.invalidateResolvedInstallation()
94-
continue
100+
/** Creates the client and completes `initialize`; a partially created client is cleaned up on failure. */
101+
private async startProcess({ serverPath, serverRootDir }: ResolvedServer): Promise<LanguageClient> {
102+
logger.info(`${this.config.name}: creating client for server at ${serverPath}`)
103+
let candidate: LanguageClient | undefined
104+
try {
105+
candidate = await this.config.clientFactory({
106+
serverPath,
107+
serverRootDir,
108+
errorHandler: this.lifecycle.createErrorHandler(),
109+
})
110+
await candidate.start()
111+
return candidate
112+
} catch (err) {
113+
if (candidate) {
114+
await bestEffortStopDispose(candidate, this.config.name)
95115
}
116+
throw err
117+
}
118+
}
96119

120+
private async doStart(): Promise<LanguageClient> {
121+
// Resolving may download the server; don't start that work if we were disposed during the
122+
// previous attempt (e.g. a restart or deactivation raced with the invalidate-and-retry).
123+
const candidate = await this.lifecycle.launchWithRetry(() => {
97124
if (this.disposed) {
98-
await bestEffortStopDispose(candidate, this.config.name)
99-
throw new ToolkitError(`${this.config.name}: launcher disposed during start`, {
100-
code: 'LspLauncherDisposed',
101-
})
125+
throw this.disposedError('launcher disposed during start')
102126
}
127+
})
103128

104-
this.client = candidate
105-
logger.info(`${this.config.name}: language client started successfully`)
106-
107-
if (this.config.onStarted) {
108-
try {
109-
await this.config.onStarted(candidate)
110-
} catch (onStartedErr) {
111-
logger.warn(`${this.config.name}: onStarted hook failed, cleaning up client: ${onStartedErr}`)
112-
await this.cleanupClient()
113-
throw onStartedErr
114-
}
115-
}
129+
if (this.disposed) {
130+
await bestEffortStopDispose(candidate, this.config.name)
131+
throw this.disposedError('launcher disposed during start')
132+
}
116133

117-
if (this.disposed) {
134+
this.client = candidate
135+
this.lifecycle.onInitialized()
136+
logger.info(`${this.config.name}: language client started successfully`)
137+
138+
if (this.config.onStarted) {
139+
try {
140+
await this.config.onStarted(candidate)
141+
} catch (onStartedErr) {
142+
logger.warn(`${this.config.name}: onStarted hook failed, cleaning up client: ${onStartedErr}`)
118143
await this.cleanupClient()
119-
throw new ToolkitError(`${this.config.name}: launcher disposed during start`, {
120-
code: 'LspLauncherDisposed',
121-
})
144+
throw onStartedErr
122145
}
146+
}
123147

124-
return candidate
148+
if (this.disposed) {
149+
await this.cleanupClient()
150+
throw this.disposedError('launcher disposed during start')
125151
}
126152

127-
throw new ToolkitError(`${this.config.name}: language server start did not complete`, {
128-
code: 'LspStartFailed',
129-
})
153+
return candidate
154+
}
155+
156+
private disposedError(message: string): ToolkitError {
157+
return new ToolkitError(`${this.config.name}: ${message}`, { code: 'LspLauncherDisposed' })
130158
}
131159

132160
private async cleanupClient(): Promise<void> {
@@ -136,6 +164,8 @@ export class LspLauncher implements Disposable {
136164
return
137165
}
138166

167+
// We initiated this stop, so the ErrorHandler will not see it; tell the lifecycle directly.
168+
this.lifecycle.onServerStopped(true)
139169
try {
140170
await client.stop()
141171
} catch (err) {

0 commit comments

Comments
 (0)