Merge pull request #69 from yusufipk/fix/download-unload-guard

fix(download): warn before the tab closes mid-download

app/(dashboard)/projects/[projectId]/project-content-client.tsx

@@ -52,6 +52,7 @@ import {
   type ProjectDownloadManifest,
 } from '@/lib/client/project-download';
 import { downloadProgressPercent } from '@/lib/client/download-file';
+import { beginUnloadGuard } from '@/lib/client/unload-guard';
 import {
   createDownloadProgressToast,
   type DownloadProgressToastHandle,
@@ -209,6 +210,7 @@ export function ProjectContentClient({
 
       setIsDownloading(true);
       let progressToast: DownloadProgressToastHandle | null = null;
+      let releaseUnloadGuard: (() => void) | null = null;
       try {
         const response = await fetch(`/api/projects/${projectId}/download${query}`, {
           cache: 'no-store',
@@ -232,6 +234,9 @@ export function ProjectContentClient({
           title: `Downloading ${manifest.totalFiles} files`,
           description: 'Starting…',
         });
+        // The files are pulled one by one through this tab, so closing it drops
+        // everything that hasn't been saved yet. Warn before that happens.
+        releaseUnloadGuard = beginUnloadGuard();
         await runProjectDownloadManifest(manifest, (p) => {
           const percent = downloadProgressPercent({
             receivedBytes: p.receivedBytes,
@@ -250,6 +255,7 @@ export function ProjectContentClient({
         progressToast?.dismiss();
         toast.error('Failed to start project download');
       } finally {
+        releaseUnloadGuard?.();
         setIsDownloading(false);
       }
     },

components/video-page/hooks/use-download-actions.ts

@@ -22,6 +22,7 @@ import {
   createDownloadProgressToast,
   type DownloadProgressToastHandle,
 } from '@/components/download-progress-toast';
+import { beginUnloadGuard } from '@/lib/client/unload-guard';
 
 function sanitizeDownloadFileName(value: string): string {
   return value
@@ -94,6 +95,7 @@ export function useDownloadActions({ activeVersion, video }: UseDownloadActionsP
       isDownloadingRef.current = true;
       setActiveDownloadTarget(target);
       let progressToast: DownloadProgressToastHandle | null = null;
+      let releaseUnloadGuard: (() => void) | null = null;
       try {
         let downloadUrl: string | null = null;
 
@@ -167,6 +169,9 @@ export function useDownloadActions({ activeVersion, video }: UseDownloadActionsP
             title: `Downloading “${baseName}”`,
             description: 'Starting…',
           });
+          // The bytes only exist in this tab until the blob is saved, so warn
+          // before the page goes away instead of losing the whole transfer.
+          releaseUnloadGuard = beginUnloadGuard();
           const saved = await downloadNamedFile(downloadUrl, `${baseName}.${fallbackExt}`, (p) => {
             progressToast?.update({
               description: downloadProgressLabel(p),
@@ -195,6 +200,7 @@ export function useDownloadActions({ activeVersion, video }: UseDownloadActionsP
           toast.error('Failed to start download');
         }
       } finally {
+        releaseUnloadGuard?.();
         isDownloadingRef.current = false;
         setActiveDownloadTarget(null);
       }

lib/client/unload-guard.ts

@@ -0,0 +1,48 @@
+'use client';
+
+/**
+ * Downloads that we pull through fetch() live inside the page: closing the tab
+ * (or reloading) throws away every byte received so far and the browser gives no
+ * warning, because as far as it knows nothing is downloading. While one of those
+ * is in flight we register a beforeunload handler so the user gets the native
+ * "leave site?" dialog instead of silently losing the transfer.
+ *
+ * Plain navigation downloads (the `download` attribute / a redirect to the CDN)
+ * are owned by the browser and survive a tab close, so they must NOT be guarded.
+ */
+
+let activeCount = 0;
+
+function handleBeforeUnload(event: BeforeUnloadEvent) {
+  event.preventDefault();
+  // Legacy browsers only show the dialog when returnValue is set; the string
+  // itself is ignored, every browser shows its own wording.
+  event.returnValue = '';
+}
+
+/** Registers the guard and returns a release function. Safe to call again while
+ * another download is already guarded — the listener is reference counted and
+ * only detaches once the last one releases. Releasing twice is a no-op. */
+export function beginUnloadGuard(): () => void {
+  if (typeof window === 'undefined') return () => {};
+
+  if (activeCount === 0) {
+    window.addEventListener('beforeunload', handleBeforeUnload);
+  }
+  activeCount += 1;
+
+  let released = false;
+  return () => {
+    if (released) return;
+    released = true;
+    activeCount -= 1;
+    if (activeCount === 0) {
+      window.removeEventListener('beforeunload', handleBeforeUnload);
+    }
+  };
+}
+
+/** Test helper: number of downloads currently holding the guard. */
+export function unloadGuardCount(): number {
+  return activeCount;
+}

tests/component/hooks/use-download-actions.test.ts

@@ -652,3 +652,69 @@ describe('useDownloadActions repeated clicks', () => {
     expect(clicked).toHaveLength(1);
   });
 });
+
+describe('useDownloadActions guarding the tab', () => {
+  function fireBeforeUnload(): BeforeUnloadEvent {
+    const event = new Event('beforeunload', { cancelable: true }) as BeforeUnloadEvent;
+    window.dispatchEvent(event);
+    return event;
+  }
+
+  // Closing the tab used to throw away a half-pulled file without a word,
+  // because the browser has no idea a fetch-driven download is running.
+  it('warns before the tab closes while the bytes are being pulled', async () => {
+    const pending = deferred<unknown>();
+    const harness = renderDownload();
+    fetchMock.mockImplementation((url: string) => {
+      if (typeof url === 'string' && url.includes('prepare=1')) {
+        return Promise.resolve(prepareResponse(true, { data: {} }));
+      }
+      return pending.promise;
+    });
+
+    let started: Promise<void> | undefined;
+    await act(async () => {
+      started = harness.result.current.startDownload();
+      // Let the prepare call settle so the byte fetch is the pending one.
+      await Promise.resolve();
+    });
+
+    expect(fireBeforeUnload().defaultPrevented).toBe(true);
+
+    await act(async () => {
+      pending.resolve(fileResponse());
+      await started;
+    });
+
+    expect(fireBeforeUnload().defaultPrevented).toBe(false);
+  });
+
+  it('releases the guard when the download fails', async () => {
+    downloadResponse = fileResponse({ ok: false });
+    const harness = renderDownload();
+
+    await act(async () => {
+      await harness.result.current.startDownload();
+    });
+
+    expect(fireBeforeUnload().defaultPrevented).toBe(false);
+  });
+
+  // A same-origin proxy download is handed to the browser, which keeps going
+  // after the tab closes, so nothing should block the unload there.
+  it('does not warn for a browser-owned download', async () => {
+    const harness = renderDownload({
+      activeVersion: makeVersion({
+        providerId: 'r2',
+        originalUrl: '/api/upload/video/abc.mp4',
+      }),
+    });
+
+    await act(async () => {
+      await harness.result.current.startDownload();
+    });
+
+    expect(clicked).toHaveLength(1);
+    expect(fireBeforeUnload().defaultPrevented).toBe(false);
+  });
+});

tests/component/unload-guard.test.ts

@@ -0,0 +1,74 @@
+import { describe, it, expect, beforeEach, afterEach, vi, type MockInstance } from 'vitest';
+import { beginUnloadGuard, unloadGuardCount } from '@/lib/client/unload-guard';
+
+let addSpy: MockInstance<typeof window.addEventListener>;
+let removeSpy: MockInstance<typeof window.removeEventListener>;
+
+/** True when something cancelled the unload, which is what makes the browser
+ * show its "leave site?" dialog. */
+function unloadWasBlocked(): boolean {
+  const event = new Event('beforeunload', { cancelable: true });
+  window.dispatchEvent(event);
+  return event.defaultPrevented;
+}
+
+function listenerCalls(spy: MockInstance<typeof window.addEventListener>): number {
+  return spy.mock.calls.filter(([type]) => type === 'beforeunload').length;
+}
+
+beforeEach(() => {
+  addSpy = vi.spyOn(window, 'addEventListener');
+  removeSpy = vi.spyOn(window, 'removeEventListener');
+});
+
+afterEach(() => {
+  vi.restoreAllMocks();
+  // A leaked guard would block the unload for the rest of the session, so a
+  // test that leaves one behind must fail here rather than in the next test.
+  expect(unloadGuardCount()).toBe(0);
+});
+
+describe('beginUnloadGuard', () => {
+  it('cancels the unload while a download holds it', () => {
+    const release = beginUnloadGuard();
+    const blocked = unloadWasBlocked();
+    release();
+
+    expect(blocked).toBe(true);
+  });
+
+  it('lets the page go once the download is released', () => {
+    beginUnloadGuard()();
+
+    expect(unloadWasBlocked()).toBe(false);
+  });
+
+  it('keeps the listener until the last concurrent download releases', () => {
+    const releaseA = beginUnloadGuard();
+    const releaseB = beginUnloadGuard();
+    expect(listenerCalls(addSpy)).toBe(1);
+
+    releaseA();
+    const stillBlocked = unloadWasBlocked();
+    releaseB();
+
+    expect(stillBlocked).toBe(true);
+    expect(listenerCalls(removeSpy)).toBe(1);
+    expect(unloadWasBlocked()).toBe(false);
+  });
+
+  // The download hook releases from a finally block, and a caller could hold
+  // the returned function longer; a double release must not drop a guard
+  // another download still holds.
+  it('ignores a second release', () => {
+    const releaseA = beginUnloadGuard();
+    const releaseB = beginUnloadGuard();
+
+    releaseA();
+    releaseA();
+    const stillBlocked = unloadWasBlocked();
+    releaseB();
+
+    expect(stillBlocked).toBe(true);
+  });
+});