Skip to content

Commit 710f25c

Browse files
authored
fix(amazonq): add a more descriptive issue for firewall problems (aws#7251)
## Problem If a users firewall is blocking downloading the zips or reaching the manifest its not clear to the user what to do ## Solution If the user: 1. doesn't have the latest version cached 2. download of the manifest failed 3. they don't have any previous language servers downloaded it might be a firewall issue. Rather than just `Unable to find a compatible version of the Language Server` we show a more descriptive message to the user <img width="454" alt="Screenshot 2025-05-07 at 2 48 39 PM" src="https://github.com/user-attachments/assets/48053a0f-7dff-4ba2-8b7d-65769b30a56f" /> --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 1d9064f commit 710f25c

File tree

4 files changed

+66
-16
lines changed

4 files changed

+66
-16
lines changed

packages/amazonq/test/e2e/lsp/lspInstallerUtil.ts

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
ManifestResolver,
1515
request,
1616
TargetContent,
17+
ToolkitError,
1718
} from 'aws-core-vscode/shared'
1819
import * as semver from 'semver'
1920
import { assertTelemetry } from 'aws-core-vscode/test'
@@ -201,12 +202,6 @@ export function createLspInstallerTests({
201202
id: lspConfig.id,
202203
manifestLocation: 'remote',
203204
languageServerSetupStage: 'getManifest',
204-
result: 'Failed',
205-
},
206-
{
207-
id: lspConfig.id,
208-
manifestLocation: 'cache',
209-
languageServerSetupStage: 'getManifest',
210205
result: 'Succeeded',
211206
},
212207
{
@@ -282,6 +277,34 @@ export function createLspInstallerTests({
282277
const download = await createInstaller(lspConfig).resolve()
283278
assert.ok(download.assetDirectory.endsWith('-rc.0'))
284279
})
280+
281+
it('throws on firewall error', async () => {
282+
// Stub the manifest resolver to return a valid manifest
283+
sandbox.stub(ManifestResolver.prototype, 'resolve').resolves({
284+
manifestSchemaVersion: '0.0.0',
285+
artifactId: 'foo',
286+
artifactDescription: 'foo',
287+
isManifestDeprecated: false,
288+
versions: [createVersion('1.0.0', targetContents)],
289+
})
290+
291+
// Fail all HTTP requests for the language server
292+
sandbox.stub(request, 'fetch').returns({
293+
response: Promise.resolve({
294+
ok: false,
295+
}),
296+
} as any)
297+
298+
// This should now throw a NetworkConnectivityError
299+
await assert.rejects(
300+
async () => await installer.resolve(),
301+
(err: ToolkitError) => {
302+
assert.strictEqual(err.code, 'NetworkConnectivityError')
303+
assert.ok(err.message.includes('Unable to download dependencies'))
304+
return true
305+
}
306+
)
307+
})
285308
})
286309
})
287310
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export abstract class BaseLspInstaller<T extends ResourcePaths = ResourcePaths,
4545
new Range(supportedVersions, {
4646
includePrerelease: true,
4747
}),
48+
manifestUrl,
4849
this.downloadMessageOverride
4950
).resolve()
5051

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

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export class LanguageServerResolver {
2929
private readonly manifest: Manifest,
3030
private readonly lsName: string,
3131
private readonly versionRange: semver.Range,
32+
private readonly manifestUrl: string,
3233
/**
3334
* Custom message to show user when downloading, if undefined it will use the default.
3435
*/
@@ -90,7 +91,22 @@ export class LanguageServerResolver {
9091

9192
/** Finds an older, cached version of the LSP server bundle. */
9293
private async getFallbackServer(latestVersion: LspVersion): Promise<LspResult> {
93-
const fallbackDirectory = await this.getFallbackDir(latestVersion.serverVersion)
94+
const cachedVersions = await this.getCachedVersions()
95+
if (cachedVersions.length === 0) {
96+
/**
97+
* at this point the latest version doesn't exist locally, lsp download (with retries) failed, and there are no cached fallback versions.
98+
* This _probably_ only happens when the user hit a firewall/proxy issue, since otherwise they would probably have at least
99+
* one other language server locally
100+
*/
101+
throw new ToolkitError(
102+
`Unable to download dependencies from ${this.manifestUrl}. Check your network connectivity or firewall configuration and then try again.`,
103+
{
104+
code: 'NetworkConnectivityError',
105+
}
106+
)
107+
}
108+
109+
const fallbackDirectory = await this.getFallbackDir(latestVersion.serverVersion, cachedVersions)
94110
if (!fallbackDirectory) {
95111
throw new ToolkitError('Unable to find a compatible version of the Language Server', {
96112
code: 'IncompatibleVersion',
@@ -142,13 +158,22 @@ export class LanguageServerResolver {
142158
assetDirectory: cacheDirectory,
143159
}
144160
} else {
161+
await this.cleanupVersion(latestVersion.serverVersion)
145162
throw new ToolkitError('Failed to download server from remote', { code: 'RemoteDownloadFailed' })
146163
}
147164
} finally {
148165
timeout.dispose()
149166
}
150167
}
151168

169+
private async cleanupVersion(version: string) {
170+
// clean up the X.X.X download directory since the download failed
171+
const downloadDirectory = this.getDownloadDirectory(version)
172+
if (await fs.existsDir(downloadDirectory)) {
173+
await fs.delete(downloadDirectory)
174+
}
175+
}
176+
152177
/** Gets the current local ("cached") LSP server bundle. */
153178
private async getLocalServer(
154179
cacheDirectory: string,
@@ -178,16 +203,9 @@ export class LanguageServerResolver {
178203
/**
179204
* Returns the path to the most compatible cached LSP version that can serve as a fallback
180205
**/
181-
private async getFallbackDir(version: string) {
206+
private async getFallbackDir(version: string, cachedVersions: string[]) {
182207
const compatibleLspVersions = this.compatibleManifestLspVersion()
183208

184-
// determine all folders containing lsp versions in the fallback parent folder
185-
const cachedVersions = (await fs.readdir(this.defaultDownloadFolder()))
186-
.filter(([_, filetype]) => filetype === FileType.Directory)
187-
.map(([pathName, _]) => semver.parse(pathName))
188-
.filter((ver): ver is semver.SemVer => ver !== null)
189-
.map((x) => x.version)
190-
191209
const expectedVersion = semver.parse(version)
192210
if (!expectedVersion) {
193211
return undefined
@@ -203,6 +221,15 @@ export class LanguageServerResolver {
203221
return fallbackDir.length > 0 ? fallbackDir[0] : undefined
204222
}
205223

224+
private async getCachedVersions() {
225+
// determine all folders containing lsp versions in the parent folder
226+
return (await fs.readdir(this.defaultDownloadFolder()))
227+
.filter(([_, filetype]) => filetype === FileType.Directory)
228+
.map(([pathName, _]) => semver.parse(pathName))
229+
.filter((ver): ver is semver.SemVer => ver !== null)
230+
.map((x) => x.version)
231+
}
232+
206233
/**
207234
* Validate the local cache directory of the given lsp version (matches expected hash)
208235
* If valid return cache directory, else return undefined

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ export class ManifestResolver {
6969

7070
const localManifest = await this.getLocalManifest(true).catch(() => undefined)
7171
if (localManifest) {
72-
localManifest.location = 'remote'
7372
return localManifest
7473
} else {
7574
// Will emit a `languageServer_setup` result=failed metric...

0 commit comments

Comments
 (0)