feat(drivers/s3): support direct multipart upload - #609
Conversation
c0ab82b to
afb0937
Compare
xrgzs
changed the title
- Upload S3 direct multipart parts using presigned part URLs. - Complete multipart uploads with collected ETags and abort on failures. - Treat embedded CompleteMultipartUpload XML errors as upload failures. - Build completion XML with DOM APIs instead of manual escaping.
pikachuren
left a comment
pikachuren
left a comment
There was a problem hiding this comment.
结论：方向正确，但有几处会影响大文件上传成功率的问题需要处理
整体设计合理：通过 upload_urls 是否存在来判断走 multipart 分支，对后端未返回该字段的旧行为完全无影响；失败时调用 abort_url 避免在对象存储侧留下计费中的未完成分片，这个考虑很到位；validateS3CompleteResponse 特意处理了 S3「HTTP 200 但响应体是 <Error>」这一经典陷阱，说明作者对 S3 协议细节是了解的。下面几点建议在合并前处理。
一、分片失败没有重试，大文件成功率会受影响（建议优先处理）
for (let i = 0; i < uploadInfo.upload_urls.length; i++) {
...
const etag = await uploadS3Part(...)任何一个分片因网络抖动失败，整个上传就直接抛错并 abort，用户只能从头再来。multipart 的主要价值恰恰在于「失败只需重传单个分片」，目前的实现把这个优势丢掉了。一个几十 GB 的文件在传输后期失败一次，代价非常大。建议对单个分片加入有限次数的指数退避重试。
二、ETag 的引号处理需要确认
const etag = xhr.getResponseHeader("ETag")
...
etagNode.textContent = etagS3 返回的 ETag 通常带双引号（如 "d41d8cd98f00b204e9800998ecf8427e"），而这里原样写入了 CompleteMultipartUpload 的 XML。多数实现能容忍，但部分 S3 兼容存储（MinIO、Ceph RGW 等）对此校验较严，会返回 InvalidPart。建议明确处理，比如去掉首尾引号后再写入，并在注释里说明依据。
三、getResponseHeader("ETag") 依赖 CORS 配置，需要给出明确的错误引导
浏览器只能读取 Access-Control-Expose-Headers 中列出的响应头。如果存储桶的 CORS 没有暴露 ETag，这里会稳定拿到 null，然后抛出 Upload part N did not return an ETag。这个报错对用户来说指向性很差——实际是服务端配置问题，却看起来像上传失败。
建议在这个错误信息里直接提示可能的原因（需在存储桶 CORS 中暴露 ETag 响应头），并在文档中补充说明。否则这会成为一个高频且难以自助排查的支持问题。
四、abort 是 fire-and-forget
void fetch(uploadInfo.abort_url, { method: "DELETE" })不等待也不处理失败。abort 本身失败（网络断开、URL 过期）时用户和日志都无感知，而未清理的分片会持续产生存储费用。至少建议 catch 后打一条日志；考虑到这里紧接着就要 throw error，也可以 await 后再抛，让清理有确定结果。
五、两处小问题
uploadURL和method在 multipart 早退分支之前就被计算了，走 multipart 时这两个变量是无用的。建议把if (uploadInfo.upload_urls)判断上移到取值之前，逻辑更清晰。- 分片是完全串行上传的。既然后端一次性下发了全部
upload_urls，并发上传能显著提升吞吐。仓库里已有src/utils/async_pool.ts可以复用。若刻意选择串行（例如为了让进度条单调递增、或控制内存占用），建议加注释说明，否则容易被后来者误认为是遗漏。
另外这个 PR 的 CI 没有运行（分支为 feat/s3，而 PR 构建工作流只在目标为 main 的 PR 上触发），合并前请确认已通过实际的 S3 兼容存储做过端到端验证，最好覆盖一次超过分片阈值的大文件。
2 participants
Summary / 摘要
There is no user-visible behavior changes for uploading.
HTTP Direct for S3 is now upload by multipart unless DirectUploadMaxParts=1 while the default value is 10000. If S3 is standard, there will be nothing change.
/ 此 PR 包含破坏性变更。
/ 此 PR 修改了公开 API、配置、存储格式或迁移行为。
/ 此 PR 需要关联仓库同步修改。
Related repository PRs / 关联仓库 PR:
Related Issues / 关联 Issue
Testing / 测试
go test ./...no go test available
test on Aliyun OSS and it uploads in 50 parts as planned. No larger file tested.
Checklist / 检查清单
/ 我已阅读 CONTRIBUTING。
/ 我确认此贡献符合仓库许可证、贡献规范和行为准则。
gofmt,go fmt, orprettierwhere applicable./ 我已按适用情况使用
gofmt、go fmt或prettier格式化变更代码。/ 我已在适用情况下请求相关维护者或代码所有者审查。
No right to request review.
AI Disclosure / AI 使用声明
/ 此 PR 包含 AI 辅助内容。
Tools used / 使用工具:
Usage scope / 使用范围:
Code generation / 代码生成
Refactoring / 重构
Documentation / 文档
Tests / 测试
Translation / 翻译
Review assistance / 审查辅助
I have reviewed and validated all AI-assisted content included in this PR.
/ 我已审核并验证此 PR 中的所有 AI 辅助内容。
I have ensured that all AI-assisted commits include
Co-Authored-Byattribution./ 我已确保所有 AI 辅助提交都包含
Co-Authored-By归属信息。I can reproduce all AI-assisted content included in this PR without any AI tools.
/ 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。