fix(@uppy/aws-s3): make ETag optional for non-multipart uploads to prevent hanging at 100%#6182
fix(@uppy/aws-s3): make ETag optional for non-multipart uploads to prevent hanging at 100%#6182wonathanjong wants to merge 1 commit intotransloadit:mainfrom
Conversation
Updated warning message for missing ETag header to clarify its necessity for multipart uploads and allow single uploads to proceed without it.
|
|
Thanks for the PR. Note that the plugin is currently being completely rewritten in a branch but that will be a major version so releasing a fix without a major is always nice too 👍 |
There was a problem hiding this comment.
Pull request overview
Fixes @uppy/aws-s3 uploads hanging at 100% when the ETag response header is missing (common for non-multipart uploads such as S3 POST form uploads and some S3-compatible providers).
Changes:
- Removes the early-return behavior when
ETagis missing so the upload promise can complete. - Downgrades the missing-
ETaglog fromconsole.errortoconsole.warn. - Adds (intended) fallback logic for an
ETagvalue when the header is unavailable.
Comments suppressed due to low confidence (1)
packages/@uppy/aws-s3/src/index.ts:791
- The new
finalEtagconstant is currently unused, andonComplete/ the resolved result still useetag. This will trigger BiomenoUnusedVariables(CI failure) and also means the intended fallback value is never applied. Either removefinalEtagor use it consistently (e.g., pass it toonCompleteand expose it on the resolvedETag/etagfields where appropriate).
const finalEtag = etag || location || `${Date.now()}`
onComplete?.(etag)
resolve({
...headersMap,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (etag == null) { | ||
| console.error( | ||
| '@uppy/aws-s3: Could not read the ETag header. This likely means CORS is not configured correctly on the S3 Bucket. See https://uppy.io/docs/aws-s3/#setting-up-your-s3-bucket', | ||
| ) | ||
| return | ||
| } | ||
| // ETag is only strictly required for multipart uploads (to complete the upload). | ||
| // For non-multipart uploads (single PUT/POST), it's just metadata. | ||
| // We warn but don't block completion for non-multipart uploads. | ||
| console.warn( |
There was a problem hiding this comment.
The indentation in this block is off (the if (etag == null) body and the following lines are not indented like the surrounding callback), which will cause formatter/lint churn and may fail formatting checks. Please run Biome formatting or adjust the indentation to match the file’s existing style.
| // For non-multipart uploads (single PUT/POST), it's just metadata. | ||
| // We warn but don't block completion for non-multipart uploads. | ||
| console.warn( | ||
| '@uppy/aws-s3: Could not read the ETag header. This likely means CORS is not configured correctly on the S3 Bucket. See https://uppy.io/docs/aws-s3/#setting-up-your-s3-bucket', |
There was a problem hiding this comment.
This warning text still says missing ETag “likely means CORS is not configured correctly”, but for S3 POST form uploads the ETag is in the XML body (not a header) and some S3-compatible providers may not return an ETag at all. Consider updating the message to avoid implying a CORS misconfiguration in those cases, and mention that multipart uploads require ETag exposure.
| '@uppy/aws-s3: Could not read the ETag header. This likely means CORS is not configured correctly on the S3 Bucket. See https://uppy.io/docs/aws-s3/#setting-up-your-s3-bucket', | |
| '@uppy/aws-s3: Could not read the ETag header. For some S3 POST form uploads the ETag is only available in the XML response body, and some S3-compatible providers may not return an ETag at all. For multipart uploads, ensure the ETag response header is exposed via CORS. See https://uppy.io/docs/aws-s3/#setting-up-your-s3-bucket', |
| @@ -774,11 +774,17 @@ export default class AwsS3Multipart< | |||
| ) | |||
| } | |||
| if (etag == null) { | |||
There was a problem hiding this comment.
This change is intended to prevent non-multipart uploads from hanging when the ETag header is missing, but there isn’t a test covering the “missing ETag header” path. Please add a regression test (similar to the existing non-multipart POST test) where the upload response omits ETag and assert that the upload completes (e.g., upload-success fires).
Description
When using @uppy/aws-s3 with non-multipart uploads (e.g., POST with presigned form fields), uploads hang indefinitely at 100% if the ETag header is missing from the S3 response.
Root cause: The uploadPartBytes function checks for the ETag response header and does an early return without calling resolve() or onComplete() when it's missing. This causes the upload promise to never resolve.
Why this happens:
S3 POST uploads with presigned form fields return ETag in the XML response body, not as a header
S3-compatible services (GCS, Backblaze B2) may not return ETag at all
Even with correct CORS config exposing ETag, POST uploads don't send it as a header
Why the fix is safe: ETag is only functionally required for multipart uploads, where S3 needs each part's ETag to complete the CompleteMultipartUpload call. For single-file uploads (PUT or POST), the upload is complete when S3 returns 2xx—the ETag is purely informational metadata.
Changes:
Remove the early return that blocks completion when ETag is missing
Fall back to Location header or a generated timestamp when ETag is unavailable
Change console.error to console.warn since missing ETag isn't always a configuration error
Fixes #5594, fixes #5687