Skip to content

Commit 5e4bfe6

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 5e4bfe6

File tree

4 files changed

+26
-10
lines changed

4 files changed

+26
-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: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,25 +64,38 @@ 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

80+
logger.debug(`fetched "${this.lsName}" manifest: ${this.manifestURL}`)
7281
const manifest = this.parseManifest(resp.content)
7382
await this.saveManifest(resp.eTag, resp.content)
7483
await this.checkDeprecation(manifest)
7584
manifest.location = 'remote'
7685
return manifest
7786
}
7887

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

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

88101
const manifest = this.parseManifest(manifestData.content)
@@ -145,6 +158,7 @@ export class ManifestResolver {
145158
...storage,
146159
[this.lsName]: {
147160
etag,
161+
// XXX: this stores the entire manifest. vscode warns about our globalStorage size...
148162
content,
149163
},
150164
})

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)