Skip to content

Surface Google Play download errors and exit non-zero on failure - #245

Open
minner-fun wants to merge 1 commit into
EFForg:masterfrom
minner-fun:fix/google-play-silent-failures
Open

Surface Google Play download errors and exit non-zero on failure#245
minner-fun wants to merge 1 commit into
EFForg:masterfrom
minner-fun:fix/google-play-silent-failures

Conversation

@minner-fun

Copy link
Copy Markdown

The problem

Every error message in the Google Play download path is silently discarded whenever apkeep runs without a TTY on stderr — piped, redirected, or under CI, cron, or Docker without -t. The process also exits 0 regardless, so a caller cannot detect the failure at all.

Reproducing, before this change:

$ apkeep -a com.mojang.minecraftpe -d google-play -e ... -t ... ./out 2>&1
Downloading com.mojang.minecraftpe...
$ echo $?
0

No error, no file, no non-zero exit. The same happens for a nonexistent package, a permission error, or three exhausted retries.

Why it happens

MultiProgress::println() writes through indicatif's draw target. ProgressDrawTarget::term() returns Self::hidden() when stderr is not a color-capable terminal, and a hidden target makes width() return None, which makes draw() return Ok(()) before emitting anything. The message is dropped, and because the result is Ok, the .unwrap() at each call site does not panic either.

The two success messages were unaffected only because they go through suspend(), which is documented to run its closure even when the draw target is hidden. Every one of the seven error messages used println().

The change

  • Route the error messages through suspend() + eprintln!. They now survive in non-interactive contexts and stay on stderr, which is where the draw target had been sending them all along.
  • Propagate a failure count out of download_apps() and exit non-zero when it is greater than zero. An already-present file counts as success; unavailable apps, permission errors, exhausted retries, and version-pinned requests count as failures.
  • Reword the InvalidApp message. The API returns that same error kind for several conditions that are indistinguishable in the response, so naming just one of them is misleading.

After

Case Before After
Paid / unavailable app silent, exit 0 message, exit 1
Nonexistent package silent, exit 0 message, exit 1
File already exists silent, exit 0 message, exit 0
Version pinned (app@1.2.3) silent, exit 0 message, exit 1
Successful download works works, exit 0

Testing

Built with cargo build --release (no new warnings) and ran each row of the table above against a live Google Play account. Also confirmed that 2>/dev/null leaves only the Downloading ... line on stdout, so the stdout/stderr split is intact, and that the progress bars still render normally in a terminal.

The same println() pattern exists in the APKPure, F-Droid, and Huawei App Gallery sources. This PR deliberately leaves those alone to stay reviewable; happy to follow up with the same treatment if you would like it in a separate change.

🤖 Generated with Claude Code

`MultiProgress::println()` routes through indicatif's draw target, and
`ProgressDrawTarget::term()` swaps in a hidden target whenever stderr is
not a color-capable terminal. A hidden target drops the message and
still returns `Ok`, so every Google Play error was discarded without a
trace whenever output was piped, redirected, or run under CI, cron, or
Docker without a TTY. The success messages were unaffected only because
they go through `suspend()`, which runs its closure even when the draw
target is hidden.

Route the error messages through `suspend()` + `eprintln!` so they
survive in non-interactive contexts while staying on stderr, where the
draw target had been sending them all along.

Also propagate a failure count out of `download_apps()` so that a failed
run exits non-zero, which lets callers detect failures at all. An
already-present file counts as success; unavailable apps, permission
errors, exhausted retries, and version-pinned requests count as
failures.

Finally, reword the `InvalidApp` message: the API returns that same kind
for several conditions that cannot be told apart from the response --
the app may be paid, nonexistent, restricted to particular accounts,
region-locked, or incompatible with the selected device.

Co-Authored-By: Claude Fable 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