-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
An experimental System API extension to expose error codes to canisters #310
base: master
Are you sure you want to change the base?
Conversation
@@ -1334,7 +1334,8 @@ The following sections describe various System API functions, also referred to a | |||
ic0.msg_reject_code : () -> i32; // Ry Rt CRy CRt | |||
ic0.msg_reject_msg_size : () -> i32; // Rt CRt | |||
ic0.msg_reject_msg_copy : (dst : i32, offset : i32, size : i32) -> (); // Rt CRt | |||
|
|||
ic0.msg_error_code_size : () -> i32; // Rt CRt | |||
ic0.msg_error_code_copy : (dst : i32, offset : i32, size : i32) -> (); // Rt CRt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is consistent with what we provide over the read_state
API. But I expect that any users of this would anyway parse the numeric code from ICXXX
, so we might as well just do an ic0.msg_error_code : () -> i32
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think using just the numeric code would be compatible with the spec, which writes:
The specification reserves error codes matching the regular expression IC[0-9]+ (e.g., IC502) for the DFINITY implementation of the API.
So applications should expect error codes that look different.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I'm aware numeric codes would be inconsistent with the read_state
, but I wouldn't say it's incompatible, as this is a new API so we can do whatever we want here. I guess I'm just questioning why we went with IC[0-9]+
instead of just a number, I don't remember the rationale.
I'm also not sure what the canister would do with the error code. Just print it? In that case it doesn't matter what it is and we might as well keep the IC
part. But if the code is used for anything else, I suspect the canister first needs to parse IC[0-9]+
to retrieve the numeric code, in which case we might as well provide just the numeric code in the first place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember the rationale
I don't know either, but maybe in the future, there could be a new system API allowing a canister to trap with its own error code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still find it inconsistent to report the same value as a different type in two different places – I feel we would regret this at some point when we extend the scheme to other (e.g. canister-specified) error codes.
Maybe we can start by asking (internal) teams what they prefer?
- `ic0.error_code_size : () → i32` and `ic0.error_code_copy : (dst : i32, offset : i32, size : i32) → ()` | ||
|
||
The error code. Traps if there is no reject message (i.e. if `reject_code` is `0`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `ic0.error_code_size : () → i32` and `ic0.error_code_copy : (dst : i32, offset : i32, size : i32) → ()` | |
The error code. Traps if there is no reject message (i.e. if `reject_code` is `0`). | |
- `ic0.msg_error_code_size : () → i32` and `ic0.msg_error_code_copy : (dst : i32, offset : i32, size : i32) → ()` | |
The error code. |
I'm making this PR a draft PR. |
Currently this PR is an idea scratchpad on how error codes could be exposed to canisters (in addition to the API users).