Merge pull request #3750 from github/koesie10/switch-to-native-fetch

Switch from node-fetch to native Node.js fetch

Author: koesie10

extensions/ql-vscode/CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [UNRELEASED]
 
 - Increase the required version of VS Code to 1.90.0. [#3737](https://github.com/github/vscode-codeql/pull/3737)
+- Fix a bug where some variant analysis results failed to download. [#3750](https://github.com/github/vscode-codeql/pull/3750)
 
 ## 1.15.0 - 26 September 2024
 

extensions/ql-vscode/package-lock.json

@@ -1985,7 +1985,6 @@
     "js-yaml": "^4.1.0",
     "msw": "^2.2.13",
     "nanoid": "^5.0.7",
-    "node-fetch": "^3.3.2",
     "p-queue": "^8.0.1",
     "react": "^18.3.1",
     "react-dom": "^18.3.1",

extensions/ql-vscode/package.json

@@ -28,7 +28,6 @@ import { reportUnzipProgress } from "../common/vscode/unzip-progress";
 import type { Release } from "./distribution/release";
 import { ReleasesApiConsumer } from "./distribution/releases-api-consumer";
 import { createTimeoutSignal } from "../common/fetch-stream";
-import { AbortError } from "node-fetch";
 
 /**
  * distribution.ts
@@ -416,24 +415,40 @@ class ExtensionSpecificDistributionManager {
       const totalNumBytes = contentLength
         ? parseInt(contentLength, 10)
         : undefined;
-      reportStreamProgress(
-        body,
+
+      const reportProgress = reportStreamProgress(
         `Downloading CodeQL CLI ${release.name}…`,
         totalNumBytes,
         progressCallback,
       );
 
-      body.on("data", onData);
-
-      await new Promise((resolve, reject) => {
-        if (!archiveFile) {
-          throw new Error("Invariant violation: archiveFile not set");
+      const reader = body.getReader();
+      for (;;) {
+        const { done, value } = await reader.read();
+        if (done) {
+          break;
         }
 
-        body.pipe(archiveFile).on("finish", resolve).on("error", reject);
+        onData();
+        reportProgress(value?.length ?? 0);
+
+        await new Promise((resolve, reject) => {
+          archiveFile?.write(value, (err) => {
+            if (err) {
+              reject(err);
+            }
+            resolve(undefined);
+          });
+        });
+      }
 
-        // If an error occurs on the body, we also want to reject the promise (e.g. during a timeout error).
-        body.on("error", reject);
+      await new Promise((resolve, reject) => {
+        archiveFile?.close((err) => {
+          if (err) {
+            reject(err);
+          }
+          resolve(undefined);
+        });
       });
 
       disposeTimeout();
@@ -454,8 +469,8 @@ class ExtensionSpecificDistributionManager {
           : undefined,
       );
     } catch (e) {
-      if (e instanceof AbortError) {
-        const thrownError = new AbortError("The download timed out.");
+      if (e instanceof DOMException && e.name === "AbortError") {
+        const thrownError = new Error("The download timed out.");
         thrownError.stack = e.stack;
         throw thrownError;
       }

extensions/ql-vscode/src/codeql-cli/distribution.ts

@@ -1,5 +1,3 @@
-import type { Response } from "node-fetch";
-import { default as fetch } from "node-fetch";
 import type { Range } from "semver";
 import { compare, parse, satisfies } from "semver";
 import { URL } from "url";

extensions/ql-vscode/src/codeql-cli/distribution/releases-api-consumer.ts

@@ -2,6 +2,7 @@ import { join, resolve } from "path";
 import { pathExists } from "fs-extra";
 import type { SetupServer } from "msw/node";
 import { setupServer } from "msw/node";
+import type { UnhandledRequestStrategy } from "msw/lib/core/utils/request/onUnhandledRequest";
 
 import { DisposableObject } from "../disposable-object";
 
@@ -26,12 +27,14 @@ export class MockGitHubApiServer extends DisposableObject {
     this.recorder = this.push(new Recorder(this.server));
   }
 
-  public startServer(): void {
+  public startServer(
+    onUnhandledRequest: UnhandledRequestStrategy = "bypass",
+  ): void {
     if (this._isListening) {
       return;
     }
 
-    this.server.listen({ onUnhandledRequest: "bypass" });
+    this.server.listen({ onUnhandledRequest });
     this._isListening = true;
   }
 
@@ -54,8 +57,7 @@ export class MockGitHubApiServer extends DisposableObject {
     const scenarioPath = join(scenariosPath, scenarioName);
 
     const handlers = await createRequestHandlers(scenarioPath);
-    this.server.resetHandlers();
-    this.server.use(...handlers);
+    this.server.resetHandlers(...handlers);
   }
 
   public async saveScenario(

extensions/ql-vscode/src/common/mock-gh-api/mock-gh-api-server.ts

@@ -1,7 +1,6 @@
 import { ensureDir, writeFile } from "fs-extra";
 import { join } from "path";
 
-import fetch from "node-fetch";
 import type { SetupServer } from "msw/node";
 
 import { DisposableObject } from "../disposable-object";

extensions/ql-vscode/src/common/mock-gh-api/recorder.ts

@@ -1,10 +1,13 @@
 import { Octokit } from "@octokit/rest";
 import { retry } from "@octokit/plugin-retry";
-import fetch from "node-fetch";
 
 export const AppOctokit = Octokit.defaults({
   request: {
-    fetch,
+    // MSW replaces the global fetch object, so we can't just pass a reference to the
+    // fetch object at initialization time. Instead, we pass a function that will
+    // always call the global fetch object.
+    fetch: (input: string | URL | Request, init?: RequestInit) =>
+      fetch(input, init),
   },
   retry,
 });

extensions/ql-vscode/src/common/octokit.ts

@@ -97,17 +97,15 @@ export function withProgress<R>(
  * Displays a progress monitor that indicates how much progess has been made
  * reading from a stream.
  *
- * @param readable The stream to read progress from
  * @param messagePrefix A prefix for displaying the message
  * @param totalNumBytes Total number of bytes in this stream
  * @param progress The progress callback used to set messages
  */
 export function reportStreamProgress(
-  readable: NodeJS.ReadableStream,
   messagePrefix: string,
   totalNumBytes?: number,
   progress?: ProgressCallback,
-) {
+): (bytesRead: number) => void {
   if (progress && totalNumBytes) {
     let numBytesDownloaded = 0;
     const updateProgress = () => {
@@ -123,15 +121,17 @@ export function reportStreamProgress(
     // Display the progress straight away rather than waiting for the first chunk.
     updateProgress();
 
-    readable.on("data", (data) => {
-      numBytesDownloaded += data.length;
+    return (bytesRead: number) => {
+      numBytesDownloaded += bytesRead;
       updateProgress();
-    });
+    };
   } else if (progress) {
     progress({
       step: 1,
       maxStep: 2,
       message: `${messagePrefix} (Size unknown)`,
     });
   }
+
+  return () => {};
 }

extensions/ql-vscode/src/common/vscode/progress.ts

@@ -1,5 +1,3 @@
-import type { Response } from "node-fetch";
-import fetch, { AbortError } from "node-fetch";
 import type { InputBoxOptions } from "vscode";
 import { Uri, window } from "vscode";
 import type { CodeQLCliServer } from "../codeql-cli/cli";
@@ -536,8 +534,8 @@ export class DatabaseFetcher {
     } catch (e) {
       disposeTimeout();
 
-      if (e instanceof AbortError) {
-        const thrownError = new AbortError("The request timed out.");
+      if (e instanceof DOMException && e.name === "AbortError") {
+        const thrownError = new Error("The request timed out.");
         thrownError.stack = e.stack;
         throw thrownError;
       }
@@ -556,25 +554,50 @@ export class DatabaseFetcher {
     const totalNumBytes = contentLength
       ? parseInt(contentLength, 10)
       : undefined;
-    reportStreamProgress(body, "Downloading database", totalNumBytes, progress);
 
-    body.on("data", onData);
+    const reportProgress = reportStreamProgress(
+      "Downloading database",
+      totalNumBytes,
+      progress,
+    );
 
     try {
-      await new Promise((resolve, reject) => {
-        body.pipe(archiveFileStream).on("finish", resolve).on("error", reject);
+      const reader = body.getReader();
+      for (;;) {
+        const { done, value } = await reader.read();
+        if (done) {
+          break;
+        }
+
+        onData();
+        reportProgress(value?.length ?? 0);
+
+        await new Promise((resolve, reject) => {
+          archiveFileStream.write(value, (err) => {
+            if (err) {
+              reject(err);
+            }
+            resolve(undefined);
+          });
+        });
+      }
 
-        // If an error occurs on the body, we also want to reject the promise (e.g. during a timeout error).
-        body.on("error", reject);
+      await new Promise((resolve, reject) => {
+        archiveFileStream.close((err) => {
+          if (err) {
+            reject(err);
+          }
+          resolve(undefined);
+        });
       });
     } catch (e) {
       // Close and remove the file if an error occurs
       archiveFileStream.close(() => {
         void remove(archivePath);
       });
 
-      if (e instanceof AbortError) {
-        const thrownError = new AbortError("The download timed out.");
+      if (e instanceof DOMException && e.name === "AbortError") {
+        const thrownError = new Error("The download timed out.");
         thrownError.stack = e.stack;
         throw thrownError;
       }