diff --git a/src/main/core/bridgeInstall.ts b/src/main/core/bridgeInstall.ts index f29e6e5..670c78a 100644 --- a/src/main/core/bridgeInstall.ts +++ b/src/main/core/bridgeInstall.ts @@ -24,6 +24,7 @@ import { bridgeSupported, bridgeVersionOf, compareBridgeVersions, + installPlan, isGithubAssetUrl, pickBridgeAsset, type BridgeAsset, @@ -48,19 +49,24 @@ function pluginsDir(serverId: string): string { * and the operator sees "MSMS-Bridge could not be enabled" with no hint that * the cause is the jar they installed sitting next to the one they already had. */ -function installedJars(serverId: string): { name: string; version: string }[] { +function installedJars(serverId: string): { name: string; version: string; enabled: boolean }[] { const dir = pluginsDir(serverId) if (!existsSync(dir)) return [] - const out: { name: string; version: string }[] = [] + const out: { name: string; version: string; enabled: boolean }[] = [] for (const name of readdirSync(dir)) { - const version = bridgeVersionOf(name) - if (version) out.push({ name, version }) + // A `.jar.disabled` is not loaded by Bukkit, so it is not installed as far + // as the status is concerned — but it IS a stale copy, and leaving one next + // to a newly installed jar means the folder accumulates a bridge per + // version an operator ever turned off. + const enabled = !/\.disabled$/i.test(name) + const version = bridgeVersionOf(enabled ? name : name.replace(/\.disabled$/i, '')) + if (version) out.push({ name, version, enabled }) } return out.sort((a, b) => compareBridgeVersions(b.version, a.version)) } export function installedBridgeVersion(serverId: string): string | null { - return installedJars(serverId)[0]?.version ?? null + return installedJars(serverId).find((j) => j.enabled)?.version ?? null } // ---- the copy that ships with the app ---- @@ -141,19 +147,11 @@ export async function bridgeStatus(serverId: string): Promise { const bundled = bundledBridge() const offline = remote === null - let source: 'github' | 'bundled' | null = null - let latest: string | null = null - if (remote && bundled) { - const useRemote = compareBridgeVersions(remote.version, bundled.version) >= 0 - source = useRemote ? 'github' : 'bundled' - latest = useRemote ? remote.version : bundled.version - } else if (remote) { - source = 'github' - latest = remote.version - } else if (bundled) { - source = 'bundled' - latest = bundled.version - } + // The same ordering the install will actually use, so the version the warning + // names is the version the button delivers. + const first = installPlan({ remote, bundled })[0] ?? null + const source = first + const latest = first === 'github' ? (remote?.version ?? null) : (bundled?.version ?? null) const need = bridgeNeed({ type: s.type, installed, latest }) return { serverId, ...need, source: need.actionable ? source : null, ...(offline ? { offline: true } : {}) } @@ -180,43 +178,55 @@ export async function installBridge( const remote = await latestBridge() const bundled = bundledBridge() - const preferRemote = - !!remote && (!bundled || compareBridgeVersions(remote.version, bundled.version) >= 0) - if (!remote && !bundled) return refuse(serverId, who, 'no-jar-available') + const plan = installPlan({ remote, bundled }) + if (!plan.length) return refuse(serverId, who, 'no-jar-available') const dir = pluginsDir(serverId) mkdirSync(dir, { recursive: true }) - let version: string - let written: string - let from: 'github' | 'bundled' + let version = '' + let written = '' + let from: 'github' | 'bundled' | null = null + let lastError = 'no-jar-available' - if (preferRemote && remote) { - // Re-checked here and not only where the asset was picked. This is the last - // line before a URL from a response body is handed to the downloader, and - // the two checks are cheap next to what passing a bad one would cost. - if (!isGithubAssetUrl(remote.url)) return refuse(serverId, who, 'bad-asset-url') - if (!BRIDGE_JAR_RE.test(remote.name)) return refuse(serverId, who, 'bad-asset-name') - written = join(dir, remote.name) - try { - await downloadFile(remote.url, written, { - ...(remote.sha256 ? { sha256: remote.sha256 } : {}), - timeoutMs: 60_000 - }) - } catch (e) { - rmSync(written, { force: true }) - return refuse(serverId, who, 'download-failed: ' + String(e)) + for (const step of plan) { + if (step === 'github' && remote) { + // Re-checked here and not only where the asset was picked. This is the + // last line before a URL from a response body is handed to the + // downloader, and the two checks are cheap next to what passing a bad one + // would cost. A refusal here does NOT abandon the install — it falls to + // the bundled jar like any other failure of this step. + if (!isGithubAssetUrl(remote.url) || !BRIDGE_JAR_RE.test(remote.name)) { + lastError = 'bad-asset' + continue + } + const dest = join(dir, remote.name) + try { + await downloadFile(remote.url, dest, { + ...(remote.sha256 ? { sha256: remote.sha256 } : {}), + timeoutMs: 60_000 + }) + } catch (e) { + // A half-written or hash-mismatched file must not be left behind: the + // next status check would read its name and report it as installed. + rmSync(dest, { force: true }) + lastError = 'download-failed: ' + String(e) + continue + } + version = remote.version + written = dest + from = 'github' + break + } + if (step === 'bundled' && bundled) { + written = join(dir, bundled.name) + copyFileSync(bundled.path, written) + version = bundled.version + from = 'bundled' + break } - version = remote.version - from = 'github' - } else if (bundled) { - written = join(dir, bundled.name) - copyFileSync(bundled.path, written) - version = bundled.version - from = 'bundled' - } else { - return refuse(serverId, who, 'no-jar-available') } + if (!from) return refuse(serverId, who, lastError) // Only after the new jar is on disk. Removing first would leave a server with // no bridge at all if the download failed halfway. diff --git a/src/main/smoke.ts b/src/main/smoke.ts index 8b4b0c4..34ec104 100644 --- a/src/main/smoke.ts +++ b/src/main/smoke.ts @@ -82,6 +82,7 @@ import { bridgeNeed, bridgeVersionOf, compareBridgeVersions, + installPlan, pickBridgeAsset, sha256Of } from '@shared/bridgeRelease' @@ -1042,6 +1043,27 @@ export async function runModUpdateSmoke(): Promise { if (need(t, null, '1.0.0') !== 'missing') return fail(t + ' should be offered the bridge') } + // What the install actually tries, in order. Committing to one source + // makes the bundled jar a fallback for exactly one failure — GitHub's API + // being unreachable — and leaves the commoner one uncovered: the API + // answering while the asset download fails, which would refuse the + // install with a perfectly good jar sitting on disk. + const plan = (r: string | null, b: string | null): string => + installPlan({ + remote: r ? { version: r } : null, + bundled: b ? { version: b } : null + }).join(',') + if (plan('1.2.0', '1.0.0') !== 'github,bundled') { + return fail('a failed download would not fall back to the bundled jar: ' + plan('1.2.0', '1.0.0')) + } + if (plan('1.0.0', '1.0.0') !== 'github,bundled') return fail('equal versions should still fall back') + // Downloading a jar older than the one on disk is work done to arrive + // somewhere worse. + if (plan('0.9.0', '1.0.0') !== 'bundled') return fail('an older release should not be downloaded') + if (plan('1.0.0', null) !== 'github') return fail('with no bundled jar there is nothing to fall back to') + if (plan(null, '1.0.0') !== 'bundled') return fail('with no release the bundled jar is the plan') + if (plan(null, null) !== '') return fail('with nothing available the plan must be empty') + // The jar that ships with the app, which is the whole offline story. An // absent one would make every assertion above true and the feature // useless on the box a server manager actually runs on. diff --git a/src/shared/bridgeRelease.ts b/src/shared/bridgeRelease.ts index 6d57a6a..b6f7584 100644 --- a/src/shared/bridgeRelease.ts +++ b/src/shared/bridgeRelease.ts @@ -201,6 +201,37 @@ export function bridgeNeed(input: { return { state: 'ok', installed, ...(latest ? { latest } : {}), actionable: false } } +/** + * What to try, in order, when installing. + * + * The obvious implementation picks one source and commits to it, and that turns + * the bundled jar into a fallback for exactly one failure — GitHub's API being + * unreachable — while leaving the more common one uncovered. The API answering + * and the asset download then failing (a CDN hiccup, a proxy that allows + * api.github.com but not objects.githubusercontent.com, a connection dropped + * mid-stream) would refuse the install with a perfectly good jar sitting on + * disk. + * + * So it is a list. An older bundled jar is still a working bridge, and a + * working bridge beats none; the result reports which source it came from, so + * nothing is claimed that is not true. + * + * `bundled` alone when it is the newer of the two: downloading a jar older than + * the one already on disk is work done to arrive somewhere worse. + */ +export function installPlan(o: { + remote?: { version: string } | null + bundled?: { version: string } | null +}): ('github' | 'bundled')[] { + const { remote, bundled } = o + if (!remote && !bundled) return [] + if (!remote) return ['bundled'] + if (!bundled) return ['github'] + return compareBridgeVersions(remote.version, bundled.version) >= 0 + ? ['github', 'bundled'] + : ['bundled'] +} + /** What the panel, the desktop app and the API all receive. */ export interface BridgeStatus extends BridgeNeed { serverId: string