fix(nervous-system): log GitHub API status/body on failure in sync-with-released-nervous-system-wasms by claude[bot] · Pull Request #10802 · dfinity/ic

claude · 2026-07-17T08:56:44Z

Requested by Daniel Wong · Slack thread

Summary

The CI job "Update mainnet canister versions file" fails intermittently and, when it does, gives no diagnostic information about why.

Before: in rs/nervous_system/tools/sync-with-released-nervous-system-wasms/src/main.rs, get_mainnet_canister_release() fetched the GitHub tags list and immediately called .json() on the response with no status check. Any non-success response, or a malformed/empty body, would fail JSON decoding and kill the whole process with an unhelpful error like error decoding response body ... expected value at line 1 column 1. The two asset-download calls in the same file had a similar gap, and Tag::release_for_canister() checked status but discarded the body on decode failure.

After: before decoding a response as JSON (or, for the asset downloads, before reading the bytes/text), the code checks response.status() and reads the raw body. If the status isn't success, or the body subsequently fails to decode as JSON, the returned error includes the actual HTTP status code and the response body (truncated to 2000 chars). This turns a future occurrence of this bug into a diagnosable log line instead of a bare JSON-decode error.

Note: an earlier version of this PR also added a retry-with-backoff loop around these calls, using the backoff crate. Per review feedback, that was removed — the CI step that invokes this binary already retries the whole process (up to 10 attempts with shell-level backoff), so a second retry layer inside the binary was redundant. This PR now only adds the diagnostic status/body logging described above; no new dependency, no retry loop.

How

Added two small helpers to main.rs:

truncate_body(body) -> String: truncates a response body to 2000 chars for inclusion in error/log messages.
decode_json_response<T>(url, response) -> Result<T>: reads the response status and body once, returns an error containing the status + (truncated) body if the status isn't success or if the body fails to decode as JSON, and otherwise decodes and returns T.

Applied this to the same four call sites as before: the tags-list fetch in get_mainnet_canister_release() (the reported bug), Tag::release_for_canister(), and the two asset-download calls in ReleaseAsset::sha256()/ReleaseAsset::text(). release_for_canister() keeps its existing behavior of returning Ok(None) for a non-success status (e.g. 404 for a tag with no release) — the new diagnostics only kick in once we get to decoding the body.

No dependency changes: the backoff crate addition to Cargo.toml/BUILD.bazel from the earlier version of this PR has been reverted; this crate has no new dependencies.

Verification

This is a shallow clone in a sandboxed environment, so a full cargo check/bazel build of the workspace isn't possible here (the workspace has a private git dependency, dfinity-lab/build-info, that isn't reachable from this sandbox — unrelated to this change). Instead:

Carefully reviewed the diff for type/borrow correctness and to confirm the Ok(None)-on-404 behavior of release_for_canister() is unchanged.
Extracted the new helper functions and call sites into an isolated scratch crate pinned to compatible reqwest/anyhow/serde/serde_json/sha2/tokio versions, and confirmed it compiles cleanly with cargo build (no errors or warnings).

🤖 Generated with Claude Code

Generated by Claude Code

…-released-nervous-system-wasms The "Update mainnet canister versions file" CI job intermittently fails because get_mainnet_canister_release() calls .json() on the GitHub tags-list response with no status check and no retry: any non-success or malformed response body fails JSON decoding and kills the whole process with an unhelpful "expected value at line 1 column 1" error. Add get_with_retry()/get_json_with_retry() helpers that check the response status, retry transient failures (server errors, rate limiting, network errors) with exponential backoff via the `backoff` crate, and log the status code and response body on each failed attempt. Apply them to the tags-list fetch (the reported bug), the release lookup, and the two asset-download calls in the same file, which had the same unguarded pattern.

…iew feedback The CI step that invokes this binary already retries the whole process up to 10 times with shell-level backoff, so adding a second retry-with-backoff layer inside the binary (via the new get_with_retry()/get_json_with_retry() helpers and the backoff crate) was redundant. Drop the backoff dependency and the retry loops, but keep the actual fix: before decoding a GitHub API response as JSON, check response.status() and read the raw body, and if the status isn't success or the body fails to decode, return an error that includes the HTTP status code and the (body, truncated to 2000 chars) response text. This turns a future occurrence of this bug into a diagnosable log line instead of a generic "expected value at line 1 column 1" error, without adding any new retry logic or dependencies. Applied to the same four call sites as before: the tags-list fetch in get_mainnet_canister_release(), Tag::release_for_canister(), and the two asset-download calls in ReleaseAsset::sha256()/text(). The existing Ok(None)-on-404 behavior of release_for_canister() is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFbd4J2BnNBNGeEVKpdm6B

daniel-wong-dfinity-org-twin · 2026-07-17T08:59:00Z

+fn github_api_backoff_policy() -> backoff::ExponentialBackoff {
+    backoff::ExponentialBackoff {
+        initial_interval: Duration::from_secs(1),
+        max_interval: Duration::from_secs(30),


Make this 5 minutes.

daniel-wong-dfinity-org-twin · 2026-07-17T08:59:21Z

+        initial_interval: Duration::from_secs(1),
+        max_interval: Duration::from_secs(30),
+        multiplier: 2.0,
+        max_elapsed_time: Some(Duration::from_secs(120)),


Make this 30 minutes.

daniel-wong-dfinity-org-twin · 2026-07-17T08:59:58Z

    Ok((client, token))
 }

+fn github_api_backoff_policy() -> backoff::ExponentialBackoff {


Use lazy_static instead.

daniel-wong-dfinity-org-twin · 2026-07-17T09:51:43Z

+fn truncate_body(body: &str) -> String {
+    if body.len() <= MAX_LOGGED_BODY_LEN {
+        return body.to_string();
+    }
+    let truncated: String = body.chars().take(MAX_LOGGED_BODY_LEN).collect();
+    format!(
+        "{truncated}... <truncated, full response body was {} bytes>",
+        body.len()
+    )
+}


Use https://sourcegraph.com/r/github.com/dfinity/ic/-/blob/rs/nervous_system/string/src/lib.rs?L16 instead.

daniel-wong-dfinity-org-twin · 2026-07-17T09:52:32Z

+        let status = res.status();

        // not every tag must have a release so we do not report an error if it does not
-        if !res.status().is_success() {
+        if !status.is_success() {


This seems orthogonal. Please, back out.

daniel-wong-dfinity-org-twin · 2026-07-17T09:56:14Z

+///
+/// This function does not retry: the CI step that invokes this binary already retries the whole
+/// process several times with backoff, so retrying here as well would be redundant.
+async fn decode_json_response<T: serde::de::DeserializeOwned>(


This is a really basic helper -> move it to the end.

daniel-wong-dfinity-org-twin · 2026-07-17T09:57:23Z

+/// This function does not retry: the CI step that invokes this binary already retries the whole
+/// process several times with backoff, so retrying here as well would be redundant.
+async fn decode_json_response<T: serde::de::DeserializeOwned>(
+    url: &str,


Is this just for diagnostics? If so, it should be last. Also, say that this is just for diagnostic purposes.

daniel-wong-dfinity-org-twin · 2026-07-17T10:15:08Z

+/// If the response status is not a success, or if the body fails to decode as JSON (which is
+/// exactly what happens when the GitHub API responds with something other than the expected
+/// JSON, e.g., a rate-limiting or transient error page), this returns an error that includes the
+/// HTTP status code and the (possibly truncated) response body, so that a failure is diagnosable
+/// from the CI logs instead of just showing a generic "expected value at line 1 column 1" error.


Actually, decode failure that we've been looking at could just be that GitHub's response did not fit the shape that we expected. It is possible that the response was something like

{ "error": "our servers are melting right now", "code": 503 }

In any case, what your comment is basically saying is, "Err gets returned when something bad happens", and "we include breadcrumbs". These really go without saying. breadcrumbs is SUPPOSED to be a STANDARD feature, like seat belts. No car ad brags about seat belts, because lack of seat belts is a straight up DEFECT.

daniel-wong-dfinity-org-twin · 2026-07-17T10:16:10Z

+/// This function does not retry: the CI step that invokes this binary already retries the whole
+/// process several times with backoff, so retrying here as well would be redundant.


Nobody would think that this retries.

daniel-wong-dfinity-org-twin · 2026-07-17T10:19:03Z

+    )
+}
+
+/// Reads the body of `response` (which was fetched from `url`) and decodes it as JSON into `T`.


I think the name already explains this.

What we might want to say is that this provides more diagnostics.

Actually, instead of replacing json, maybe what we can do is something like

.json() // as before .map_err(add_diagnostics)?

…ents Per Daniel Wong's review on PR #10802: - Reuse ic_nervous_system_string::clamp_string_len instead of a hand-rolled truncate_body helper. - Back out the incidental res.status() hoisting in Tag::release_for_canister and ReleaseAsset::{sha256,text} that was unrelated to the diagnostics fix; the status/body checking now goes through one small `ensure_success` helper (and `decode_json_response`, which now delegates to it), so each call site is back to roughly as concise as it was before this feature was added, instead of hand-rolling the status+body+error plumbing at every call site. - Trimmed decode_json_response's doc comment: dropped the parts that just restate what the name/type signature already say ("decodes as JSON", "returns Err on failure") and the now-inapplicable note about retries (there is no retry logic in this file). - Moved the two diagnostic-only helpers (ensure_success, decode_json_response) to the end of the file and labeled them as diagnostic-only. Note: three earlier review comments (use lazy_static; make a duration 5 minutes; make a duration 30 minutes) were anchored to commit b87459f, which had the retry/backoff mechanism that was already removed in 7e2f89c ("drop redundant retry layer") in response to separate feedback. GitHub marks those three threads as outdated/no-longer-applicable to the current diff, so no action was taken for them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DFbd4J2BnNBNGeEVKpdm6B

Addresses review comment #10802 (comment): ReleaseAsset::sha256() and ReleaseAsset::text() inlined an identical client/token/get/bearer_auth/send/ensure_success sequence before diverging on .bytes() vs .text(). Factor that into a fetch(url) helper (client+token via github_api_client_and_token(), GET with bearer auth, then ensure_success), and have both methods call it. Also apply the same helper to the tags-list GET in get_mainnet_canister_release() for consistency, composing it with decode_json_response as "fetch, then decode". Tag::release_for_canister() keeps its manual get+status check since it deliberately treats a non-success status as "no release" rather than an error.

github-actions Bot added the fix label Jul 17, 2026

IDX GitHub Automation and others added 2 commits July 17, 2026 08:57

Automatically updated Cargo*.lock

b87459f

claude Bot changed the title ~~fix(nervous-system): retry GitHub API calls with backoff in sync-with-released-nervous-system-wasms~~ fix(nervous-system): log GitHub API status/body on failure in sync-with-released-nervous-system-wasms Jul 17, 2026

daniel-wong-dfinity-org-twin requested changes Jul 17, 2026

View reviewed changes

claude added 2 commits July 17, 2026 10:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(nervous-system): log GitHub API status/body on failure in sync-with-released-nervous-system-wasms#10802

fix(nervous-system): log GitHub API status/body on failure in sync-with-released-nervous-system-wasms#10802
claude[bot] wants to merge 5 commits into
masterfrom
fix/sync-nervous-system-wasms-github-retry

claude Bot commented Jul 17, 2026 •

edited

Loading

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

daniel-wong-dfinity-org-twin Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		/// This function does not retry: the CI step that invokes this binary already retries the whole
		/// process several times with backoff, so retrying here as well would be redundant.

Uh oh!

Conversation

claude Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How

Verification

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented Jul 17, 2026 •

edited

Loading