Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- **Pinned `starlette` and `botocore` so self-hosted Docker builds are reproducible** — both were floating transitives, so rebuilding the same commit on a different day could silently install different versions with no diff and no PR. FastAPI declares `starlette>=0.46.0` with no upper bound, meaning builds were free to cross a Starlette major (the ASGI layer under the SSE endpoint and the middleware stack); `botocore` is where S3 request signing lives, and unreviewed moves there have broken S3 compatibility before. Both are pinned to the versions already resolving, so no installed version changes.

### Fixed
- **A single transient storage error no longer discards an entire multipart upload** — each part is now retried up to 8 times with exponential backoff and jitter (≈254s of tolerance per part) instead of failing the whole upload on the first non-OK response. A multi-gigabyte file is several hundred sequential requests, so one 500/503 from the object store or a brief network drop was enough to throw away everything already transferred; on a slow uplink that can be an hour of work. The presigned URL is re-fetched on every attempt, since `presign_upload_part` expires after an hour and a large upload can outlive that. User-initiated cancels are never retried, and a 4xx is thrown straight through instead of being repeated eight times over four minutes — it would fail identically every time, so retrying it only delays the error. The part loop, previously duplicated between new-asset and new-version uploads, is now shared.

## [1.7.5] - 2026-07-23

### Fixed
Expand Down
143 changes: 143 additions & 0 deletions apps/web/stores/__tests__/upload-store.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'

vi.mock('@/lib/api', () => ({
api: {
post: vi.fn(),
get: vi.fn(),
},
}))

import { api } from '@/lib/api'
import { uploadAllParts } from '../upload-store'

const CHUNK_SIZE = 10 * 1024 * 1024

/** Builds a File of `bytes` length; 15 MB spans two 10 MB parts. */
function makeFile(bytes: number): File {
return new File([new Uint8Array(bytes)], 'clip.mp4', { type: 'video/mp4' })
}

function okResponse(etag: string) {
return { ok: true, headers: { get: () => etag } }
}

function failResponse(status = 503, statusText = 'Service Unavailable') {
return { ok: false, status, statusText, headers: { get: () => null } }
}

describe('uploadAllParts', () => {
let controller: AbortController

beforeEach(() => {
vi.clearAllMocks()
vi.useFakeTimers()
controller = new AbortController()
// Every attempt fetches its own presigned URL.
vi.mocked(api.post).mockResolvedValue({ presigned_url: 'https://s3.example/part' })
})

afterEach(() => {
vi.useRealTimers()
})

it('uploads each part in order and reports the collected ETags', async () => {
global.fetch = vi.fn()
.mockResolvedValueOnce(okResponse('"etag-1"'))
.mockResolvedValueOnce(okResponse('"etag-2"')) as unknown as typeof fetch

const onProgress = vi.fn()
const promise = uploadAllParts(makeFile(CHUNK_SIZE + 5_000_000), 'key', 'upload-1', controller, onProgress)
await vi.runAllTimersAsync()

expect(await promise).toEqual([
{ PartNumber: 1, ETag: '"etag-1"' },
{ PartNumber: 2, ETag: '"etag-2"' },
])
expect(onProgress).toHaveBeenLastCalledWith(95)
})

it('retries a part that fails transiently and still succeeds', async () => {
global.fetch = vi.fn()
.mockResolvedValueOnce(failResponse())
.mockResolvedValueOnce(failResponse())
.mockResolvedValueOnce(okResponse('"etag-1"')) as unknown as typeof fetch

const promise = uploadAllParts(makeFile(1024), 'key', 'upload-1', controller, vi.fn())
await vi.runAllTimersAsync()

expect(await promise).toEqual([{ PartNumber: 1, ETag: '"etag-1"' }])
expect(global.fetch).toHaveBeenCalledTimes(3)
})

it('re-fetches the presigned URL on every attempt, since it can expire mid-backoff', async () => {
global.fetch = vi.fn()
.mockResolvedValueOnce(failResponse())
.mockResolvedValueOnce(okResponse('"etag-1"')) as unknown as typeof fetch

const promise = uploadAllParts(makeFile(1024), 'key', 'upload-1', controller, vi.fn())
await vi.runAllTimersAsync()
await promise

const presignCalls = vi.mocked(api.post).mock.calls.filter((c) => c[0] === '/upload/presign-part')
expect(presignCalls).toHaveLength(2)
})

it('gives up after the attempt limit and surfaces the last error', async () => {
global.fetch = vi.fn().mockResolvedValue(failResponse(500, 'Internal Server Error')) as unknown as typeof fetch

const promise = uploadAllParts(makeFile(1024), 'key', 'upload-1', controller, vi.fn())
const assertion = expect(promise).rejects.toThrow(/Part 1 failed/)
await vi.runAllTimersAsync()
await assertion

expect(global.fetch).toHaveBeenCalledTimes(8)
})

it('does not retry a 4xx, which would fail identically every time', async () => {
global.fetch = vi.fn().mockResolvedValue(failResponse(403, 'Forbidden')) as unknown as typeof fetch

const promise = uploadAllParts(makeFile(1024), 'key', 'upload-1', controller, vi.fn())
const assertion = expect(promise).rejects.toThrow(/Forbidden/)
await vi.runAllTimersAsync()
await assertion

expect(global.fetch).toHaveBeenCalledTimes(1)
})

it('does retry a 429, which explicitly invites a later attempt', async () => {
global.fetch = vi.fn()
.mockResolvedValueOnce(failResponse(429, 'Too Many Requests'))
.mockResolvedValueOnce(okResponse('"etag-1"')) as unknown as typeof fetch

const promise = uploadAllParts(makeFile(1024), 'key', 'upload-1', controller, vi.fn())
await vi.runAllTimersAsync()

expect(await promise).toEqual([{ PartNumber: 1, ETag: '"etag-1"' }])
expect(global.fetch).toHaveBeenCalledTimes(2)
})

it('does not retry once the upload was cancelled', async () => {
global.fetch = vi.fn().mockImplementation(() => {
controller.abort()
return Promise.reject(new DOMException('The operation was aborted', 'AbortError'))
}) as unknown as typeof fetch

const promise = uploadAllParts(makeFile(1024), 'key', 'upload-1', controller, vi.fn())
const assertion = expect(promise).rejects.toThrow(/aborted/)
await vi.runAllTimersAsync()
await assertion

expect(global.fetch).toHaveBeenCalledTimes(1)
})

it('throws before any request when cancelled upfront', async () => {
global.fetch = vi.fn() as unknown as typeof fetch
controller.abort()

await expect(
uploadAllParts(makeFile(1024), 'key', 'upload-1', controller, vi.fn()),
).rejects.toThrow(/cancelled/)

expect(global.fetch).not.toHaveBeenCalled()
})
})
174 changes: 127 additions & 47 deletions apps/web/stores/upload-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,127 @@ import type { AssetResponse } from '@/types'

const CHUNK_SIZE = 10 * 1024 * 1024 // 10 MB
const HISTORY_PAGE_SIZE = 20
const PART_MAX_ATTEMPTS = 8 // per part, including the first try
const PART_RETRY_BASE_MS = 2000 // 2s, 4s, 8s … ≈254s of total tolerance per part

/**
* A failure that repeating cannot fix — a 4xx from the storage backend means the
* request itself was rejected (bad signature, expired policy, wrong length), and
* every further attempt is rejected identically.
*/
interface PartUploadError extends Error {
permanent?: boolean
}

/** 408 Request Timeout and 429 Too Many Requests explicitly invite a later attempt. */
function isPermanentStatus(status: number): boolean {
return status >= 400 && status < 500 && status !== 408 && status !== 429
}

/** Uploads one part exactly once. Returns its ETag. */
async function uploadPartOnce(
file: File,
s3Key: string,
uploadId: string,
partNumber: number,
controller: AbortController,
): Promise<string> {
const start = (partNumber - 1) * CHUNK_SIZE
const chunk = file.slice(start, Math.min(start + CHUNK_SIZE, file.size))

// The presigned URL is fetched per attempt, not cached across retries:
// presign_upload_part expires after an hour and a large upload can outlive
// that, so a URL obtained before a long backoff may already be dead.
const { presigned_url } = await api.post<{ presigned_url: string }>('/upload/presign-part', {
s3_key: s3Key,
upload_id: uploadId,
part_number: partNumber,
})

const putResponse = await fetch(presigned_url, {
method: 'PUT',
body: chunk,
signal: controller.signal,
})

if (!putResponse.ok) {
const error: PartUploadError = new Error(`Part ${partNumber} failed: ${putResponse.statusText}`)
error.permanent = isPermanentStatus(putResponse.status)
throw error
}

return putResponse.headers.get('ETag') ?? ''
}

/**
* Uploads one part, retrying with exponential backoff and jitter.
*
* A multi-gigabyte file is several hundred requests. Previously the first
* non-OK response aborted the entire upload without a second attempt, so a
* single transient 500/503 from the storage backend — or a brief network drop —
* discarded everything already transferred.
*
* Only failures that can plausibly recover are repeated: network errors and 5xx.
* A 4xx is thrown straight through, because retrying it eight times over four
* minutes would delay the error message without ever changing the outcome.
*
* Jitter keeps concurrent uploads from retrying in lockstep. A user-initiated
* cancel is never retried.
*/
async function uploadPart(
file: File,
s3Key: string,
uploadId: string,
partNumber: number,
controller: AbortController,
): Promise<string> {
let lastError: unknown

for (let attempt = 1; attempt <= PART_MAX_ATTEMPTS; attempt++) {
try {
return await uploadPartOnce(file, s3Key, uploadId, partNumber, controller)
} catch (err) {
if (controller.signal.aborted) throw err
if (err instanceof DOMException && err.name === 'AbortError') throw err
if ((err as PartUploadError)?.permanent) throw err

lastError = err
if (attempt === PART_MAX_ATTEMPTS) break

const delay = PART_RETRY_BASE_MS * 2 ** (attempt - 1) + Math.random() * 250
await new Promise((resolve) => setTimeout(resolve, delay))
}
}

throw lastError
}

/**
* Uploads every part of a multipart upload in order and returns the part list
* for /upload/complete. Exported for tests.
*/
export async function uploadAllParts(
file: File,
s3Key: string,
uploadId: string,
controller: AbortController,
onProgress: (percent: number) => void,
): Promise<Array<{ PartNumber: number; ETag: string }>> {
const totalChunks = Math.ceil(file.size / CHUNK_SIZE)
const parts: Array<{ PartNumber: number; ETag: string }> = []

for (let partNumber = 1; partNumber <= totalChunks; partNumber++) {
if (controller.signal.aborted) {
throw new DOMException('Upload cancelled', 'AbortError')
}

const etag = await uploadPart(file, s3Key, uploadId, partNumber, controller)
parts.push({ PartNumber: partNumber, ETag: etag })
onProgress(Math.round((partNumber / totalChunks) * 95))
}

return parts
}

export type UploadStatus = 'pending' | 'uploading' | 'processing' | 'complete' | 'failed' | 'cancelled'

Expand Down Expand Up @@ -179,39 +300,9 @@ const storeCreator: StateCreator<UploadStore, [['zustand/persist', unknown]]> =

updateFile(id, { uploadId: upload_id, assetId: asset_id, versionId: version_id })

const totalChunks = Math.ceil(file.size / CHUNK_SIZE)
const parts: Array<{ PartNumber: number; ETag: string }> = []

for (let partNumber = 1; partNumber <= totalChunks; partNumber++) {
if (controller.signal.aborted) {
throw new DOMException('Upload cancelled', 'AbortError')
}

const start = (partNumber - 1) * CHUNK_SIZE
const end = Math.min(start + CHUNK_SIZE, file.size)
const chunk = file.slice(start, end)

const { presigned_url } = await api.post<{ presigned_url: string }>('/upload/presign-part', {
s3_key,
upload_id,
part_number: partNumber,
})

const putResponse = await fetch(presigned_url, {
method: 'PUT',
body: chunk,
signal: controller.signal,
})

if (!putResponse.ok) {
throw new Error(`Part ${partNumber} failed: ${putResponse.statusText}`)
}

const etag = putResponse.headers.get('ETag') ?? ''
parts.push({ PartNumber: partNumber, ETag: etag })

updateFile(id, { progress: Math.round((partNumber / totalChunks) * 95) })
}
const parts = await uploadAllParts(file, s3_key, upload_id, controller, (percent) =>
updateFile(id, { progress: percent }),
)

await api.post('/upload/complete', {
s3_key,
Expand Down Expand Up @@ -293,20 +384,9 @@ const storeCreator: StateCreator<UploadStore, [['zustand/persist', unknown]]> =
version_id = initRes.version_id
updateFile(id, { uploadId: upload_id, versionId: version_id })

const totalChunks = Math.ceil(file.size / CHUNK_SIZE)
const parts: Array<{ PartNumber: number; ETag: string }> = []
for (let partNumber = 1; partNumber <= totalChunks; partNumber++) {
if (controller.signal.aborted) throw new DOMException('Upload cancelled', 'AbortError')
const start = (partNumber - 1) * CHUNK_SIZE
const chunk = file.slice(start, Math.min(start + CHUNK_SIZE, file.size))
const { presigned_url } = await api.post<{ presigned_url: string }>('/upload/presign-part', {
s3_key, upload_id, part_number: partNumber,
})
const putResponse = await fetch(presigned_url, { method: 'PUT', body: chunk, signal: controller.signal })
if (!putResponse.ok) throw new Error(`Part ${partNumber} failed: ${putResponse.statusText}`)
parts.push({ PartNumber: partNumber, ETag: putResponse.headers.get('ETag') ?? '' })
updateFile(id, { progress: Math.round((partNumber / totalChunks) * 95) })
}
const parts = await uploadAllParts(file, s3_key, upload_id, controller, (percent) =>
updateFile(id, { progress: percent }),
)

await api.post('/upload/complete', { s3_key, upload_id, asset_id: assetId, version_id, parts })
const isMedia = file.type.startsWith('video/') || file.type.startsWith('audio/') || file.type.startsWith('image/')
Expand Down