fix: correctly map canister error calls - #11171
Open
nmattia wants to merge 1 commit into
Open
Conversation
This makes all the ic-cdk error mapping helpers exhaustive and ensures canister errors are reported correctly; for instance, a "insuffient cycles balance" error should _not_ be treated as a transient error. See code for exact behavior.
nmattia
changed the title
nmattia
marked this pull request as ready for review
August 18, 2026 10:07
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
|
✅ No security or compliance issues detected. Reviewed everything up to 85b6a9b. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to 85b6a9b. Security Overview
Detected Code Changes
|
mbjorkqvist
approved these changes
Aug 18, 2026
Contributor
I'm not sure about this: the corresponding error code is mapped to SysTransient in the protocol for asynchronous errors and the synchronous error is reported as the return value of 2 (SysTransient) of the corresponding system API; so I wouldn't remap the reject code in the helper. |
mraszyk
reviewed
Aug 18, 2026
| other => (RejectionCode::Unknown, other.to_string()), | ||
| // The callee replied, but the response could not be decoded into the | ||
| // expected type, so the callee did not honor its interface: a canister-side error. | ||
| CallError::CandidDecodeFailed(e) => (RejectionCode::CanisterError, e.to_string()), |
Contributor
There was a problem hiding this comment.
I'm not sure about this reject code: the following comment applies here, too - Neither of these produced a response from the callee, so there is no callee-assigned reject code to report; surface them as Unknown.
mraszyk
reviewed
Aug 18, 2026
mraszyk
left a comment
mraszyk
left a comment
Contributor
There was a problem hiding this comment.
The link https://internetcomputer.org/docs/references/ic-interface-spec#reject-codes does not work for me: use https://docs.internetcomputer.org/references/ic-interface-spec/https-interface/#reject-codes
There's also a reject code SYS_UNKNOWN (6) produced by the protocol which is different from Unknown meaning that the reject code could not be parsed.
daniel-wong-dfinity-org-twin
approved these changes
Aug 19, 2026
daniel-wong-dfinity-org-twin
left a comment
daniel-wong-dfinity-org-twin
left a comment
Contributor
There was a problem hiding this comment.
Sorry if I'm beating a dead horse, but I would like to again advocate for minimizing copy & paste.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This makes all the ic-cdk error mapping helpers exhaustive and ensures canister errors are reported correctly; for instance, a "insuffient cycles balance" error should not be treated as a transient error.
See code for exact behavior.