Skip to content

Make file download fail-safe and retry on transient errors#131

Open
jimexist wants to merge 1 commit into
simplelocalize:masterfrom
jimexist:jiayu/goofy-margulis-418e5b
Open

Make file download fail-safe and retry on transient errors#131
jimexist wants to merge 1 commit into
simplelocalize:masterfrom
jimexist:jiayu/goofy-margulis-418e5b

Conversation

@jimexist

@jimexist jimexist commented Jul 6, 2026

Copy link
Copy Markdown

What

Fixes a crash when a translation file download fails, and adds bounded retry for transient download errors.

Why

A failed download (e.g. an expired or forbidden S3 link) crashed the CLI with an unhelpful stack trace instead of a proper error message:

java.lang.IllegalArgumentException: json string can not be null or empty
	at com.jayway.jsonpath.internal.Utils.notEmpty(Utils.java:401)
	at com.jayway.jsonpath.internal.ParseContextImpl.parse(ParseContextImpl.java:36)
	at io.simplelocalize.cli.client.SimpleLocalizeClient.throwOnError(SimpleLocalizeClient.java:201)
	at io.simplelocalize.cli.client.SimpleLocalizeClient.downloadFile(SimpleLocalizeClient.java:133)
	...

Root cause: for file downloads the response body is a Path (from BodyHandlers.ofFile), which safeCastHttpBodyToString turned into "", and JsonPath.parse("") throws before SUPPRESS_EXCEPTIONS can apply. The same crash affected any endpoint returning an error with an empty body.

Changes

  • throwOnError never crashes on unparseable bodies: error-message extraction skips blank bodies and catches any JsonPath failure, falling back to Unknown error, HTTP Status: <code>.
  • Path bodies are read for error messages: failed downloads now surface the server's actual error message (e.g. link expired) instead of a generic one.
  • No error payload left on disk: BodyHandlers.ofFile(..., TRUNCATE_EXISTING) writes the error response to the target translation file on failure; it is now deleted instead of being left behind masquerading as a translation file.
  • Downloads retry on transient errors: up to 3 attempts with linear backoff (1s, 2s) on IOException, HTTP 429, and 5xx. Other 4xx (403/404) fail fast since retrying can't help.

Notes for reviewers

  • Existing behavior for permanent errors is unchanged: an ApiRequestException with the extracted (or fallback) message.
  • One known pre-existing limitation not addressed here: a failed download still truncates a pre-existing local file before the failure is detected. Downloading to a temp file and moving on success would fix that; happy to do it in a follow-up.

Tests

Added coverage in SimpleLocalizeClientTest (mock-server based):

  • download fails with empty body → clean ApiRequestException, no leftover file (the reported crash)
  • download fails with JSON error body → message extracted from $.msg
  • server returns 500 twice then 200 → download succeeds on the third attempt with correct content
  • server returns 503 three times → gives up after max attempts, no leftover file

Full suite passes: 88 tests, 0 failures.

🤖 Generated with Claude Code

A failed download crashed with IllegalArgumentException ("json string
can not be null or empty") because throwOnError passed an empty string
to JsonPath: for file downloads the response body is a Path, which
safeCastHttpBodyToString turned into "".

- Skip error-message parsing when the body is blank and catch any
  JsonPath failure, falling back to "Unknown error, HTTP Status: <code>"
- Read Path response bodies so download errors surface the server's
  actual error message
- Delete the file written by the ofFile body handler on non-200
  responses instead of leaving the error payload as a translation file
- Retry downloads up to 3 times with linear backoff on IOException,
  HTTP 429 and 5xx; fail fast on other 4xx

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jpomykala

Copy link
Copy Markdown
Contributor

Hi @jimexist , thanks for the PR. I will review and merge it later this week. 😊

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