feat: adapt macos download hold to file size and connection speed (#413)

* feat: adapt macos download hold to file size and connection speed

* fix: address review — abort-aware sleep, lower max hold, inline helpers, more tests

Author: juanmahidalgo

src/modules/streamOrFallback.test.ts

@@ -1,5 +1,11 @@
 import { OperativeSystem } from '../types/download.types'
-import { FALLBACK_LOADER_HOLD_MS, streamOrFallback } from './streamOrFallback'
+import {
+  ESTIMATE_MAX_HOLD_MS,
+  ESTIMATE_MIN_HOLD_MS,
+  FALLBACK_LOADER_HOLD_MS,
+  estimateDownloadHoldMs,
+  streamOrFallback
+} from './streamOrFallback'
 
 const mockTriggerFileDownload = jest.fn()
 const mockDownloadFileWithProgress = jest.fn()
@@ -9,20 +15,136 @@ jest.mock('./file', () => ({
   downloadFileWithProgress: (...args: unknown[]) => mockDownloadFileWithProgress(...args)
 }))
 
+const buildHeadResponse = (contentLength: string | null, ok = true): Response =>
+  ({
+    ok,
+    headers: { get: (name: string) => (name.toLowerCase() === 'content-length' ? contentLength : null) }
+  }) as unknown as Response
+
+const setNavigatorConnection = (downlink: number | undefined): void => {
+  Object.defineProperty(window.navigator, 'connection', {
+    configurable: true,
+    value: downlink !== undefined ? { downlink } : undefined
+  })
+}
+
+const clearNavigatorConnection = (): void => {
+  Object.defineProperty(window.navigator, 'connection', { configurable: true, value: undefined })
+}
+
+describe('estimateDownloadHoldMs', () => {
+  let mockFetch: jest.Mock
+
+  beforeEach(() => {
+    mockFetch = jest.fn()
+    global.fetch = mockFetch as unknown as typeof fetch
+  })
+
+  afterEach(() => {
+    jest.resetAllMocks()
+    clearNavigatorConnection()
+  })
+
+  describe('when the HEAD request returns a Content-Length and the browser exposes downlink', () => {
+    beforeEach(() => {
+      // 80 MB in bytes — the macOS DMG ballpark.
+      mockFetch.mockResolvedValue(buildHeadResponse(String(80 * 1024 * 1024)))
+    })
+
+    it('should compute a hold proportional to size / downlink with the safety margin applied', async () => {
+      // 100 Mbps → 80MB ≈ 6.4s pure transfer + safety margin (~1.5s) → ~7.9s.
+      setNavigatorConnection(100)
+      const ms = await estimateDownloadHoldMs('https://example.com/file.dmg')
+      expect(ms).toBeGreaterThanOrEqual(7000)
+      expect(ms).toBeLessThanOrEqual(9000)
+    })
+
+    it('should produce a longer hold for a slower connection at the same size', async () => {
+      // 5 Mbps → 80MB ≈ 128s pure transfer, way over ESTIMATE_MAX_HOLD_MS.
+      setNavigatorConnection(5)
+      const ms = await estimateDownloadHoldMs('https://example.com/file.dmg')
+      expect(ms).toBe(ESTIMATE_MAX_HOLD_MS)
+    })
+  })
+
+  describe('when the HEAD request fails', () => {
+    it('should return the minimum hold instead of guessing', async () => {
+      mockFetch.mockRejectedValue(new TypeError('CORS blocked'))
+      setNavigatorConnection(50)
+
+      const ms = await estimateDownloadHoldMs('https://example.com/file.dmg')
+
+      expect(ms).toBe(ESTIMATE_MIN_HOLD_MS)
+    })
+  })
+
+  describe('when the HEAD response is non-2xx (403, 405, etc.)', () => {
+    it('should treat it like a missing Content-Length and return the minimum hold', async () => {
+      mockFetch.mockResolvedValue(buildHeadResponse(String(80 * 1024 * 1024), false))
+      setNavigatorConnection(50)
+
+      const ms = await estimateDownloadHoldMs('https://example.com/file.dmg')
+
+      expect(ms).toBe(ESTIMATE_MIN_HOLD_MS)
+    })
+  })
+
+  describe('when the HEAD response omits Content-Length', () => {
+    it('should return the minimum hold', async () => {
+      mockFetch.mockResolvedValue(buildHeadResponse(null))
+      setNavigatorConnection(50)
+
+      const ms = await estimateDownloadHoldMs('https://example.com/file.dmg')
+
+      expect(ms).toBe(ESTIMATE_MIN_HOLD_MS)
+    })
+  })
+
+  describe('when the browser does not expose navigator.connection (Safari, Firefox)', () => {
+    it('should fall back to the default downlink and still produce an adaptive hold', async () => {
+      // 10 MB at the default 25 Mbps ≈ 3.2s pure transfer + safety margin.
+      // Sized to land between MIN and MAX so the assertion exercises the
+      // adaptive arithmetic rather than the clamps.
+      mockFetch.mockResolvedValue(buildHeadResponse(String(10 * 1024 * 1024)))
+      clearNavigatorConnection()
+
+      const ms = await estimateDownloadHoldMs('https://example.com/file.dmg')
+
+      expect(ms).toBeGreaterThan(ESTIMATE_MIN_HOLD_MS)
+      expect(ms).toBeLessThan(ESTIMATE_MAX_HOLD_MS)
+    })
+  })
+
+  describe('when the file is tiny', () => {
+    it('should clamp to the minimum hold so the modal does not vanish instantly', async () => {
+      mockFetch.mockResolvedValue(buildHeadResponse('512')) // 512 bytes
+      setNavigatorConnection(100)
+
+      const ms = await estimateDownloadHoldMs('https://example.com/file.dmg')
+
+      expect(ms).toBe(ESTIMATE_MIN_HOLD_MS)
+    })
+  })
+})
+
 describe('streamOrFallback', () => {
   let abortController: AbortController
   let onProgress: jest.Mock
+  let mockFetch: jest.Mock
 
   beforeEach(() => {
-    jest.useFakeTimers()
+    jest.useFakeTimers({ doNotFake: ['performance', 'Date'] })
     abortController = new AbortController()
     onProgress = jest.fn()
+    mockFetch = jest.fn().mockResolvedValue(buildHeadResponse(null))
+    global.fetch = mockFetch as unknown as typeof fetch
   })
 
   afterEach(() => {
     jest.runOnlyPendingTimers()
     jest.useRealTimers()
     jest.resetAllMocks()
+    clearNavigatorConnection()
   })
 
   describe('when the OS is macOS', () => {
@@ -35,14 +157,32 @@ describe('streamOrFallback', () => {
         onProgress
       })
 
-      jest.advanceTimersByTime(FALLBACK_LOADER_HOLD_MS)
+      // Drain the HEAD round-trip and the timer.
+      await jest.advanceTimersByTimeAsync(ESTIMATE_MAX_HOLD_MS)
       await promise
 
       expect(mockTriggerFileDownload).toHaveBeenCalledWith('https://example.com/file.dmg', 'Decentraland-Installer.dmg')
       expect(mockDownloadFileWithProgress).not.toHaveBeenCalled()
     })
 
-    it('should hold the backdrop for FALLBACK_LOADER_HOLD_MS before resolving', async () => {
+    it('should HEAD the URL to get Content-Length for the adaptive estimate', async () => {
+      const promise = streamOrFallback({
+        url: 'https://example.com/file.dmg',
+        filename: 'Decentraland-Installer.dmg',
+        os: OperativeSystem.MACOS,
+        signal: abortController.signal,
+        onProgress
+      })
+
+      await jest.advanceTimersByTimeAsync(ESTIMATE_MAX_HOLD_MS)
+      await promise
+
+      expect(mockFetch).toHaveBeenCalledWith('https://example.com/file.dmg', expect.objectContaining({ method: 'HEAD' }))
+    })
+
+    it('should hold for at least ESTIMATE_MIN_HOLD_MS when HEAD provides no size signal', async () => {
+      mockFetch.mockResolvedValue(buildHeadResponse(null))
+
       let resolved = false
       streamOrFallback({
         url: 'https://example.com/file.dmg',
@@ -54,12 +194,41 @@ describe('streamOrFallback', () => {
         resolved = true
       })
 
-      // Drain the microtask that schedules the timer, then advance just
-      // below the threshold — the hold should still be pending.
-      await jest.advanceTimersByTimeAsync(FALLBACK_LOADER_HOLD_MS - 1)
+      await jest.advanceTimersByTimeAsync(ESTIMATE_MIN_HOLD_MS - 1)
       expect(resolved).toBe(false)
 
-      await jest.advanceTimersByTimeAsync(1)
+      await jest.advanceTimersByTimeAsync(2)
+      expect(resolved).toBe(true)
+    })
+
+    it('should resolve promptly when the signal aborts mid-hold', async () => {
+      // Long estimated hold so the test exercises a real wait window:
+      // 80 MB at 5 Mbps caps at MAX_HOLD_MS.
+      mockFetch.mockResolvedValue(buildHeadResponse(String(80 * 1024 * 1024)))
+      setNavigatorConnection(5)
+
+      let resolved = false
+      streamOrFallback({
+        url: 'https://example.com/file.dmg',
+        filename: 'Decentraland-Installer.dmg',
+        os: OperativeSystem.MACOS,
+        signal: abortController.signal,
+        onProgress
+      }).then(() => {
+        resolved = true
+      })
+
+      // Let the HEAD resolve and the sleep get scheduled, then verify we are
+      // mid-hold (not yet resolved).
+      await jest.advanceTimersByTimeAsync(ESTIMATE_MIN_HOLD_MS)
+      expect(resolved).toBe(false)
+
+      // Abort while still inside the hold.
+      abortController.abort()
+      await jest.advanceTimersByTimeAsync(0)
+
+      // Should resolve now without needing to advance through the rest of
+      // the long hold.
       expect(resolved).toBe(true)
     })
   })
@@ -158,10 +327,7 @@ describe('streamOrFallback', () => {
           onProgress
         })
 
-        // Drain the rejected fetch promise, then advance through the hold.
-        await Promise.resolve()
-        await Promise.resolve()
-        jest.advanceTimersByTime(FALLBACK_LOADER_HOLD_MS)
+        await jest.advanceTimersByTimeAsync(FALLBACK_LOADER_HOLD_MS)
         await promise
 
         expect(mockTriggerFileDownload).toHaveBeenCalledWith('https://example.com/file.exe', 'Decentraland-Installer.exe')

src/modules/streamOrFallback.ts

@@ -2,14 +2,104 @@ import { OperativeSystem } from '../types/download.types'
 import { downloadFileWithProgress, triggerFileDownload } from './file'
 import type { StreamOrFallbackArgs } from './streamOrFallback.types'
 
-// Hold time when we can't observe the actual transfer — used on macOS (where
-// the native anchor is mandatory to preserve the kMDItemWhereFroms xattr the
-// launcher reads) and as the fetch-error fallback on Windows. The gateway's
-// per-request NSIS+sign step on Windows typically dominates and can run 5-30s,
-// so this is a coarse approximation rather than a real progress signal.
+// Fallback hold for the Windows fetch-error path. The HEAD that powers the
+// macOS adaptive estimate would likely fail for the same reason the streamed
+// fetch did (CORS, network), so we hardcode a coarse approximation here
+// instead of trying twice. The native anchor takes over the actual save and
+// the browser's download manager remains visible to the user as the real
+// progress signal.
 const FALLBACK_LOADER_HOLD_MS = 4000
 
-const sleep = (ms: number): Promise<void> => new Promise(resolve => setTimeout(resolve, ms))
+// Bandwidth assumption when the browser doesn't expose `navigator.connection`
+// (Safari, Firefox). Conservative middle of "typical broadband" so fast users
+// see the modal close a bit late and slow users see it close a bit early —
+// the alternative (no signal at all) is worse than an imperfect estimate.
+const DEFAULT_DOWNLINK_MBPS = 25
+
+// Wall-clock margin added on top of the pure transfer estimate to absorb
+// connection setup, HTTP/TLS handshake, server-side processing, and the
+// browser's write-to-disk after the bytes arrive. Picked empirically from
+// typical Mac DMG downloads.
+const ESTIMATE_SAFETY_MARGIN_MS = 1500
+
+// Always hold for at least this long so the modal doesn't blink-and-vanish
+// on cache hits / instant downloads.
+const ESTIMATE_MIN_HOLD_MS = 2000
+
+// Never hang a static (no progress bar) modal beyond this. Past this point
+// the browser's own download manager already shows as the user-facing
+// progress signal — keeping our modal up longer would just look stuck.
+const ESTIMATE_MAX_HOLD_MS = 20000
+
+interface NavigatorConnectionLike {
+  downlink?: number
+}
+
+/**
+ * Sleeps for `ms` milliseconds, resolving early if `signal` aborts. Resolves
+ * (rather than rejecting) on abort so callers can decide whether the caller
+ * was already past the point of caring — e.g. the download has already been
+ * dispatched to the OS, so an aborted hold just means "stop waiting and go
+ * tear down the UI".
+ */
+const sleep = (ms: number, signal?: AbortSignal): Promise<void> =>
+  new Promise(resolve => {
+    if (signal?.aborted) return resolve()
+    const id = setTimeout(resolve, ms)
+    signal?.addEventListener(
+      'abort',
+      () => {
+        clearTimeout(id)
+        resolve()
+      },
+      { once: true }
+    )
+  })
+
+const fetchContentLength = async (url: string, signal?: AbortSignal): Promise<number | null> => {
+  try {
+    const response = await fetch(url, {
+      method: 'HEAD',
+      mode: 'cors',
+      credentials: 'omit',
+      cache: 'no-store',
+      signal
+    })
+    if (!response.ok) return null
+    const header = response.headers.get('content-length')
+    if (!header) return null
+    const size = parseInt(header, 10)
+    return Number.isFinite(size) && size > 0 ? size : null
+  } catch {
+    return null
+  }
+}
+
+/**
+ * Estimates how long the modal should stay open given the file size and
+ * the browser's reported connection speed. Adaptive per-user instead of a
+ * single hardcoded constant. Returns the clamped hold in milliseconds.
+ *
+ * - Reads `Content-Length` from a HEAD request to the gateway.
+ * - Reads `navigator.connection.downlink` (Mbps) when available; falls back
+ *   to a conservative default on Safari and Firefox where the API doesn't
+ *   exist.
+ * - Adds a margin for handshake / write-to-disk variance and clamps the
+ *   result so the modal can't disappear instantly nor hang forever.
+ */
+const estimateDownloadHoldMs = async (url: string, signal?: AbortSignal): Promise<number> => {
+  const sizeBytes = await fetchContentLength(url, signal)
+  if (sizeBytes === null) return ESTIMATE_MIN_HOLD_MS
+
+  const connection =
+    typeof navigator !== 'undefined' ? (navigator as Navigator & { connection?: NavigatorConnectionLike }).connection : undefined
+  const downlinkMbps = connection?.downlink && connection.downlink > 0 ? connection.downlink : DEFAULT_DOWNLINK_MBPS
+
+  const downlinkBytesPerSec = (downlinkMbps * 1024 * 1024) / 8
+  const transferMs = (sizeBytes / downlinkBytesPerSec) * 1000
+  const total = transferMs + ESTIMATE_SAFETY_MARGIN_MS
+  return Math.min(ESTIMATE_MAX_HOLD_MS, Math.max(ESTIMATE_MIN_HOLD_MS, total))
+}
 
 /**
  * Triggers the installer download and resolves only when the file has
@@ -21,21 +111,24 @@ const sleep = (ms: number): Promise<void> => new Promise(resolve => setTimeout(r
  *    into the NSIS-wrapped EXE bytes (the wrapper writes
  *    campaign-anon-user-id.txt during install) — a blob: URL changing the
  *    Zone.Identifier doesn't affect the launcher's UUID recovery.
- *  - **macOS**: native `<a>.click()` only. The launcher reads
- *    `anon_user_id` from the DMG's `kMDItemWhereFroms` xattr that the OS
- *    writes from the original download URL; a blob: URL would replace it
- *    with the page's blob origin and break attribution. We fall back to a
- *    fixed-duration backdrop hold instead.
+ *  - **macOS**: native `<a>.click()` (the launcher reads `anon_user_id`
+ *    from the DMG's `kMDItemWhereFroms` xattr and a blob: URL would replace
+ *    it with the page's blob origin) plus an adaptive hold based on
+ *    `Content-Length` from a HEAD request and `navigator.connection.downlink`.
  *
  * If the streamed fetch fails on Windows (CORS not configured for the
  * current origin — true on Vercel preview deploys, where the gateway only
- * allows known prod/staging hosts), fall back to the native anchor + hold
- * so the download still happens.
+ * allows known prod/staging hosts), fall back to the native anchor + a
+ * fixed hold so the download still happens.
  */
 const streamOrFallback = async ({ url, filename, os, signal, onProgress }: StreamOrFallbackArgs): Promise<void> => {
   if (os === OperativeSystem.MACOS) {
+    const startedAt = Date.now()
     triggerFileDownload(url, filename)
-    await sleep(FALLBACK_LOADER_HOLD_MS)
+    const holdMs = await estimateDownloadHoldMs(url, signal)
+    if (signal.aborted) return
+    const remaining = Math.max(0, holdMs - (Date.now() - startedAt))
+    await sleep(remaining, signal)
     return
   }
 
@@ -58,8 +151,8 @@ const streamOrFallback = async ({ url, filename, os, signal, onProgress }: Strea
       name: error instanceof Error ? error.name : 'unknown'
     })
     triggerFileDownload(url, filename)
-    await sleep(FALLBACK_LOADER_HOLD_MS)
+    await sleep(FALLBACK_LOADER_HOLD_MS, signal)
   }
 }
 
-export { FALLBACK_LOADER_HOLD_MS, streamOrFallback }
+export { ESTIMATE_MAX_HOLD_MS, ESTIMATE_MIN_HOLD_MS, FALLBACK_LOADER_HOLD_MS, estimateDownloadHoldMs, streamOrFallback }