Skip to content

Commit 4659b80

Browse files
committed
fix(lsp): manifest resolver always reports "Failed to download"
Problem: Manifest resolver always reports: Failed to download latest "…" manifest. Falling back to local manifest. Solution: In `fetchRemoteManifest()`, if the ETag indicates no new manifest is needed, return the local manifest instead of throwing an error
1 parent e5fa306 commit 4659b80

File tree

4 files changed

+25
-10
lines changed

4 files changed

+25
-10
lines changed

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ export abstract class BaseLspInstaller<T extends ResourcePaths = ResourcePaths,
5252
await this.postInstall(assetDirectory)
5353

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

5759
const r = {
5860
...installationResult,

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ export class LanguageServerResolver {
7373
* ```
7474
*/
7575
const resolved = await tryStageResolvers('getServer', serverResolvers, getServerVersion)
76-
logger.info('Finished updating "%s" LSP server: %O', this.lsName, resolved.assetDirectory)
76+
logger.info('Finished preparing "%s" LSP server: %O', this.lsName, resolved.assetDirectory)
7777
return resolved
7878
}
7979

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

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,17 @@ export class ManifestResolver {
6464
}).getNewETagContent(this.getEtag())
6565

6666
if (!resp.content) {
67-
throw new ToolkitError(
68-
`New content was not downloaded; fallback to the locally stored "${this.lsName}" manifest`
69-
)
67+
const msg = `"${this.lsName}" manifest unchanged, skipping download: ${this.manifestURL}`
68+
logger.debug(msg)
69+
70+
const localManifest = await this.getLocalManifest(true).catch(() => undefined)
71+
if (localManifest) {
72+
localManifest.location = 'remote'
73+
return localManifest
74+
} else {
75+
// Will emit a `languageServer_setup` result=failed metric...
76+
throw new ToolkitError(msg)
77+
}
7078
}
7179

7280
const manifest = this.parseManifest(resp.content)
@@ -76,13 +84,17 @@ export class ManifestResolver {
7684
return manifest
7785
}
7886

79-
private async getLocalManifest(): Promise<Manifest> {
80-
logger.info(`Failed to download latest "${this.lsName}" manifest. Falling back to local manifest.`)
87+
private async getLocalManifest(silent: boolean = false): Promise<Manifest> {
88+
if (!silent) {
89+
logger.info(`trying local "${this.lsName}" manifest...`)
90+
}
8191
const storage = this.getStorage()
8292
const manifestData = storage[this.lsName]
8393

8494
if (!manifestData?.content) {
85-
throw new ToolkitError(`Failed to download "${this.lsName}" manifest and no local manifest found.`)
95+
const msg = `local "${this.lsName}" manifest not found`
96+
logger.warn(msg)
97+
throw new ToolkitError(msg)
8698
}
8799

88100
const manifest = this.parseManifest(manifestData.content)
@@ -145,6 +157,7 @@ export class ManifestResolver {
145157
...storage,
146158
[this.lsName]: {
147159
etag,
160+
// XXX: this stores the entire manifest. vscode warns about our globalStorage size...
148161
content,
149162
},
150163
})

packages/core/src/shared/resourcefetcher/httpResourceFetcher.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ export class HttpResourceFetcher implements ResourceFetcher<Response> {
6868
if (response.status === 304) {
6969
// Explanation: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-None-Match
7070
contents = undefined
71-
this.logger.verbose(`E-Tag, ${eTagResponse}, matched. No content downloaded from: ${this.url}`)
71+
this.logger.verbose(`E-Tag matched (${eTagResponse}). Download skipped: ${this.url}`)
7272
} else {
73-
this.logger.verbose(`No E-Tag match. Downloaded content from: ${this.logText()}`)
73+
this.logger.verbose(`E-Tag not matched. Downloaded: ${this.logText()}`)
7474
}
7575

7676
return { content: contents, eTag: eTagResponse }

0 commit comments

Comments
 (0)