Skip to content

feat(cot): Bug 2034520 improve json download and parsing error handling#791

Open
hneiva wants to merge 2 commits intomainfrom
hneiva/json-error
Open

feat(cot): Bug 2034520 improve json download and parsing error handling#791
hneiva wants to merge 2 commits intomainfrom
hneiva/json-error

Conversation

@hneiva
Copy link
Copy Markdown
Contributor

@hneiva hneiva commented Apr 23, 2026

On load_json_or_yaml:

load_json_or_yaml's default exception message now names the file when
is_path=True ("Failed to load json from /path/to/x.json: ..." instead
of just "Failed to load json: ..."). load_json_or_yaml_from_url passes
a custom message that also includes the (loggable) source URL alongside
the cache path.

Motivation: a chain-of-trust verification failure surfaced only the
JSONDecodeError offset, forcing a manual trace through several
concurrent downloads to identify the bad file. The extra context pins
the culprit immediately.

On load_json_or_yaml_from_url:

Combine download and parse into a single retry unit in
load_json_or_yaml_from_url: if the downloaded file fails to parse as
JSON/YAML, the cache is invalidated and retry_async triggers a fresh
download (up to the default 5 attempts). This turns transient artifact-
server glitches (e.g. GCS gunzip-transcoding returning 'Bad Request' with
200 OK, HTML error pages from hg, truncated responses) from hard failures
into self-healing retries.

download_file gains an optional expected_content_type kwarg that rejects
'text/html' responses when the caller asked for something else, failing
fast with a clearer error before writing a byte. DownloadError is reused
so retry_async picks it up if the HTML turns out to be transient.

The misleading 'overwrite' parameter semantics are preserved
(overwrite=True keeps any existing cached file; overwrite=False always
redownloads) and flagged with a comment for future cleanup.

Tests added for HTML rejection and the parse-failure-then-retry-succeeds
flow.

load_json_or_yaml's default exception message now names the file when
is_path=True ("Failed to load json from /path/to/x.json: ..." instead
of just "Failed to load json: ..."). load_json_or_yaml_from_url passes
a custom message that also includes the (loggable) source URL alongside
the cache path.

Motivation: a chain-of-trust verification failure surfaced only the
JSONDecodeError offset, forcing a manual trace through several
concurrent downloads to identify the bad file. The extra context pins
the culprit immediately.
@hneiva hneiva requested a review from a team as a code owner April 23, 2026 17:24
…lure

Combine download and parse into a single retry unit in
load_json_or_yaml_from_url: if the downloaded file fails to parse as
JSON/YAML, the cache is invalidated and retry_async triggers a fresh
download (up to the default 5 attempts). This turns transient artifact-
server glitches (e.g. GCS gunzip-transcoding returning 'Bad Request' with
200 OK, HTML error pages from hg, truncated responses) from hard failures
into self-healing retries.

download_file gains an optional expected_content_type kwarg that rejects
'text/html' responses when the caller asked for something else, failing
fast with a clearer error before writing a byte. DownloadError is reused
so retry_async picks it up if the HTML turns out to be transient.

The misleading 'overwrite' parameter semantics are preserved
(overwrite=True keeps any existing cached file; overwrite=False always
redownloads) and flagged with a comment for future cleanup.

Tests added for HTML rejection and the parse-failure-then-retry-succeeds
flow.
@hneiva hneiva force-pushed the hneiva/json-error branch from 331b32b to c5ca4ce Compare April 23, 2026 22:03
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