Skip to content

fix(upload,sync): surface partial upload failures instead of silently swallowing them#4

Merged
Bebbssos merged 1 commit into
mainfrom
upload-sync/surface-partial-failures
Jul 21, 2026
Merged

fix(upload,sync): surface partial upload failures instead of silently swallowing them#4
Bebbssos merged 1 commit into
mainfrom
upload-sync/surface-partial-failures

Conversation

@Bebbssos

Copy link
Copy Markdown
Owner

Summary

  • Root-caused a "why did upload folder skip a file with no error" report: per-file failures in upload folder were only ever reported via pb.println, and indicatif fully hides a ProgressBar's draw target (including println output, not just the animated bar) whenever stderr isn't a real terminal — so any piped/scripted/CI run reported blanket success and exit 0 no matter how many files failed. Confirmed live: stdout and stderr both came back completely empty on a run where a file demonstrably failed.
  • upload folder now tracks failures independently of the progress bar and returns an error naming every failed file + reason, causing a non-zero exit.
  • sync up/sync down (and download folder, which reuses sync_down) already tracked failures in a Summary independent of the bar, but always returned Ok(()) regardless; they now exit non-zero too when failed > 0.
  • Separately confirmed: Internxt's free/legacy plans reject 0-byte files with HTTP 402, and upload folder's local scan silently skipped 0-byte files before this fix — which mirrors og's own local-filesystem.service.ts (stats.isFile() && stats.size > 0) exactly, not a divergence. Per explicit request, flipped that default: empty files are now included by default in both upload folder and sync up (left to fail/succeed visibly per the account's plan), with a new --exclude-empty-files flag to restore the old skip-silently behavior.
  • README updated for both commands' flags and JSON output.

Test plan

  • cargo build --all-features, cargo clippy --all-features --all-targets, cargo test all clean
  • Live-verified against a real test account: default upload folder on a 0-byte file now exits 1 with "1 of 2 file(s) failed to upload ... HTTP 402 Payment Required..." on stderr
  • Live-verified --exclude-empty-files restores the old clean skip / exit 0
  • Re-ran a 210-file concurrent upload to confirm no regression on the success path

🤖 Generated with Claude Code

https://claude.ai/code/session_019s9ZyfTYiVG8nHemEWFdEM

… swallowing them

Root-caused a "why did upload folder skip a file with no error" report:
per-file failures in `upload folder` were only ever reported via
`pb.println`, and indicatif fully hides a ProgressBar's draw target
(including println output, not just the animated bar) whenever stderr
isn't a real terminal. Any piped/scripted/CI/non-interactive run of
`upload folder` therefore reported blanket success and exit 0 no matter
how many files failed, with zero trace of what happened. Confirmed via
a live repro: stdout and stderr both came back completely empty for a
run where a file demonstrably failed to upload.

`upload folder` now tracks failures independently of the progress bar
and returns an error naming every failed file and its reason, causing a
non-zero exit. `sync up`/`sync down` (and `download folder`, which
reuses `sync_down`) already tracked failures in a `Summary` independent
of the bar, but still always returned `Ok(())`; they now exit non-zero
too when `failed > 0`, without double-printing (the existing summary
output already reports it).

Separately, confirmed (and initially misdiagnosed) two things while
investigating: Internxt's free/legacy plans reject 0-byte files with
HTTP 402, and `upload folder`'s local scan silently skipped 0-byte files
before this fix — which, per og's own `local-filesystem.service.ts`
(`stats.isFile() && stats.size > 0`), mirrors the reference CLI exactly,
not a divergence. Per explicit request, changed that default: empty
files are now included by default in both `upload folder` and `sync up`
(and left to fail/succeed based on the account's plan, now visibly), with
a new `--exclude-empty-files` flag to restore the old skip-silently
behavior for accounts where the 402 is expected and not worth reporting.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019s9ZyfTYiVG8nHemEWFdEM
@Bebbssos
Bebbssos merged commit b1e87c5 into main Jul 21, 2026
1 check passed
@Bebbssos
Bebbssos deleted the upload-sync/surface-partial-failures branch July 21, 2026 17:20
Bebbssos added a commit that referenced this pull request Jul 22, 2026
… bailout

internxt-core 0.1.2 (released, includes PR #4) makes create_folder_with_retry
look up and return the existing folder's uuid on a name collision instead of
giving up, so the common case is now fully transparent: a colliding folder
(and its contents) upload normally instead of being skipped.

Ok(None) is now only the rarer case (lookup miss on a race, or exhausted
non-conflict retries) — treat it the same as any other per-item failure:
track it in the shared `failed` list (moved up so both the folder and file
loops share it) and skip just that subtree, instead of aborting the whole
upload via the old `folder_map.is_empty()` bailout (removed). A missing
parent (because its own folder failed) is now also tracked in `failed`
rather than just a status line, so it's no longer invisible in the final
summary/exit code either.

Live-verified: re-uploading an identical tree (folder-name collision on
every level) now succeeds at the folder level ("Folder ready: x" for all
three, no skipping) and only the genuinely-conflicting files fail with a
clear 409 each — exactly the behavior the original bug report wanted,
instead of the old unconditional "Failed to create folders, cannot upload
files" abort.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant