From 8171414f8ad5842596710261a8351701f970ed6d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 19:51:17 +0000 Subject: [PATCH] Remove the legacy single-request video upload path Multipart is now the only upload transport, so drop the legacy `app.bsky.video.uploadVideo` request and everything that existed to choose between the two: - `MultipartFallbackError` is gone. A `startUpload` failure (a service without multipart support, or with the kill switch active) now propagates as the `MultipartUploadError` the API layer already built, and `abortThenRethrowOrResolve` rethrows the failure that forced the abort instead of a fallback signal - which keeps the real error class for retry classification and telemetry. - `getUploadErrorMessage` matches on `MultipartUploadError` where it matched on `ServerError`, so video-service messages (upload disabled, daily limits, oversized file) still map to their user-facing copy. `ServerError` had no other thrower and is removed. - With the legacy branch gone, `upload.ts` and `upload.web.ts` were identical, so the platform split collapses into one `upload.ts`. The web chunk reader already fetches the uri when `bytes` is missing, so nothing depended on the legacy path's buffer fetch. - The `did` parameter only seeded the legacy endpoint URL; it is dropped from `uploadVideo` and `processVideo`. - Transport reporting only existed to distinguish multipart from legacy, so `VideoUploadTransport`, `telemetry.uploadTransport`, the `video.upload.transport` span attribute, and the `transport` field on the uploadCompleted/uploadFailed events are removed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mn8DoxP5wQk6NbPmapWs9A --- src/analytics/metrics/types.ts | 7 +- src/lib/media/video/errors.ts | 7 -- src/lib/media/video/multipart/upload.ts | 37 +++---- src/lib/media/video/telemetry.ts | 10 -- src/lib/media/video/types.ts | 2 - src/lib/media/video/upload.ts | 88 ++--------------- src/lib/media/video/upload.web.ts | 124 ------------------------ src/view/com/composer/Composer.tsx | 23 +---- src/view/com/composer/state/video.ts | 12 +-- 9 files changed, 26 insertions(+), 284 deletions(-) delete mode 100644 src/lib/media/video/upload.web.ts diff --git a/src/analytics/metrics/types.ts b/src/analytics/metrics/types.ts index 8e43589281..78a4574f0f 100644 --- a/src/analytics/metrics/types.ts +++ b/src/analytics/metrics/types.ts @@ -5,10 +5,7 @@ import {type Platform} from 'react-native' import {type NotificationReason} from '#/lib/hooks/useNotificationHandler' -import { - type VideoCompressSkipReason, - type VideoUploadTransport, -} from '#/lib/media/video/types' +import {type VideoCompressSkipReason} from '#/lib/media/video/types' import {type NotificationType} from '#/state/queries/notifications/types' import {type FeedDescriptor} from '#/state/queries/post-feed' import {type LiveEventFeedMetricContext} from '#/features/liveEvents/types' @@ -1473,7 +1470,6 @@ export type Events = { bytes: number elapsedMs: number throughputBytesPerSec: number - transport: VideoUploadTransport } 'video:upload:uploadFailed': { uploadId: string @@ -1481,7 +1477,6 @@ export type Events = { bytes: number errorClass: string elapsedMs: number - transport: VideoUploadTransport } 'video:upload:processingStarted': { uploadId: string diff --git a/src/lib/media/video/errors.ts b/src/lib/media/video/errors.ts index cbf3083d1f..cdeeb4095e 100644 --- a/src/lib/media/video/errors.ts +++ b/src/lib/media/video/errors.ts @@ -5,13 +5,6 @@ export class VideoTooLargeError extends Error { } } -export class ServerError extends Error { - constructor(message: string) { - super(message) - this.name = 'ServerError' - } -} - export class UploadLimitError extends Error { constructor(message: string) { super(message) diff --git a/src/lib/media/video/multipart/upload.ts b/src/lib/media/video/multipart/upload.ts index 3b0048ecd2..db9cc50218 100644 --- a/src/lib/media/video/multipart/upload.ts +++ b/src/lib/media/video/multipart/upload.ts @@ -26,15 +26,12 @@ import {createUploadPart} from './uploadPart' import {uploadParts} from './uploadParts' import {delay, isRetryableMultipartError, retryDelayMs} from './utils' -export class MultipartFallbackError extends Error {} - export async function uploadVideoMultipart({ video, client, dispatchUrl, setProgress, signal, - onStarted, }: { video: CompressedVideo client: Client @@ -42,25 +39,12 @@ export async function uploadVideoMultipart({ dispatchUrl: string | URL setProgress: (progress: number) => void signal: AbortSignal - onStarted?: () => void }): Promise { throwIfAborted(signal) const tokenProvider = createTokenProvider(client, dispatchUrl, signal) const token = await tokenProvider.get() const name = `${nanoid(12)}.${mimeToExt(video.mimeType)}` - let session - try { - session = await startUpload({token, video, name, signal}) - } catch (err) { - if (signal.aborted) throw new AbortError() - // A server without multipart support, or one with the kill switch active, - // leaves no reservation behind. The legacy path remains authoritative. - throw new MultipartFallbackError( - err instanceof Error ? err.message : 'Multipart upload unavailable', - ) - } - onStarted?.() - + const session = await startUpload({token, video, name, signal}) const {jobId} = session const abortOnCancel = () => { void tokenProvider @@ -89,7 +73,7 @@ export async function uploadVideoMultipart({ }) } catch (err) { if (signal.aborted) throw new AbortError() - return await abortThenFallbackOrResolve( + return await abortThenRethrowOrResolve( jobId, await tokenProvider.get(), err, @@ -166,14 +150,14 @@ async function finishAndRecover({ } } catch (err) { throwIfAborted(signal) - return await abortThenFallbackOrResolve(jobId, token, err) + return await abortThenRethrowOrResolve(jobId, token, err) } createdFailures++ if (createdFailures < MULTIPART_FINISH_ATTEMPTS) { await delay(500 * 2 ** (createdFailures - 1), signal) continue } - return await abortThenFallbackOrResolve(jobId, token, finishError) + return await abortThenRethrowOrResolve(jobId, token, finishError) case 'finishing': // The service may have assembled the upload even though the finish // request failed. Poll and retry instead of starting a second upload. @@ -224,16 +208,21 @@ async function getUploadStatusWithRetry( throw lastError } -async function abortThenFallbackOrResolve( +/** + * Releases the reservation for an upload we can no longer finish, then surfaces + * the failure that got us here. The abort can race a service-side completion, + * so a `completed` result is resolved as a success instead. + */ +async function abortThenRethrowOrResolve( jobId: string, token: string, cause: unknown, ): Promise { const result = await abortUploadWithRetry(jobId, token) if (result.state === 'aborted') { - throw new MultipartFallbackError( - cause instanceof Error ? cause.message : 'Multipart upload failed', - ) + throw cause instanceof Error + ? cause + : new MultipartUploadError('Multipart upload failed') } if (result.state === 'completed' && result.completedJobId) { const status = await getUploadStatus(jobId, token) diff --git a/src/lib/media/video/telemetry.ts b/src/lib/media/video/telemetry.ts index 717db0e0c7..6b17ffa571 100644 --- a/src/lib/media/video/telemetry.ts +++ b/src/lib/media/video/telemetry.ts @@ -5,7 +5,6 @@ import {nanoid} from 'nanoid/non-secure' import { type ProbedMetadata, type VideoCompressSkipReason, - type VideoUploadTransport, } from '#/lib/media/video/types' import {Sentry} from '#/logger/sentry/lib' import {type Metrics} from '#/analytics/metrics' @@ -46,7 +45,6 @@ export type VideoTelemetry = { compressCompleted: (video: {size: number; mimeType: string}) => void compressFailed: (e: unknown) => void uploadStarted: (bytes: number) => void - uploadTransport: (transport: VideoUploadTransport) => void uploadCompleted: (jobId: string) => void uploadFailed: (e: unknown) => void processingStarted: (jobId: string) => void @@ -72,7 +70,6 @@ export function createVideoTelemetry({ let phaseStartedAt = startedAt let jobId: string | undefined let uploadBytes: number | undefined - let uploadTransport: VideoUploadTransport = 'multipart' let txnEnded = false let abortBound = true @@ -229,11 +226,6 @@ export function createVideoTelemetry({ metric('video:upload:uploadStarted', {uploadId, engine, bytes}) }, - uploadTransport(transport) { - uploadTransport = transport - phaseSpan?.setAttribute('video.upload.transport', transport) - }, - uploadCompleted(id) { jobId = id const elapsedMs = Date.now() - phaseStartedAt @@ -246,7 +238,6 @@ export function createVideoTelemetry({ elapsedMs, throughputBytesPerSec: elapsedMs > 0 ? Math.round((bytes * 1000) / elapsedMs) : 0, - transport: uploadTransport, }) endPhaseSpan() phase = undefined @@ -259,7 +250,6 @@ export function createVideoTelemetry({ bytes: uploadBytes ?? 0, errorClass: errorClass(e), elapsedMs: Date.now() - phaseStartedAt, - transport: uploadTransport, }) endTxn('error') detachAbort() diff --git a/src/lib/media/video/types.ts b/src/lib/media/video/types.ts index 607b0b2acc..64984d4388 100644 --- a/src/lib/media/video/types.ts +++ b/src/lib/media/video/types.ts @@ -5,8 +5,6 @@ export type VideoCompressSkipReason = 'gif' | 'below-byte-threshold' | 'no-webcodecs' | 'compress-error-fallback' -export type VideoUploadTransport = 'multipart' | 'legacy-fallback' - export type CompressedVideo = { uri: string mimeType: string diff --git a/src/lib/media/video/upload.ts b/src/lib/media/video/upload.ts index 3a98c77647..1e390372b5 100644 --- a/src/lib/media/video/upload.ts +++ b/src/lib/media/video/upload.ts @@ -1,111 +1,37 @@ -import {createUploadTask, FileSystemUploadType} from 'expo-file-system/legacy' import {type Client} from '@atproto/lex' import {type I18n} from '@lingui/core' -import {msg} from '@lingui/core/macro' -import {nanoid} from 'nanoid/non-secure' import {AbortError} from '#/lib/async/cancelable' -import {ServerError} from '#/lib/media/video/errors' -import { - type CompressedVideo, - type VideoUploadTransport, -} from '#/lib/media/video/types' -import {type app} from '#/lexicons' -import {MultipartFallbackError, uploadVideoMultipart} from './multipart/upload' -import { - getServiceAuthToken, - getVideoUploadLimits, - serviceAuthExp, -} from './upload.shared' -import {createVideoEndpointUrl, mimeToExt} from './util' +import {type CompressedVideo} from '#/lib/media/video/types' +import {uploadVideoMultipart} from './multipart/upload' +import {getVideoUploadLimits} from './upload.shared' export async function uploadVideo({ video, client, dispatchUrl, - did, setProgress, signal, i18n, - onTransport, }: { video: CompressedVideo client: Client /** The account's PDS/dispatch URL, for the uploadBlob service-auth token. */ dispatchUrl: string | URL - did: string setProgress: (progress: number) => void signal: AbortSignal i18n: I18n - onTransport?: (transport: VideoUploadTransport) => void }) { if (signal.aborted) { throw new AbortError() } await getVideoUploadLimits(client, i18n) - try { - return await uploadVideoMultipart({ - video, - client, - dispatchUrl, - setProgress, - signal, - onStarted: () => onTransport?.('multipart'), - }) - } catch (err) { - if (!(err instanceof MultipartFallbackError)) throw err - onTransport?.('legacy-fallback') - setProgress(0) - } - - const uri = createVideoEndpointUrl('/xrpc/app.bsky.video.uploadVideo', { - did, - name: `${nanoid(12)}.${mimeToExt(video.mimeType)}`, - }) - - if (signal.aborted) { - throw new AbortError() - } - const token = await getServiceAuthToken({ + return await uploadVideoMultipart({ + video, client, dispatchUrl, - lxm: 'com.atproto.repo.uploadBlob', - exp: serviceAuthExp(), + setProgress, + signal, }) - const uploadTask = createUploadTask( - uri, - video.uri, - { - headers: { - 'content-type': video.mimeType, - Authorization: `Bearer ${token}`, - }, - httpMethod: 'POST', - uploadType: FileSystemUploadType.BINARY_CONTENT, - }, - p => setProgress(p.totalBytesSent / p.totalBytesExpectedToSend), - ) - - if (signal.aborted) { - throw new AbortError() - } - const res = await uploadTask.uploadAsync() - - if (!res?.body) { - throw new Error('No response') - } - - const responseBody = JSON.parse(res.body) as app.bsky.video.defs.JobStatus - - if (!responseBody.jobId) { - throw new ServerError( - responseBody.error || i18n._(msg`Failed to upload video`), - ) - } - - if (signal.aborted) { - throw new AbortError() - } - return responseBody } diff --git a/src/lib/media/video/upload.web.ts b/src/lib/media/video/upload.web.ts deleted file mode 100644 index 97048271a4..0000000000 --- a/src/lib/media/video/upload.web.ts +++ /dev/null @@ -1,124 +0,0 @@ -import {type Client} from '@atproto/lex' -import {type I18n} from '@lingui/core' -import {msg} from '@lingui/core/macro' -import {nanoid} from 'nanoid/non-secure' - -import {AbortError} from '#/lib/async/cancelable' -import {ServerError} from '#/lib/media/video/errors' -import { - type CompressedVideo, - type VideoUploadTransport, -} from '#/lib/media/video/types' -import {type app} from '#/lexicons' -import {MultipartFallbackError, uploadVideoMultipart} from './multipart/upload' -import { - getServiceAuthToken, - getVideoUploadLimits, - serviceAuthExp, -} from './upload.shared' -import {createVideoEndpointUrl, mimeToExt} from './util' - -export async function uploadVideo({ - video, - client, - dispatchUrl, - did, - setProgress, - signal, - i18n, - onTransport, -}: { - video: CompressedVideo - client: Client - /** The account's PDS/dispatch URL, for the uploadBlob service-auth token. */ - dispatchUrl: string | URL - did: string - setProgress: (progress: number) => void - signal: AbortSignal - i18n: I18n - onTransport?: (transport: VideoUploadTransport) => void -}) { - if (signal.aborted) { - throw new AbortError() - } - await getVideoUploadLimits(client, i18n) - - try { - return await uploadVideoMultipart({ - video, - client, - dispatchUrl, - setProgress, - signal, - onStarted: () => onTransport?.('multipart'), - }) - } catch (err) { - if (!(err instanceof MultipartFallbackError)) throw err - onTransport?.('legacy-fallback') - setProgress(0) - } - - const uri = createVideoEndpointUrl('/xrpc/app.bsky.video.uploadVideo', { - did, - name: `${nanoid(12)}.${mimeToExt(video.mimeType)}`, - }) - - let bytes = video.bytes - if (!bytes) { - if (signal.aborted) { - throw new AbortError() - } - bytes = await fetch(video.uri).then(res => res.arrayBuffer()) - } - - if (signal.aborted) { - throw new AbortError() - } - const token = await getServiceAuthToken({ - client, - dispatchUrl, - lxm: 'com.atproto.repo.uploadBlob', - exp: serviceAuthExp(), - }) - - if (signal.aborted) { - throw new AbortError() - } - const xhr = new XMLHttpRequest() - const res = await new Promise( - (resolve, reject) => { - xhr.upload.addEventListener('progress', e => { - const progress = e.loaded / e.total - setProgress(progress) - }) - xhr.onloadend = () => { - if (signal.aborted) { - reject(new AbortError()) - } else if (xhr.readyState === 4) { - const uploadRes = JSON.parse( - xhr.responseText, - ) as app.bsky.video.defs.JobStatus - resolve(uploadRes) - } else { - reject(new ServerError(i18n._(msg`Failed to upload video`))) - } - } - xhr.onerror = () => { - reject(new ServerError(i18n._(msg`Failed to upload video`))) - } - xhr.open('POST', uri) - xhr.setRequestHeader('Content-Type', video.mimeType) - xhr.setRequestHeader('Authorization', `Bearer ${token}`) - xhr.send(bytes) - }, - ) - - if (!res.jobId) { - throw new ServerError(res.error || i18n._(msg`Failed to upload video`)) - } - - if (signal.aborted) { - throw new AbortError() - } - return res -} diff --git a/src/view/com/composer/Composer.tsx b/src/view/com/composer/Composer.tsx index 6af000c3ff..8738b1e1ed 100644 --- a/src/view/com/composer/Composer.tsx +++ b/src/view/com/composer/Composer.tsx @@ -273,7 +273,6 @@ export const ComposePost = ({ const chatClient = useChatClient() const pdsClient = usePdsClient() const queryClient = useQueryClient() - const currentDid = currentAccount!.did /* * The host the video service-auth token is minted for. This is the same value * that seeds the session's PDS routing, so the audience always matches the host @@ -474,21 +473,12 @@ export const ComposePost = ({ }, pdsClient, currentDispatchUrl, - currentDid, abortController.signal, i18n, telemetry, ) }, - [ - l, - i18n, - pdsClient, - currentDispatchUrl, - currentDid, - composerDispatch, - ax.metric, - ], + [l, i18n, pdsClient, currentDispatchUrl, composerDispatch, ax.metric], ) const onInitVideo = useNonReactiveCallback(() => { @@ -654,7 +644,6 @@ export const ComposePost = ({ }, pdsClient, currentDispatchUrl, - currentDid, abortController.signal, i18n, telemetry, @@ -666,15 +655,7 @@ export const ComposePost = ({ }) } }, - [ - l, - i18n, - pdsClient, - currentDispatchUrl, - currentDid, - composerDispatch, - ax.metric, - ], + [l, i18n, pdsClient, currentDispatchUrl, composerDispatch, ax.metric], ) const handleSelectDraft = useCallback( diff --git a/src/view/com/composer/state/video.ts b/src/view/com/composer/state/video.ts index 2f7a1ead62..c0764d343e 100644 --- a/src/view/com/composer/state/video.ts +++ b/src/view/com/composer/state/video.ts @@ -6,11 +6,8 @@ import {msg} from '@lingui/core/macro' import {AbortError} from '#/lib/async/cancelable' import {VIDEO_MAX_SIZE_MB} from '#/lib/constants' import {compressVideo} from '#/lib/media/video/compress' -import { - ServerError, - UploadLimitError, - VideoTooLargeError, -} from '#/lib/media/video/errors' +import {UploadLimitError, VideoTooLargeError} from '#/lib/media/video/errors' +import {MultipartUploadError} from '#/lib/media/video/multipart/api' import {type VideoTelemetry} from '#/lib/media/video/telemetry' import {type CompressedVideo} from '#/lib/media/video/types' import {uploadVideo} from '#/lib/media/video/upload' @@ -294,7 +291,6 @@ export async function processVideo( dispatch: (action: VideoAction) => void, client: Client, dispatchUrl: string | URL, - did: string, signal: AbortSignal, i18n: I18n, telemetry: VideoTelemetry, @@ -344,10 +340,8 @@ export async function processVideo( video, client, dispatchUrl, - did, signal, i18n, - onTransport: telemetry.uploadTransport, setProgress: p => { dispatch({type: 'update_progress', progress: p, signal}) }, @@ -513,7 +507,7 @@ function getUploadErrorMessage(e: unknown, i18n: I18n): string | null { if (e instanceof AbortError) { return null } - if (e instanceof ServerError || e instanceof UploadLimitError) { + if (e instanceof MultipartUploadError || e instanceof UploadLimitError) { // https://github.com/bluesky-social/tango/blob/lumi/lumi/worker/permissions.go#L77 switch (e.message) { case 'User is not allowed to upload videos':