Skip to content

fix(upload): honor root and retries upload options - #1745

Merged
susnux merged 1 commit into
mainfrom
fix/params
Sep 16, 2026
Merged

susnux merged 1 commit into
mainfrom
fix/params

Conversation

@susnux

@susnux susnux commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Both options were declared and documented on UploadOptions but never read: upload() and batchUpload() hard-coded the uploader destination as the base, and retries was spread into UploadFile, which only destructured headers and noChunking - so chunk workspace creation used a hard-coded 5 and the upload request got no retry count at all.

🤖 AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Both options were declared and documented on `UploadOptions` but never read:
`upload()` and `batchUpload()` hard-coded the uploader destination as the base,
and `retries` was spread into `UploadFile`, which only destructured `headers`
and `noChunking` - so chunk workspace creation used a hard-coded 5 and the
upload request got no retry count at all.

`upload()` and `batchUpload()` now resolve their target via a shared helper
falling back to the uploader destination, and `retries` (default 5) is stored
on `UploadFile`/`UploadFileTree`, forwarded to `uploadData`/`initChunkWorkspace`
and propagated to child uploads.

Assisted-by: ClaudeCode:claude-opus-5
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux
susnux requested a review from skjnldsv September 16, 2026 17:02
@susnux susnux added type: bug 🐛 Something isn't working 3. to review 3️⃣ Waiting for reviews labels Sep 16, 2026
@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.18%. Comparing base (850dce6) to head (6c7dbfc).

Files with missing lines Patch % Lines
lib/upload/uploader/UploadFile.ts 75.00% 1 Missing ⚠️
lib/upload/uploader/UploadFileTree.ts 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1745      +/-   ##
==========================================
- Coverage   87.42%   87.18%   -0.24%     
==========================================
  Files          44       44              
  Lines        1121     1116       -5     
  Branches      281      280       -1     
==========================================
- Hits          980      973       -7     
- Misses        105      107       +2     
  Partials       36       36              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@skjnldsv skjnldsv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner! 👍
Was a bit confused by the extend initially, but the options forwarding simplicity speaks for itself

@susnux
susnux merged commit ba0f6d0 into main Sep 16, 2026
11 of 13 checks passed
@susnux
susnux deleted the fix/params branch September 16, 2026 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review 3️⃣ Waiting for reviews type: bug 🐛 Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants