Skip to content

Commit c3d3903

Browse files
committed
refactor: linearize fetchBinary decision tree for readability
The version check, lock-wait, and rename logic in fetchBinary used nested if/else-if blocks and a needsDownload flag that made the control flow hard to follow. This refactors it into sequential guard clauses (version match, downloads disabled, version mismatch) and replaces the flag with an early return. The duplicated rename-to- final-path logic is extracted into a small helper used in both the lock-wait and download paths. No behavior changes.
1 parent 7cda5ed commit c3d3903

File tree

1 file changed

+59
-51
lines changed

1 file changed

+59
-51
lines changed

src/core/cliManager.ts

Lines changed: 59 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -139,27 +139,15 @@ export class CliManager {
139139
`Resolved binary: ${resolved.binPath} (${resolved.source})`,
140140
);
141141

142-
// Check existing binary version when one was found.
142+
let existingVersion: string | null = null;
143143
if (resolved.source !== "not-found") {
144144
this.output.debug(
145145
"Existing binary size is",
146146
prettyBytes(resolved.stat.size),
147147
);
148148
try {
149-
const version = await cliVersion(resolved.binPath);
150-
this.output.debug("Existing binary version is", version);
151-
if (version === buildInfo.version) {
152-
this.output.debug("Existing binary matches server version");
153-
return resolved.binPath;
154-
} else if (!enableDownloads) {
155-
this.output.info(
156-
"Using existing binary despite version mismatch because downloads are disabled",
157-
);
158-
return resolved.binPath;
159-
}
160-
this.output.info(
161-
"Downloading since existing binary does not match the server version",
162-
);
149+
existingVersion = await cliVersion(resolved.binPath);
150+
this.output.debug("Existing binary version is", existingVersion);
163151
} catch (error) {
164152
this.output.warn(
165153
"Unable to get version of existing binary, downloading instead",
@@ -170,11 +158,28 @@ export class CliManager {
170158
this.output.info("No existing binary found, starting download");
171159
}
172160

161+
if (existingVersion === buildInfo.version) {
162+
this.output.debug("Existing binary matches server version");
163+
return resolved.binPath;
164+
}
165+
173166
if (!enableDownloads) {
167+
if (existingVersion) {
168+
this.output.info(
169+
"Using existing binary despite version mismatch because downloads are disabled",
170+
);
171+
return resolved.binPath;
172+
}
174173
this.output.warn("Unable to download CLI because downloads are disabled");
175174
throw new Error("Unable to download CLI because downloads are disabled");
176175
}
177176

177+
if (existingVersion) {
178+
this.output.info(
179+
"Downloading since existing binary does not match the server version",
180+
);
181+
}
182+
178183
// Always download using the platform-specific name.
179184
const downloadBinPath = path.join(
180185
path.dirname(resolved.binPath),
@@ -196,8 +201,7 @@ export class CliManager {
196201
);
197202
this.output.debug("Acquired download lock");
198203

199-
// If we waited for another process, re-check if binary is now ready
200-
let needsDownload = true;
204+
// Another process may have finished the download while we waited.
201205
if (lockResult.waited) {
202206
const latestBuildInfo = await restClient.getBuildInfo();
203207
this.output.debug("Got latest server version", latestBuildInfo.version);
@@ -207,43 +211,26 @@ export class CliManager {
207211
latestBuildInfo.version,
208212
);
209213
if (recheckAfterWait.matches) {
210-
this.output.debug(
211-
"Using existing binary since it matches the latest server version",
212-
);
213-
needsDownload = false;
214-
} else {
215-
const latestParsedVersion = semver.parse(latestBuildInfo.version);
216-
if (!latestParsedVersion) {
217-
throw new Error(
218-
`Got invalid version from deployment: ${latestBuildInfo.version}`,
219-
);
220-
}
221-
latestVersion = latestParsedVersion;
214+
this.output.debug("Binary already matches server version after wait");
215+
return await this.renameToFinalPath(resolved, downloadBinPath);
222216
}
223-
}
224217

225-
if (needsDownload) {
226-
await this.performBinaryDownload(
227-
restClient,
228-
latestVersion,
229-
downloadBinPath,
230-
progressLogPath,
231-
);
218+
const latestParsedVersion = semver.parse(latestBuildInfo.version);
219+
if (!latestParsedVersion) {
220+
throw new Error(
221+
`Got invalid version from deployment: ${latestBuildInfo.version}`,
222+
);
223+
}
224+
latestVersion = latestParsedVersion;
232225
}
233226

234-
// Rename to user-configured file path while we hold the lock.
235-
if (
236-
resolved.source === "file-path" &&
237-
downloadBinPath !== resolved.binPath
238-
) {
239-
this.output.info(
240-
"Renaming downloaded binary to",
241-
path.basename(resolved.binPath),
242-
);
243-
await fs.rename(downloadBinPath, resolved.binPath);
244-
return resolved.binPath;
245-
}
246-
return downloadBinPath;
227+
await this.performBinaryDownload(
228+
restClient,
229+
latestVersion,
230+
downloadBinPath,
231+
progressLogPath,
232+
);
233+
return await this.renameToFinalPath(resolved, downloadBinPath);
247234
} catch (error) {
248235
const fallback = await this.handleAnyBinaryFailure(
249236
error,
@@ -288,6 +275,27 @@ export class CliManager {
288275
}
289276
}
290277

278+
/**
279+
* Rename the downloaded binary to the user-configured file path if needed.
280+
*/
281+
private async renameToFinalPath(
282+
resolved: ResolvedBinary,
283+
downloadBinPath: string,
284+
): Promise<string> {
285+
if (
286+
resolved.source === "file-path" &&
287+
downloadBinPath !== resolved.binPath
288+
) {
289+
this.output.info(
290+
"Renaming downloaded binary to",
291+
path.basename(resolved.binPath),
292+
);
293+
await fs.rename(downloadBinPath, resolved.binPath);
294+
return resolved.binPath;
295+
}
296+
return downloadBinPath;
297+
}
298+
291299
/**
292300
* Prompt the user to use an existing binary version.
293301
*/
@@ -388,7 +396,7 @@ export class CliManager {
388396
}
389397
}
390398

391-
// Last resort: try the most recent .old-* backup.
399+
// Last resort: most recent .old-* backup (deferred to avoid IO when unnecessary).
392400
const oldBinaries = await cliUtils.findOldBinaries(binPath);
393401
if (oldBinaries.length > 0) {
394402
const old = await tryCandidate(oldBinaries[0]);

0 commit comments

Comments
 (0)