Fix detection of when to retry c2c calls by hpeebles · Pull Request #9106 · open-chat-labs/open-chat

hpeebles · 2026-07-20T13:12:47Z

No description provided.

…n message

Copilot

Pull request overview

This PR refactors “should we retry this canister-to-canister (c2c) failure?” logic to avoid parsing replica-provided reject messages, instead carrying an explicit retry policy alongside C2CError and updating call sites to use it.

Changes:

Introduce C2CRetryPolicy and embed it into types::C2CError; add helpers to construct/read it.
Update utils::canister::delay_if_should_retry_failed_c2c_call to accept &C2CError and drive behavior from retry_policy.
Update many batch/job call sites and canister CHANGELOGs; add unit tests for retry policy/delay mapping.

Reviewed changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
backend/libraries/utils/src/canister/mod.rs	Adds retry-policy derivation from CDK errors, updates retry-delay helper, and adds unit tests.
backend/libraries/types/src/error.rs	Adds `C2CRetryPolicy` and stores it inside `C2CError`.
backend/libraries/storage_bucket_client/src/lib.rs	Updates retry detection call to pass `&C2CError`.
backend/libraries/fire_and_forget_handler/src/lib.rs	Updates retry detection call to pass `&C2CError`.
backend/canisters/user/impl/src/model/user_canister_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/user/impl/src/model/local_user_index_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/user/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/user_index/impl/src/model/storage_index_users_to_remove_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/user_index/impl/src/model/storage_index_user_config_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/user_index/impl/src/model/group_index_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/user_index/impl/src/jobs/sync_events_to_local_user_index_canisters.rs	Updates retry detection to use `&C2CError`.
backend/canisters/user_index/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/storage_index/impl/src/model/bucket_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/storage_index/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/storage_bucket/impl/src/model/index_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/storage_bucket/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/proposals_bot/impl/src/jobs/update_finished_proposals.rs	Updates retry detection to use `&C2CError`.
backend/canisters/proposals_bot/impl/src/jobs/retrieve_proposals.rs	Updates retry detection to use `&C2CError`.
backend/canisters/proposals_bot/impl/src/jobs/push_proposals.rs	Updates retry detection to use `&C2CError`.
backend/canisters/proposals_bot/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/online_users/impl/src/model/airdrop_bot_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/online_users/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/notifications_index/impl/src/model/local_index_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/notifications_index/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/local_user_index/impl/src/model/user_index_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/local_user_index/impl/src/model/user_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/local_user_index/impl/src/model/group_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/local_user_index/impl/src/model/community_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/local_user_index/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/group/impl/src/model/user_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/group/impl/src/model/local_user_index_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/group/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/group_index/impl/src/model/local_index_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/group_index/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/escrow/impl/src/jobs/notify_status_change.rs	Updates retry detection to use `&C2CError`.
backend/canisters/escrow/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/community/impl/src/model/user_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/community/impl/src/model/local_user_index_event_batch.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/community/CHANGELOG.md	Notes retry-detection fix.
backend/canisters/airdrop_bot/impl/src/actions.rs	Updates retry delay calculation to use `&C2CError`.
backend/canisters/airdrop_bot/CHANGELOG.md	Notes retry-detection fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

github-actions · 2026-07-20T13:21:24Z

`canbench` 🏋 (dir: .)

./canbench_results.yml is up to date ✅

---------------------------------------------------

Benchmark: add_reactions
  total:
    instructions: 1.38 B (0.00%) (change within noise threshold)
    heap_increase: 5 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

Benchmark: push_simple_text_messages
  total:
    instructions: 210.33 M (0.02%) (change within noise threshold)
    heap_increase: 12 pages (no change)
    stable_memory_increase: 0 pages (no change)

---------------------------------------------------

clippy's `items_after_test_module` lint requires the `#[cfg(test)]` module to come after all other items in the file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Resolved CHANGELOG conflicts by moving the #9106 entries into the new `[unreleased]` sections, since master released the versions they had been sitting under. `storage_index`'s `vault_event_batch.rs` is new on master and called the old two-argument `delay_if_should_retry_failed_c2c_call`, so it merged cleanly but did not compile. Updated it to pass the `C2CError`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

`retry_policy` was only reachable via `convert_cdk_error`, which is used solely for management canister calls. Every generated c2c client goes through `make_c2c_call_raw`, which flattened the CDK error itself and built the `C2CError` with `C2CError::new` - so the policy fell back to `from_reject_code`, which maps everything other than `DestinationInvalid` and `CanisterReject` to `RetryImmediately`. That meant none of the consumers of `delay_if_should_retry_failed_c2c_call` saw the new policy. A callee which is out of cycles was still retried every round, and our own liquid cycle balance being too low regressed from a five minute backoff to an immediate retry - the one case the old message based check got right, since it matched the CDK's own `Display` string rather than a replica supplied IC code. Move the conversion into `types` as `C2CError::from_cdk_error` so both paths share it, and have `make_c2c_call_raw` use it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Three fixes on top of the shared conversion: Encoding failures are no longer retried. Serializing our own args, or deserializing the callee's response, fails the same way however many times we retry - the types disagree, or one side has a bug - but these were built with `RejectCode::CanisterError`, which `from_reject_code` maps to `RetryImmediately`, so a type mismatch re-sent every round forever. They now carry `DoNotRetry`, matching how the CDK's own `CandidDecodeFailed` is treated. The timer job queues now drop items after 50 consecutive failures. They re-queued on any `Err(Some(_))` with no cap, unlike `FireAndForgetHandler`, so a permanently broken callee pinned its items for good and the queue only grew. Now that the IC no longer lets us pick the permanent failures out up front, the retries have to be bounded instead. Counted per key in the grouped queue, so one broken callee cannot cause another's items to be dropped, and reset by any success. `is_target_canister_uninstalled_or_deleted` matched on `IC0537` in the reject message, but that code is only ever sent to the frontend, so the branch never fired and an uninstalled ledger was never detected. Match on the reject text the replica actually sends ("...contains no Wasm module.") as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 49 out of 50 changed files in this pull request and generated 1 comment.

Counting failures across the queue as a whole isn't right - the items are independent, so one failing item's attempts count towards dropping a different one. Leaving `TimerJobQueue` as it was until there is a per item way to do this. The cap stays on `GroupedTimerJobQueue`, where failures are counted per key and the key is the callee. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Dropping events after N failures is the wrong trade. If something does get stuck in a loop we want to fix the cause and have the queued events replayed, not find they were discarded while we were working out what was wrong. Events should only be dropped when we know they can never succeed, and that is what the retry policy is for - `DoNotRetry` already discards them at the point we can prove it. Anything we cannot prove keeps retrying with a delay. This also takes `tracing` and the `msgpack` dev dependency back out, since both were only needed for the cap. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

A stopped canister is rejected with `CanisterError`, which we can't tell apart from the callee trapping, so it was waiting the full five minutes. But canisters are stopped whenever they are upgraded, so this stalled event delivery to a canister every time it was upgraded - which is what the integration tests were catching: reject_code=CanisterError policy=RetryAfterDelay message=Canister mzsit-hx777-77775-qaaba-cai is stopped The reject message does distinguish this case. Reading it is fair game in a way that reading an IC error code out of it is not - it is the replica's own text for the rejection, not a code the IC only ever sends to the frontend. A trapping callee still backs off, and so does one which is out of cycles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Retrying every round while a canister is upgrading just spins against a callee we already know to be unavailable, so give the stopped case its own delay rather than lumping it in with `RetryImmediately`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The `with_c2c_error` cases advanced the clock and only then restarted the canister, so the retry fell due while it was still stopped and was spent on another failure. That worked while a stopped callee was retried every round, but not now it has its own delay. Matches the ordering `send_message_with_transfer_to_group_succeeds` in the same file already uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Fix delay_if_should_retry_failed_c2c_call now that error code not i…

bc62463

…n message

hpeebles requested review from ivan-jukic, julianjelfs and megrogan as code owners July 20, 2026 13:12

Update CHANGELOGs

31fffc2

hpeebles requested a review from Copilot July 20, 2026 13:16

Copilot started reviewing on behalf of hpeebles July 20, 2026 13:16 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

Comment thread backend/libraries/types/src/error.rs

hpeebles and others added 4 commits July 23, 2026 14:39

Move test module below the remaining items to satisfy clippy

65a2cb5

clippy's `items_after_test_module` lint requires the `#[cfg(test)]` module to come after all other items in the file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

hpeebles requested a balanced review from Copilot August 11, 2026 14:10

Copilot started reviewing on behalf of hpeebles August 11, 2026 14:11 View session

Copilot AI reviewed Aug 11, 2026

View reviewed changes

Comment thread backend/libraries/timer_job_queues/src/timer_job_queue.rs Outdated

hpeebles and others added 4 commits August 11, 2026 15:23

julianjelfs approved these changes Aug 11, 2026

View reviewed changes

hpeebles merged commit f599640 into master Aug 11, 2026
8 checks passed

hpeebles deleted the c2c_errors branch August 11, 2026 16:19

Navigation Menu

Uh oh!

Fix detection of when to retry c2c calls - #9106

Fix detection of when to retry c2c calls#9106
hpeebles merged 11 commits into
masterfrom
c2c_errors

hpeebles commented Jul 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

github-actions Bot commented Jul 20, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

hpeebles commented Jul 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

github-actions Bot commented Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

canbench 🏋 (dir: .)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 20, 2026 •

edited

Loading

`canbench` 🏋 (dir: .)