Skip to content

Commit c448556

Browse files
committed
fix(uploads): only retry transient 5xx, not deterministic 4xx
DirectUploadError now carries the HTTP status. isTransientUploadError gates on 5xx so callers don't loop on 400/403/404 (e.g., malformed request, expired signature). Multipart per-part retry also short-circuits on 4xx — same reasoning.
1 parent 5abdb7c commit c448556

1 file changed

Lines changed: 33 additions & 13 deletions

File tree

apps/sim/lib/uploads/client/direct-upload.ts

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ export class DirectUploadError extends Error {
5656
constructor(
5757
message: string,
5858
public code: DirectUploadErrorCode,
59-
public details?: unknown
59+
public details?: unknown,
60+
public status?: number
6061
) {
6162
super(message)
6263
this.name = 'DirectUploadError'
@@ -65,13 +66,15 @@ export class DirectUploadError extends Error {
6566

6667
/**
6768
* Transport-level upload errors worth retrying at the outer level: timeouts,
68-
* 5xx from the storage backend, and per-part failures whose inner retry was
69-
* exhausted. Excludes deterministic failures (`PRESIGNED_URL_ERROR`,
70-
* `FALLBACK_REQUIRED`) and aborts.
69+
* network failures, and 5xx from the storage backend. Excludes deterministic
70+
* client failures (4xx, `PRESIGNED_URL_ERROR`, `FALLBACK_REQUIRED`) and aborts.
7171
*/
72-
export const isTransientUploadError = (error: unknown): boolean =>
73-
error instanceof DirectUploadError &&
74-
(error.code === 'DIRECT_UPLOAD_ERROR' || error.code === 'MULTIPART_ERROR')
72+
export const isTransientUploadError = (error: unknown): boolean => {
73+
if (!(error instanceof DirectUploadError)) return false
74+
if (error.code !== 'DIRECT_UPLOAD_ERROR' && error.code !== 'MULTIPART_ERROR') return false
75+
if (error.status === undefined) return true
76+
return error.status >= 500 && error.status < 600
77+
}
7578

7679
const calculateUploadTimeoutMs = (fileSize: number): number => {
7780
const sizeInMb = fileSize / (1024 * 1024)
@@ -269,7 +272,9 @@ const uploadViaPresignedPut = (opts: UploadViaPutOptions): Promise<void> => {
269272
reject(
270273
new DirectUploadError(
271274
`Direct upload failed for ${file.name}: ${xhr.status} ${xhr.statusText}`,
272-
'DIRECT_UPLOAD_ERROR'
275+
'DIRECT_UPLOAD_ERROR',
276+
undefined,
277+
xhr.status
273278
)
274279
)
275280
}
@@ -349,7 +354,9 @@ const uploadViaMultipart = async (
349354
}
350355
throw new DirectUploadError(
351356
`Failed to initiate multipart upload: ${initiateResponse.statusText}`,
352-
'MULTIPART_ERROR'
357+
'MULTIPART_ERROR',
358+
undefined,
359+
initiateResponse.status
353360
)
354361
}
355362

@@ -387,7 +394,9 @@ const uploadViaMultipart = async (
387394
await abortMultipart()
388395
throw new DirectUploadError(
389396
`Failed to get part URLs: ${partUrlsResponse.statusText}`,
390-
'MULTIPART_ERROR'
397+
'MULTIPART_ERROR',
398+
undefined,
399+
partUrlsResponse.status
391400
)
392401
}
393402

@@ -423,7 +432,9 @@ const uploadViaMultipart = async (
423432
if (!partResponse.ok) {
424433
throw new DirectUploadError(
425434
`Failed to upload part ${partNumber}: ${partResponse.statusText}`,
426-
'MULTIPART_ERROR'
435+
'MULTIPART_ERROR',
436+
undefined,
437+
partResponse.status
427438
)
428439
}
429440

@@ -433,7 +444,14 @@ const uploadViaMultipart = async (
433444

434445
return { partNumber, etag: etag?.replace(/"/g, '') }
435446
} catch (partError) {
436-
if (isAbortError(partError) || attempt >= MULTIPART_MAX_RETRIES) throw partError
447+
const isClientError =
448+
partError instanceof DirectUploadError &&
449+
partError.status !== undefined &&
450+
partError.status >= 400 &&
451+
partError.status < 500
452+
if (isAbortError(partError) || isClientError || attempt >= MULTIPART_MAX_RETRIES) {
453+
throw partError
454+
}
437455
const delay = MULTIPART_RETRY_DELAY_MS * MULTIPART_RETRY_BACKOFF ** attempt
438456
logger.warn(
439457
`Part ${partNumber} failed (attempt ${attempt + 1}), retrying in ${Math.round(delay / 1000)}s`
@@ -475,7 +493,9 @@ const uploadViaMultipart = async (
475493
await abortMultipart()
476494
throw new DirectUploadError(
477495
`Failed to complete multipart upload: ${completeResponse.statusText}`,
478-
'MULTIPART_ERROR'
496+
'MULTIPART_ERROR',
497+
undefined,
498+
completeResponse.status
479499
)
480500
}
481501

0 commit comments

Comments
 (0)