Skip to content

fix(CLI): restore empty-file download when a tag filter matches nothing - #1235

Merged
jablan merged 3 commits into
mainfrom
fix/cli-pull-empty-tag-filter-file
Aug 24, 2026
Merged

fix(CLI): restore empty-file download when a tag filter matches nothing#1235
jablan merged 3 commits into
mainfrom
fix/cli-pull-empty-tag-filter-file

Conversation

@jablan

@jablan jablan commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Summary

A client reported that PR #1085 broke phrase pull when a --tags filter matches zero translations: previously the CLI silently wrote an empty locale file in that case; after #1085 it fails with no content to write to ....

Root cause: the Phrase API returns 200 OK with an empty body when a filter matches nothing. The SDK's decode() leaves the *os.File return value nil in that case with no error. #1085's copyToDestination was hardened (as an "address expert review findings" cleanup, not in response to this specific case) to treat a nil file as a hard error — which is exactly the empty-export case, not a swallowed failure.

  • copyToDestination: treat a nil file as a valid empty export again, writing an empty file, with a comment explaining why nil isn't an error here.
  • Also fixed a related ordering bug in downloadSynchronously / PullParallel: the download cache (ETag/Last-Modified) was being persisted before copyToDestination ran, so a failed write could still get cached as if it succeeded, causing subsequent runs to report 304 Not Modified for a locale that was never actually written to disk.

Test plan

  • Added Go unit tests (cmd/internal/pull_parallel_test.go) covering: nil file writes an empty file, nil file truncates a previously-populated file (documents the accepted tradeoff), and a real file still copies content correctly.
  • Added a Ruby integration spec (spec/pull_spec.rb) that mocks the download endpoint returning 200 with an empty body for a --tags filter, in both sync and --parallel modes, asserting phrase pull exits 0 and writes an empty file.
  • Verified against the pre-fix binary that the new integration spec reproduces the client's exact reported error (no content to write to ...).
  • CI will build the CLI against the correct phrase-go version and run the full Go + Ruby suites (this environment has an unrelated, pre-existing phrase-go version mismatch that prevented a full local build).

🤖 Generated with Claude Code

…ring

PR #1085 made copyToDestination fail when the API returns an empty body,
which happens whenever a filter (e.g. --tags) matches zero translations.
That's a valid empty export, not a failed download, so treat it as one:
write an empty file instead of erroring.

Also fix the cache write ordering in downloadSynchronously and
PullParallel: the ETag/Last-Modified was being persisted before
copyToDestination ran, so a failed write could still poison the cache
into reporting 304 Not Modified for a locale that was never written.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

API changelog (oasdiff)

Doc-only edits (descriptions, examples) do not appear here.

No changes detected

jablan added 2 commits August 24, 2026 11:41
Redirect the openapi-generator/npm output from the generate step to a
log file that's only dumped on failure, and switch rspec to the
documentation formatter so CI logs show test names instead of dots.
@jablan
jablan merged commit d7d4344 into main Aug 24, 2026
14 checks passed
@jablan
jablan deleted the fix/cli-pull-empty-tag-filter-file branch August 24, 2026 12:40
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.

2 participants