Skip to content

fix: download-all returns error on partial failures#50

Merged
Hinne1 merged 3 commits intomainfrom
claude/fix-download-all-exit-code
Apr 14, 2026
Merged

fix: download-all returns error on partial failures#50
Hinne1 merged 3 commits intomainfrom
claude/fix-download-all-exit-code

Conversation

@Hinne1
Copy link
Copy Markdown
Contributor

@Hinne1 Hinne1 commented Apr 13, 2026

Summary

  • --download-all was returning exit code 0 even when individual file downloads failed
  • Callers (scripts, CI) had no way to detect partial failures
  • Now returns a non-zero exit code with error message when any downloads fail

Context

Discovered while building a Confluence space backup script — the script's retry logic couldn't detect failed attachment downloads because the CLI always exited 0.

Test plan

  • atl confluence page attachment <id> --download-all with a page that has attachments returns 0 on success
  • Same command returns non-zero when downloads fail (e.g. scope issue, network error)

downloadAllAttachments was returning nil even when individual file
downloads failed, causing the CLI to exit 0. Callers (and scripts)
had no way to detect partial failures.
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces error reporting for partial failures in the attachment download process by checking the error count before returning. However, the current implementation bypasses this check when JSON output is enabled, which needs to be addressed to ensure consistent exit codes.

The --json code path returned early before the error check,
so download failures with --json still exited 0.

Co-authored-by: gemini-code-assist[bot] <gemini-code-assist[bot]@users.noreply.github.com>
@Hinne1
Copy link
Copy Markdown
Contributor Author

Hinne1 commented Apr 13, 2026

@gemini-code-assist — good catch on the JSON early return. Fixed in 94cdf73: the --json path now writes output first, then checks len(errors) and returns non-zero on failures. Both output paths are now consistent.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Thanks for the update, Hinne1. The logic in internal/cmd/confluence/page/attachment.go now correctly ensures that partial failures are reported via a non-zero exit code regardless of the output format, which is a significant improvement for CI/CD pipelines and automation scripts.

@Hinne1 Hinne1 marked this pull request as ready for review April 13, 2026 09:10
@github-actions
Copy link
Copy Markdown

Merging this branch will not change overall coverage

Impacted Packages Coverage Δ 🤖
github.com/enthus-appdev/atl-cli/internal/cmd/confluence/page 0.00% (ø)

Coverage by file

Changed files (no unit tests)

Changed File Coverage Δ Total Covered Missed 🤖
github.com/enthus-appdev/atl-cli/internal/cmd/confluence/page/attachment.go 0.00% (ø) 163 (+6) 0 163 (+6)

Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code.

@Hinne1 Hinne1 merged commit 46ed1d7 into main Apr 14, 2026
12 checks passed
@Hinne1 Hinne1 deleted the claude/fix-download-all-exit-code branch April 14, 2026 05:32
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