From 775628c4a8bf866133518772c899b4a9e15874e6 Mon Sep 17 00:00:00 2001
From: Shati Patel <42641846+shati-patel@users.noreply.github.com>
Date: Tue, 14 Jul 2026 16:37:58 +0000
Subject: [PATCH] Refactor unzip functionality to limit concurrency and improve
 error handling in stream copying

---
 .../src/common/unzip-concurrently.ts          |  2 +-
 extensions/ql-vscode/src/common/unzip.ts      | 31 +++-----
 .../test/unit-tests/common/unzip.test.ts      | 78 +++++++++++++++++++
 3 files changed, 90 insertions(+), 21 deletions(-)

diff --git a/extensions/ql-vscode/src/common/unzip-concurrently.ts b/extensions/ql-vscode/src/common/unzip-concurrently.ts
index 2a8136a53d3..30140c64644 100644
--- a/extensions/ql-vscode/src/common/unzip-concurrently.ts
+++ b/extensions/ql-vscode/src/common/unzip-concurrently.ts
@@ -9,7 +9,7 @@ export async function unzipToDirectoryConcurrently(
   progress?: UnzipProgressCallback,
 ): Promise<void> {
   const queue = new PQueue({
-    concurrency: availableParallelism(),
+    concurrency: Math.min(availableParallelism(), 4),
   });
 
   return unzipToDirectory(
diff --git a/extensions/ql-vscode/src/common/unzip.ts b/extensions/ql-vscode/src/common/unzip.ts
index 9a35eedad38..c015c6d3175 100644
--- a/extensions/ql-vscode/src/common/unzip.ts
+++ b/extensions/ql-vscode/src/common/unzip.ts
@@ -2,6 +2,7 @@ import type { Entry as ZipEntry, Options as ZipOptions, ZipFile } from "yauzl";
 import { open } from "yauzl";
 import type { Readable } from "stream";
 import { Transform } from "stream";
+import { pipeline } from "stream/promises";
 import { dirname, join } from "path";
 import type { WriteStream } from "fs";
 import { createWriteStream, ensureDir } from "fs-extra";
@@ -93,26 +94,16 @@ async function copyStream(
   writeStream: WriteStream,
   bytesExtractedCallback?: (bytesExtracted: number) => void,
 ): Promise<void> {
-  return new Promise((resolve, reject) => {
-    readable.on("error", (err) => {
-      reject(err);
-    });
-    readable.on("end", () => {
-      resolve();
-    });
-
-    readable
-      .pipe(
-        new Transform({
-          transform(chunk, _encoding, callback) {
-            bytesExtractedCallback?.(chunk.length);
-            this.push(chunk);
-            callback();
-          },
-        }),
-      )
-      .pipe(writeStream);
-  });
+  await pipeline(
+    readable,
+    new Transform({
+      transform(chunk, _encoding, callback) {
+        bytesExtractedCallback?.(chunk.length);
+        callback(null, chunk);
+      },
+    }),
+    writeStream,
+  );
 }
 
 type UnzipProgress = {
diff --git a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts
index 5b724d9cffe..a7f68222247 100644
--- a/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts
+++ b/extensions/ql-vscode/test/unit-tests/common/unzip.test.ts
@@ -1,6 +1,8 @@
 import { createHash } from "crypto";
 import { open } from "fs/promises";
 import { join, relative, resolve, sep } from "path";
+import { Readable } from "stream";
+import { createWriteStream } from "fs";
 import { chmod, pathExists, readdir } from "fs-extra";
 import type { DirectoryResult } from "tmp-promise";
 import { dir } from "tmp-promise";
@@ -276,3 +278,79 @@ async function computeHash(contents: Buffer) {
 
   return hash.digest("hex");
 }
+
+describe("copyStream error handling", () => {
+  let tmpDir: DirectoryResult;
+
+  beforeEach(async () => {
+    tmpDir = await dir({
+      unsafeCleanup: true,
+    });
+  });
+
+  afterEach(async () => {
+    await tmpDir?.cleanup();
+  });
+
+  it("rejects when the write stream errors mid-extraction", async () => {
+    // Use a real zip to trigger unzip, but make the destination read-only
+    // so the write stream fails. This verifies the promise rejects rather
+    // than hanging indefinitely.
+    const destPath = join(tmpDir.path, "output");
+
+    // Extract once to create the directory structure
+    await unzipToDirectorySequentially(zipPath, destPath);
+
+    // Make a file read-only so re-extraction will fail on write
+    const targetFile = join(destPath, "directory", "file.txt");
+    await chmod(targetFile, 0o000);
+
+    // On Windows, chmod doesn't prevent writes, so skip assertion there
+    if (process.platform === "win32") {
+      await chmod(targetFile, 0o644);
+      return;
+    }
+
+    // Re-extract — should reject with a write error, not hang
+    await expect(
+      unzipToDirectorySequentially(zipPath, destPath),
+    ).rejects.toThrow();
+
+    // Restore permissions for cleanup
+    await chmod(targetFile, 0o644);
+  });
+
+  it("rejects when the write stream is destroyed mid-copy", async () => {
+    // Directly test the pipeline behavior: a destroyed write stream should
+    // cause rejection, not a hang.
+    const destFile = join(tmpDir.path, "output.bin");
+    const writeStream = createWriteStream(destFile);
+
+    // Create a readable that emits data then waits
+    const readable = new Readable({
+      read() {
+        this.push(Buffer.alloc(1024, "x"));
+        // Destroy the write stream after the first chunk to simulate an error
+        setImmediate(() => {
+          writeStream.destroy(new Error("simulated write failure"));
+        });
+      },
+    });
+
+    // Import pipeline to test the same pattern as copyStream
+    const { pipeline } = await import("stream/promises");
+    const { Transform } = await import("stream");
+
+    await expect(
+      pipeline(
+        readable,
+        new Transform({
+          transform(chunk, _encoding, callback) {
+            callback(null, chunk);
+          },
+        }),
+        writeStream,
+      ),
+    ).rejects.toThrow("simulated write failure");
+  });
+});