Skip to content

Commit 561cd6c

Browse files
Jami CogswellJami Cogswell
authored andcommitted
downloadPublicCommitSource: use 'fetch' API and 'tmp-promise'
1 parent 21a466a commit 561cd6c

File tree

1 file changed

+122
-65
lines changed

1 file changed

+122
-65
lines changed

extensions/ql-vscode/src/variant-analysis/view-autofixes.ts

Lines changed: 122 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -14,28 +14,33 @@ import type { CodeQLCliServer } from "../codeql-cli/cli";
1414
import {
1515
pathExists,
1616
ensureDir,
17-
readdir,
18-
move,
1917
remove,
2018
unlink,
21-
mkdtemp,
2219
readFile,
2320
writeFile,
21+
createWriteStream,
2422
} from "fs-extra";
25-
import { withProgress, progressUpdate } from "../common/vscode/progress";
23+
import {
24+
withProgress,
25+
progressUpdate,
26+
reportStreamProgress,
27+
} from "../common/vscode/progress";
2628
import type { ProgressCallback } from "../common/vscode/progress";
27-
import { join, dirname, parse } from "path";
29+
import { join, parse } from "path";
2830
import { tryGetQueryMetadata } from "../codeql-cli/query-metadata";
2931
import { pluralize } from "../common/word";
3032
import { readRepoTask } from "./repo-tasks-store";
31-
import { tmpdir } from "os";
33+
import { tmpDir } from "../tmp-dir";
3234
import { spawn } from "child_process";
3335
import type { execFileSync } from "child_process";
3436
import { tryOpenExternalFile } from "../common/vscode/external-files";
3537
import type { VariantAnalysisManager } from "./variant-analysis-manager";
3638
import type { VariantAnalysisResultsManager } from "./variant-analysis-results-manager";
37-
import { getAutofixPath, getAutofixModel } from "../config";
39+
import { getAutofixPath, getAutofixModel, downloadTimeout } from "../config";
3840
import { getErrorMessage } from "../common/helpers-pure";
41+
import { createTimeoutSignal } from "../common/fetch-stream";
42+
import { unzipToDirectoryConcurrently } from "../common/unzip-concurrently";
43+
import { reportUnzipProgress } from "../common/vscode/unzip-progress";
3944

4045
// Limit to three repos when generating autofixes so not sending
4146
// too many requests to autofix. Since we only need to validate
@@ -308,7 +313,7 @@ async function processSelectedRepositories(
308313
withProgress(
309314
async (progressForRepo: ProgressCallback) => {
310315
// Get the sarif file.
311-
progressForRepo(progressUpdate(1, 3, `Getting sarif`));
316+
progressForRepo(progressUpdate(1, 3, `Getting sarif...`));
312317
const sarifFile = await getRepoSarifFile(
313318
variantAnalysisResultsManager,
314319
variantAnalysisIdStoragePath,
@@ -332,17 +337,17 @@ async function processSelectedRepositories(
332337

333338
// Download the source root.
334339
// Using `0` as the progress step to force a dynamic vs static progress bar.
335-
// Consider using `reportStreamProgress` as a future enhancement.
336-
progressForRepo(progressUpdate(0, 3, `Downloading source root`));
340+
progressForRepo(progressUpdate(0, 3, `Fetching source root...`));
337341
const srcRootPath = await downloadPublicCommitSource(
338342
nwo,
339343
repoTask.databaseCommitSha,
340344
sourceRootsStoragePath,
341345
credentials,
346+
progressForRepo,
342347
);
343348

344349
// Run autofix.
345-
progressForRepo(progressUpdate(2, 3, `Running autofix`));
350+
progressForRepo(progressUpdate(2, 3, `Running autofix...`));
346351
await runAutofixForRepository(
347352
nwo,
348353
sarifFile,
@@ -354,7 +359,7 @@ async function processSelectedRepositories(
354359
);
355360
},
356361
{
357-
title: `Processing ${nwo}`,
362+
title: `${nwo}`,
358363
cancellable: false,
359364
},
360365
),
@@ -399,6 +404,7 @@ async function downloadPublicCommitSource(
399404
sha: string,
400405
outputPath: string,
401406
credentials: Credentials,
407+
progressCallback: ProgressCallback,
402408
): Promise<string> {
403409
const [owner, repo] = nwo.split("/");
404410
if (!owner || !repo) {
@@ -425,77 +431,128 @@ async function downloadPublicCommitSource(
425431
void extLogger.log(`Fetching source of repository ${nwo} at ${sha}...`);
426432

427433
try {
428-
// Create a temporary directory for downloading
429-
const downloadDir = await mkdtemp(join(tmpdir(), "download-source-"));
430-
const tarballPath = join(downloadDir, "source.tar.gz");
431-
432434
const octokit = await credentials.getOctokit();
433435

434-
// Get the tarball URL
435-
const { url } = await octokit.rest.repos.downloadTarballArchive({
436+
// Get the zipball URL
437+
const { url } = await octokit.rest.repos.downloadZipballArchive({
436438
owner,
437439
repo,
438440
ref: sha,
439441
});
440442

441-
// Download the tarball using spawn
442-
await new Promise<void>((resolve, reject) => {
443-
const curlArgs = [
444-
"-H",
445-
"Accept: application/octet-stream",
446-
"--user-agent",
447-
"GitHub-CodeQL-Extension",
448-
"-L", // Follow redirects
449-
"-o",
450-
tarballPath,
451-
url,
452-
];
453-
454-
const process = spawn("curl", curlArgs, { cwd: downloadDir });
455-
456-
process.on("error", reject);
457-
process.on("exit", (code) =>
458-
code === 0
459-
? resolve()
460-
: reject(new Error(`curl exited with code ${code}`)),
461-
);
462-
});
443+
// Create a temporary directory for downloading
444+
const archivePath = join(
445+
tmpDir.name,
446+
`source-${owner}-${repo}-${Date.now()}.zip`,
447+
);
463448

464-
void extLogger.log(`Download complete, extracting source...`);
449+
// Set timeout
450+
const {
451+
signal,
452+
onData,
453+
dispose: disposeTimeout,
454+
} = createTimeoutSignal(downloadTimeout());
465455

466-
// Extract the tarball
467-
await new Promise<void>((resolve, reject) => {
468-
const process = spawn("tar", ["-xzf", tarballPath], { cwd: downloadDir });
456+
// Fetch the url
457+
let response: Response;
458+
try {
459+
response = await fetch(url, {
460+
headers: {
461+
Accept: "application/zip",
462+
},
463+
signal,
464+
});
465+
} catch (e) {
466+
disposeTimeout();
469467

470-
process.on("error", reject);
471-
process.on("exit", (code) =>
472-
code === 0
473-
? resolve()
474-
: reject(new Error(`tar extraction failed with code ${code}`)),
468+
if (e instanceof DOMException && e.name === "AbortError") {
469+
const thrownError = new Error("The request timed out.");
470+
thrownError.stack = e.stack;
471+
throw thrownError;
472+
}
473+
throw new Error(
474+
`Error fetching source root. Reason: ${getErrorMessage(e)}`,
475475
);
476-
});
476+
}
477477

478-
// Remove the tarball to save space
479-
await unlink(tarballPath);
478+
// Download the source root from the response body
479+
const body = response.body;
480+
if (!body) {
481+
throw new Error("No response body found");
482+
}
480483

481-
// Find the extracted directory (GitHub tarballs extract to a single directory)
482-
const extractedFiles = await readdir(downloadDir);
483-
const sourceDir = extractedFiles.filter((f) => f !== "source.tar.gz")[0];
484+
const archiveFileStream = createWriteStream(archivePath);
484485

485-
if (!sourceDir) {
486-
throw new Error("Failed to find extracted source directory");
487-
}
486+
const contentLength = response.headers.get("content-length");
487+
const totalNumBytes = contentLength
488+
? parseInt(contentLength, 10)
489+
: undefined;
490+
491+
const reportProgress = reportStreamProgress(
492+
"Downloading source root...",
493+
totalNumBytes,
494+
progressCallback,
495+
);
496+
497+
try {
498+
const reader = body.getReader();
499+
for (;;) {
500+
const { done, value } = await reader.read();
501+
if (done) {
502+
break;
503+
}
504+
505+
onData();
506+
reportProgress(value?.length ?? 0);
507+
508+
await new Promise((resolve, reject) => {
509+
archiveFileStream.write(value, (err) => {
510+
if (err) {
511+
reject(err);
512+
}
513+
resolve(undefined);
514+
});
515+
});
516+
}
488517

489-
const extractedSourcePath = join(downloadDir, sourceDir);
518+
await new Promise((resolve, reject) => {
519+
archiveFileStream.close((err) => {
520+
if (err) {
521+
reject(err);
522+
}
523+
resolve(undefined);
524+
});
525+
});
526+
} catch (e) {
527+
// Close and remove the file if an error occurs
528+
archiveFileStream.close(() => {
529+
void remove(archivePath);
530+
});
531+
532+
if (e instanceof DOMException && e.name === "AbortError") {
533+
const thrownError = new Error("The download timed out.");
534+
thrownError.stack = e.stack;
535+
throw thrownError;
536+
}
490537

491-
// Ensure the destination directory's parent exists
492-
await ensureDir(dirname(checkoutDir));
538+
throw new Error(
539+
`Error downloading source root. Reason: ${getErrorMessage(e)}`,
540+
);
541+
} finally {
542+
disposeTimeout();
543+
}
493544

494-
// Move the extracted source to the final location
495-
await move(extractedSourcePath, checkoutDir);
545+
void extLogger.log(`Download complete, extracting source...`);
496546

497-
// Clean up the temporary directory
498-
await remove(downloadDir);
547+
// Extract the downloaded zip file
548+
await unzipToDirectoryConcurrently(
549+
archivePath,
550+
outputPath,
551+
progressCallback
552+
? reportUnzipProgress(`Unzipping source root...`, progressCallback)
553+
: undefined,
554+
);
555+
await remove(archivePath);
499556

500557
return checkoutDir;
501558
} catch (error) {

0 commit comments

Comments
 (0)