From cba8163286c1762e91d2208d0b0e72d4445a0779 Mon Sep 17 00:00:00 2001 From: yusufipk Date: Sat, 22 Aug 2026 12:50:45 +0300 Subject: [PATCH] fix(download): warn before the tab closes mid-download Bunny and direct downloads are pulled through fetch() so we can save them under our own filename. The browser does not treat that as a download, so closing the tab discarded everything received so far without a word. Register a reference counted beforeunload guard while those transfers are in flight, and while a project manifest is being pulled file by file. Browser owned downloads (same-origin proxy, the over-10GB fallback, asset downloads) survive a tab close on their own and stay unguarded. --- .../[projectId]/project-content-client.tsx | 6 ++ .../video-page/hooks/use-download-actions.ts | 6 ++ lib/client/unload-guard.ts | 48 ++++++++++++ .../hooks/use-download-actions.test.ts | 66 +++++++++++++++++ tests/component/unload-guard.test.ts | 74 +++++++++++++++++++ 5 files changed, 200 insertions(+) create mode 100644 lib/client/unload-guard.ts create mode 100644 tests/component/unload-guard.test.ts diff --git a/app/(dashboard)/projects/[projectId]/project-content-client.tsx b/app/(dashboard)/projects/[projectId]/project-content-client.tsx index 381d406..64d3914 100644 --- a/app/(dashboard)/projects/[projectId]/project-content-client.tsx +++ b/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); } }, diff --git a/components/video-page/hooks/use-download-actions.ts b/components/video-page/hooks/use-download-actions.ts index fa24364..caf2fb8 100644 --- a/components/video-page/hooks/use-download-actions.ts +++ b/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); } diff --git a/lib/client/unload-guard.ts b/lib/client/unload-guard.ts new file mode 100644 index 0000000..873b5a4 --- /dev/null +++ b/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; +} diff --git a/tests/component/hooks/use-download-actions.test.ts b/tests/component/hooks/use-download-actions.test.ts index 1b37943..4c369f7 100644 --- a/tests/component/hooks/use-download-actions.test.ts +++ b/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(); + 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 | 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); + }); +}); diff --git a/tests/component/unload-guard.test.ts b/tests/component/unload-guard.test.ts new file mode 100644 index 0000000..a862365 --- /dev/null +++ b/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; +let removeSpy: MockInstance; + +/** 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): 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); + }); +});