fix(setup): replace the fixed Hopper download deadline with a stall timeout (#7150)

* fix(setup): replace the fixed Hopper download deadline with a stall timeout Hopper package downloads aborted after a fixed 120 s, which was also the largest deadline the download owner accepted. The current 32 MB Linux package needs about 132 s at the reporter's sustained 245 KB/s, so setup could never finish on that connection, and the timeout surfaced only as Node's generic abort message. Abort a download when no response bytes arrive for 30 s, or when it outlives a 15-minute overall bound, with a tagged HopperDownloadTimeoutError that names the URL, the limit reached and the bytes received. Linux and macOS setup already relay a thrown download error's message; the Linux publisher now keeps it for release metadata and public key fetches instead of a generic verification failure. Metadata and key fetches keep their 10 s bound. Refs #7077 Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(setup): keep the macOS release feed on the metadata limit The stall-timeout change left the macOS release feed fetch on the package download bounds. That fetch could then wait up to 15 minutes instead of the 120 s it had, while the installation guide says release metadata keeps a 10 s limit. Only the Linux publisher owned that limit. Move the 10 s metadata limit into HopperDownload.ts beside the package bounds. The Linux feed and key fetches use it, and the macOS installer now passes it for its feed fetch through the installer host. Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: Claude Opus 5.5 <[email protected]>
2026-11-07 05:56:19 -10:00 · 2026-11-05 15:39:03 +08:00
1 parent 8e3d794834
commit 5fd325066d
@@ -720,6 +720,11 @@ and status when publisher verification cannot download the current package.
 Hopper metadata and package downloads require HTTPS throughout the redirect
 chain; a redirect to HTTP or to a URL carrying credentials is refused before
 that destination is requested, and those credentials are not reported.
+A package download fails when no bytes arrive for 30 seconds or when it has not
+finished within 15 minutes, so a slow connection that keeps delivering bytes can
+complete. Release metadata and the publisher key keep a 10-second limit. A
+`timeout` failure reports the `limit` it reached (`stalled` or `deadline`), its
+`timeoutMs` and the `receivedBytes` before it stopped.

 On macOS, approved setup downloads the official DMG, checks its published size
 and digest, validates the application bundle, and atomically installs it to
@@ -21,7 +21,14 @@ export type HopperDownloadFailure = { readonly url: string } & (
      /** Absent when the response did not carry a valid HTTP failure status. */
      readonly httpStatus?: number;
    }
-  | { readonly kind: "timeout"; readonly timeoutMs: number }
+  | {
+      readonly kind: "timeout";
+      /** No bytes arrived within the stall bound, or the overall bound elapsed. */
+      readonly limit: "stalled" | "deadline";
+      readonly timeoutMs: number;
+      /** Response body bytes received before the limit was reached. */
+      readonly receivedBytes: number;
+    }
  | { readonly kind: "network"; readonly detail: string }
  | { readonly kind: "response_too_large"; readonly maxBytes: number }
  | { readonly kind: "url_refused"; readonly refusal: HopperUrlRefusal }
@@ -41,6 +48,18 @@ export type HopperRequestFailure = HopperDownloadFailure & {
 /** Maximum admitted vendor response bytes, shared by Hopper setup adapters. */
 export const HOPPER_DOWNLOAD_BYTES = 100_000_000;

+/** Longest wait for the response or its next bytes before a download stalls. */
+export const HOPPER_DOWNLOAD_STALL_MS = 30_000;
+
+/**
+ * Overall bound for one download. A slow connection that keeps delivering
+ * bytes is bounded here rather than by a deadline sized for fast networks.
+ */
+export const HOPPER_DOWNLOAD_TIMEOUT_MS = 15 * 60_000;
+
+/** Small release feed and key requests must not use the package bounds. */
+export const HOPPER_METADATA_TIMEOUT_MS = 10_000;
+
 /**
 * Bound observed response bytes before retaining them, including chunked bodies.
 * Expected transport outcomes are returned; caller cancellation still rejects
@@ -51,11 +70,16 @@ export const downloadHopperPackage = async (
  options: {
    readonly signal?: AbortSignal;
    readonly maxBytes?: number;
+    /** Overall bound; a stall bound applies only when it is shorter. */
    readonly timeoutMs?: number;
  } = {},
 ): Promise<HopperDownload> => {
-  const timeoutMs = options.timeoutMs ?? 120_000;
-  if (!Number.isSafeInteger(timeoutMs) || timeoutMs < 1 || timeoutMs > 120_000)
+  const timeoutMs = options.timeoutMs ?? HOPPER_DOWNLOAD_TIMEOUT_MS;
+  if (
+    !Number.isSafeInteger(timeoutMs) ||
+    timeoutMs < 1 ||
+    timeoutMs > HOPPER_DOWNLOAD_TIMEOUT_MS
+  )
    throw new RangeError("Invalid Hopper response time budget");
  const maxBytes = options.maxBytes ?? HOPPER_DOWNLOAD_BYTES;
  if (
@@ -64,22 +88,47 @@ export const downloadHopperPackage = async (
    maxBytes > HOPPER_DOWNLOAD_BYTES
  )
    throw new RangeError("Invalid Hopper response byte budget");
-  const deadline = AbortSignal.timeout(timeoutMs);
  // Remove transport credentials once, where every failure is created.
  const reported = sanitizeBrowserUrl(url).url;
+  let receivedBytes = 0;
+  let expired:
+    | {
+        readonly limit: "stalled" | "deadline";
+        readonly timeoutMs: number;
+        readonly receivedBytes: number;
+      }
+    | undefined;
+  const deadline = new AbortController();
+  const expire = (limit: "stalled" | "deadline", limitMs: number) => {
+    expired ??= { limit, timeoutMs: limitMs, receivedBytes };
+    deadline.abort();
+  };
+  const overall = setTimeout(() => {
+    expire("deadline", timeoutMs);
+  }, timeoutMs);
+  let stall: NodeJS.Timeout | undefined;
+  const progressed = (bytes: number) => {
+    receivedBytes = bytes;
+    if (HOPPER_DOWNLOAD_STALL_MS >= timeoutMs) return;
+    clearTimeout(stall);
+    stall = setTimeout(() => {
+      expire("stalled", HOPPER_DOWNLOAD_STALL_MS);
+    }, HOPPER_DOWNLOAD_STALL_MS);
+  };
  try {
-    return await readHopperResponse(
-      url,
-      reported,
-      options.signal === undefined
-        ? deadline
-        : AbortSignal.any([options.signal, deadline]),
+    progressed(0);
+    return await readHopperResponse(url, reported, {
+      signal:
+        options.signal === undefined
+          ? deadline.signal
+          : AbortSignal.any([options.signal, deadline.signal]),
      maxBytes,
-    );
+      progressed,
+    });
  } catch (cause: unknown) {
    if (options.signal?.aborted === true) throw cause;
-    if (deadline.aborted)
-      return err({ url: reported, kind: "timeout", timeoutMs });
+    if (expired !== undefined)
+      return err({ url: reported, kind: "timeout", ...expired });
    // Fetch reports network errors (DNS, TLS, reset, truncated body) as TypeError.
    if (cause instanceof TypeError)
      return err({
@@ -88,14 +137,25 @@ export const downloadHopperPackage = async (
        detail: networkDetail(cause),
      });
    throw cause;
+  } finally {
+    clearTimeout(overall);
+    clearTimeout(stall);
  }
 };

 const readHopperResponse = async (
  requested: string,
  url: string,
-  signal: AbortSignal,
-  maxBytes: number,
+  {
+    signal,
+    maxBytes,
+    progressed,
+  }: {
+    readonly signal: AbortSignal;
+    readonly maxBytes: number;
+    /** Reports received body bytes so the stall bound restarts. */
+    readonly progressed: (receivedBytes: number) => void;
+  },
 ): Promise<HopperDownload> => {
  const fetched = await fetchHopperResponse(requested, signal);
  if (!fetched.ok)
@@ -117,6 +177,7 @@ const readHopperResponse = async (
    return err(tooLarge);
  }
  if (response.body === null) return ok(new Uint8Array());
+  progressed(0);
  const reader = response.body.getReader();
  const chunks: Uint8Array[] = [];
  let bytes = 0;
@@ -130,6 +191,7 @@ const readHopperResponse = async (
      }
      chunks.push(chunk.value);
      bytes += chunk.value.byteLength;
+      if (chunk.value.byteLength > 0) progressed(bytes);
    }
  } finally {
    reader.releaseLock();
@@ -228,7 +290,9 @@ export const describeHopperRequestFailure = (
        ? `${subject} received an unsuccessful HTTP response without a valid status.`
        : `${subject} returned HTTP ${failure.httpStatus}.`;
    case "timeout":
-      return `${subject} did not complete within ${failure.timeoutMs / 1000} seconds.`;
+      return failure.limit === "stalled"
+        ? `${subject} stalled: no bytes arrived for ${failure.timeoutMs / 1000} seconds after ${failure.receivedBytes} bytes were received.`
+        : `${subject} did not complete within ${failure.timeoutMs / 1000} seconds; ${failure.receivedBytes} bytes were received.`;
    case "network":
      return `${subject} failed with a network error (${failure.detail}).`;
    case "response_too_large":
@@ -35,6 +35,7 @@ import type { OwnedLease } from "../process/OwnedLease.js";
 import {
  downloadHopperPackage,
  HOPPER_DOWNLOAD_BYTES,
+  HOPPER_METADATA_TIMEOUT_MS,
 } from "./HopperDownload.js";
 import {
  HOPPER_LINUX_PRIMARY_FINGERPRINT,
@@ -52,8 +53,6 @@ import { withRegularFile } from "./RegularFileRead.js";

 /** Metadata/key inspection budget, independent of the compressed package budget. */
 const RELEASE_METADATA_BYTES = 1024 * 1024;
-/** Small feed/key requests must not consume the full package-download deadline. */
-const RELEASE_METADATA_TIMEOUT_MS = 10_000;
 const cacheRecordSchema = z.object({
  format: z.literal("rea-signed-hopper-release-v1"),
  family: z.enum(["deb", "rpm", "arch"]),
@@ -122,7 +121,7 @@ export class LinuxHopperPublisher {
        const response = await downloadHopperPackage(HOPPER_RELEASES_URL, {
          ...(signal === undefined ? {} : { signal }),
          maxBytes: RELEASE_METADATA_BYTES,
-          timeoutMs: RELEASE_METADATA_TIMEOUT_MS,
+          timeoutMs: HOPPER_METADATA_TIMEOUT_MS,
        });
        if (!response.ok)
          throw HopperReleaseError.fromDownload("release_metadata", {
@@ -186,7 +185,7 @@ export class LinuxHopperPublisher {
        const response = await downloadHopperPackage(HOPPER_PUBLIC_KEY_URL, {
          ...(signal === undefined ? {} : { signal }),
          maxBytes: RELEASE_METADATA_BYTES,
-          timeoutMs: RELEASE_METADATA_TIMEOUT_MS,
+          timeoutMs: HOPPER_METADATA_TIMEOUT_MS,
        });
        if (!response.ok)
          throw HopperReleaseError.fromDownload("verification_unavailable", {
@@ -3,9 +3,10 @@ import { createHash } from "node:crypto";
 import { describe, expect, it } from "vitest";

 import { err, ok } from "../domain/result.js";
-import type {
-  HopperDownload,
-  HopperDownloadFailure,
+import {
+  HOPPER_METADATA_TIMEOUT_MS,
+  type HopperDownload,
+  type HopperDownloadFailure,
 } from "./HopperDownload.js";
 import {
  installMacHopper,
@@ -35,8 +36,13 @@ class FakeMacHopperHost implements MacHopperInstallHost {
  installs = 0;
  opens = 0;
  replaces = false;
+  downloads: Array<{ url: string; timeoutMs: number | undefined }> = [];

-  download = (url: string): Promise<HopperDownload> => {
+  download = (
+    url: string,
+    options?: { readonly timeoutMs?: number },
+  ): Promise<HopperDownload> => {
+    this.downloads.push({ url, timeoutMs: options?.timeoutMs });
    const feed = url.includes("files-api");
    if (!feed) this.packageDownloads += 1;
    const failure = feed ? this.feedFailure : this.packageFailure;
@@ -138,6 +144,21 @@ describe("macOS Hopper installation", () => {
    expect(host.cleanups).toBe(1);
  });

+  it("bounds the release feed by the metadata limit and the package by the download bounds", async () => {
+    const host = new FakeMacHopperHost();
+    await installMacHopper({}, host);
+    expect(host.downloads).toEqual([
+      {
+        url: expect.stringContaining("request=releases"),
+        timeoutMs: HOPPER_METADATA_TIMEOUT_MS,
+      },
+      {
+        url: "https://www.hopperapp.com:443/downloader/public/Hopper-demo.dmg",
+        timeoutMs: undefined,
+      },
+    ]);
+  });
+
  it("preserves installation when macOS cannot open the installed app", async () => {
    const host = new FakeMacHopperHost();
    host.openApplication = () =>
@@ -531,7 +552,9 @@ it.each(["feed", "package"] as const)(
    const failure = {
      url: "https://fixture.invalid/request",
      kind: "timeout",
-      timeoutMs: 120_000,
+      limit: "stalled",
+      timeoutMs: 30_000,
+      receivedBytes: 0,
    } as const;
    if (stage === "feed") host.feedFailure = failure;
    else host.packageFailure = failure;
@@ -22,6 +22,7 @@ import { macDeveloperIdRequirement } from "../process/MacCodeSignature.js";
 import { macHopperUserApplicationPath } from "./MacHopperPaths.js";
 import {
  downloadHopperPackage,
+  HOPPER_METADATA_TIMEOUT_MS,
  type HopperDownload,
  type HopperRequestFailure,
 } from "./HopperDownload.js";
@@ -120,7 +121,10 @@ export type MacHopperInstallResult =

 /** External effects used by the bounded macOS Hopper installer. */
 export interface MacHopperInstallHost {
-  download(url: string): Promise<HopperDownload>;
+  download(
+    url: string,
+    options?: { readonly timeoutMs?: number },
+  ): Promise<HopperDownload>;
  createTemporaryDirectory(): Promise<string>;
  createMountDirectory(path: string): Promise<void>;
  writePackage(path: string, bytes: Uint8Array): Promise<void>;
@@ -169,7 +173,9 @@ export const installMacHopper = async (
  let result: MacHopperInstallResult;
  try {
    result = await (async (): Promise<MacHopperInstallResult> => {
-      const metadata = await host.download(HOPPER_RELEASES_URL);
+      const metadata = await host.download(HOPPER_RELEASES_URL, {
+        timeoutMs: HOPPER_METADATA_TIMEOUT_MS,
+      });
      if (!metadata.ok)
        return {
          status: "failed",
@@ -36,7 +36,10 @@ import {
  type LinuxHopperRelease,
 } from "../../../src/application/HopperReleaseFeed.js";
 import * as signatures from "../../../src/process/DetachedPgpSignature.js";
-import { downloadHopperPackage } from "../../../src/application/HopperDownload.js";
+import {
+  downloadHopperPackage,
+  HOPPER_METADATA_TIMEOUT_MS,
+} from "../../../src/application/HopperDownload.js";
 import { PrivateRuntimeRoot } from "../../../src/process/PrivateRuntimeRoot.js";
 import { OwnedFileHandle } from "../../../src/filesystem/OwnedFileHandle.js";
 import * as leases from "../../../src/process/HopperLease.js";
@@ -800,7 +803,9 @@ it("bounds a stalled metadata fetch and preserves caller cancellation", async ()
    error: {
      url: "https://www.hopperapp.com/fixture",
      kind: "timeout",
+      limit: "deadline",
      timeoutMs: 10,
+      receivedBytes: 0,
    },
  });
  const controller = new AbortController();
@@ -811,6 +816,42 @@ it("bounds a stalled metadata fetch and preserves caller cancellation", async ()
  controller.abort(reason);
  await expect(pending).rejects.toBe(reason);
 });
+it("names the release feed and its limit when the metadata fetch times out", async () => {
+  vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] });
+  try {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn(
+        (_url, options: RequestInit) =>
+          new Promise((_resolve, reject) => {
+            options.signal?.addEventListener(
+              "abort",
+              () => reject(options.signal?.reason),
+              { once: true },
+            );
+          }),
+      ),
+    );
+    const publisher = new LinuxHopperPublisher({}, async () => "deb");
+    const pending = publisher.currentRelease("deb");
+    await vi.advanceTimersByTimeAsync(HOPPER_METADATA_TIMEOUT_MS);
+    expect(await pending).toMatchObject({
+      ok: false,
+      error: {
+        reason: "release_metadata",
+        download: {
+          request: "release_feed",
+          kind: "timeout",
+          limit: "deadline",
+          timeoutMs: HOPPER_METADATA_TIMEOUT_MS,
+          receivedBytes: 0,
+        },
+      },
+    });
+  } finally {
+    vi.useRealTimers();
+  }
+});
 it("cancels an oversized chunked metadata response before retaining it", async () => {
  const cancelled = vi.fn();
  vi.stubGlobal(
@@ -0,0 +1,140 @@
+import { afterEach, beforeEach, expect, it, vi } from "vitest";
+
+import {
+  downloadHopperPackage,
+  HOPPER_DOWNLOAD_STALL_MS,
+  HOPPER_DOWNLOAD_TIMEOUT_MS,
+} from "../../src/application/HopperDownload.js";
+import { err, ok } from "../../src/domain/result.js";
+
+const url = "https://www.hopperapp.com/downloader/public/Hopper-Linux-demo.rpm";
+
+beforeEach(() => {
+  vi.useFakeTimers({ toFake: ["setTimeout", "clearTimeout"] });
+});
+afterEach(() => {
+  vi.useRealTimers();
+  vi.unstubAllGlobals();
+});
+
+// Like Fetch, error the body with the request signal's reason on abort.
+const stubPackage = (chunkEveryMs: number, chunks: number) => {
+  const fetch = vi.fn<typeof globalThis.fetch>(async (_input, options) => {
+    const signal = options?.signal ?? undefined;
+    let sent = 0;
+    return new Response(
+      new ReadableStream<Uint8Array>({
+        start(controller) {
+          signal?.addEventListener(
+            "abort",
+            () => {
+              controller.error(signal.reason);
+            },
+            { once: true },
+          );
+        },
+        pull: (controller) =>
+          new Promise<void>((resolve) => {
+            if (sent === chunks) {
+              controller.close();
+              resolve();
+              return;
+            }
+            setTimeout(() => {
+              if (signal?.aborted !== true) {
+                sent += 1;
+                controller.enqueue(new Uint8Array(1024));
+              }
+              resolve();
+            }, chunkEveryMs);
+          }),
+      }),
+    );
+  });
+  vi.stubGlobal("fetch", fetch);
+  return fetch;
+};
+
+it("completes a slow download that keeps delivering bytes past the former 120 s deadline", async () => {
+  stubPackage(20_000, 9);
+  const pending = downloadHopperPackage(url);
+  await vi.advanceTimersByTimeAsync(9 * 20_000);
+  expect(await pending).toEqual(ok(Buffer.alloc(9 * 1024)));
+});
+
+it("fails a download whose next bytes stall, naming the URL and the bytes received", async () => {
+  const fetch = vi.fn<typeof globalThis.fetch>(async (_input, options) => {
+    const signal = options?.signal ?? undefined;
+    return new Response(
+      new ReadableStream<Uint8Array>({
+        start(controller) {
+          controller.enqueue(new Uint8Array(1024));
+          signal?.addEventListener(
+            "abort",
+            () => {
+              controller.error(signal.reason);
+            },
+            { once: true },
+          );
+        },
+      }),
+    );
+  });
+  vi.stubGlobal("fetch", fetch);
+  const pending = downloadHopperPackage(url);
+  await vi.advanceTimersByTimeAsync(HOPPER_DOWNLOAD_STALL_MS);
+  expect(await pending).toEqual(
+    err({
+      url,
+      kind: "timeout",
+      limit: "stalled",
+      timeoutMs: HOPPER_DOWNLOAD_STALL_MS,
+      receivedBytes: 1024,
+    }),
+  );
+  expect(fetch.mock.calls[0]?.[1]?.signal?.aborted).toBe(true);
+});
+
+it("treats a response that never starts as stalled", async () => {
+  vi.stubGlobal(
+    "fetch",
+    vi.fn<typeof globalThis.fetch>(
+      (_input, options) =>
+        new Promise((_resolve, reject) => {
+          options?.signal?.addEventListener(
+            "abort",
+            () => {
+              reject(options.signal?.reason);
+            },
+            { once: true },
+          );
+        }),
+    ),
+  );
+  const pending = downloadHopperPackage(url);
+  await vi.advanceTimersByTimeAsync(HOPPER_DOWNLOAD_STALL_MS);
+  expect(await pending).toEqual(
+    err({
+      url,
+      kind: "timeout",
+      limit: "stalled",
+      timeoutMs: HOPPER_DOWNLOAD_STALL_MS,
+      receivedBytes: 0,
+    }),
+  );
+});
+
+it("bounds a download that keeps trickling within the stall bound", async () => {
+  stubPackage(20_000, Number.POSITIVE_INFINITY);
+  const pending = downloadHopperPackage(url);
+  await vi.advanceTimersByTimeAsync(HOPPER_DOWNLOAD_TIMEOUT_MS);
+  expect(await pending).toEqual(
+    err({
+      url,
+      kind: "timeout",
+      limit: "deadline",
+      timeoutMs: HOPPER_DOWNLOAD_TIMEOUT_MS,
+      receivedBytes: 44 * 1024,
+    }),
+  );
+});
@@ -385,7 +385,13 @@ it("chooses recovery by failure class: missing file, temporary service failure,
    expect(status(transient)).toBe(status(503));
  expect(status(undefined)).toBe(status(403));
  expect(
-    hopperRequestRecovery({ url: debUrl, kind: "timeout", timeoutMs: 10_000 }),
+    hopperRequestRecovery({
+      url: debUrl,
+      kind: "timeout",
+      limit: "stalled",
+      timeoutMs: 30_000,
+      receivedBytes: 0,
+    }),
  ).toBe(unreachable);
  expect(
    new Set([status(404), status(503), status(403), unreachable]).size,