IC-1462 extend the error space for update requests#143
Conversation
✅ Deploy Preview for ic-interface-spec ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Co-authored-by: mraszyk <31483726+mraszyk@users.noreply.github.com>
Co-authored-by: mraszyk <31483726+mraszyk@users.noreply.github.com>
Co-authored-by: Alin Sinpalean <58422065+alin-at-dfinity@users.noreply.github.com>
|
The errors that we want to return as 200s in the future – were they returned as 202 or as 4xx/5xx? If we returned a 4xx/5xx before and now we signal errors through a 200 with a CBOR, isn't that a backward-incompatible change? Meaning: shouldn't we rather do this as an |
Yeah I think that's a good point. My understanding is that certain 5xx errors would become 200 + CBOR body, so we should make it in a backwards compatible way. If you think cc @rumenov for his opinion as the original proposer of the change. |
There was actually a breaking change ~6 months ago which moved 202 codes to 5xx/4xx. Now we want partially to move those 5xx/4xx to 200. Not sure if it worth doing 'api/v3/..' if you think we should do it. What is the correct way to do this ? I assume there must be some boundary node changes as well. In addition, I am not sure it is that bad of a breaking change. I more or less consider it as an extensions. Now callers need to handle 200 status code. Client handling of 5xx/4xx stays the same. |
rikonor
left a comment
rikonor
left a comment
There was a problem hiding this comment.
No objection from BN and Trust teams. 👍
|
@Dfinity-Bjoern, can you please take one more look We discussed the changes with the BN (@rikonor ), SDK (@sesi200 ), Trust (@nathanosdev) and we didn't flag any backward incompatible problems with the proposed changes. |
Co-authored-by: mraszyk <31483726+mraszyk@users.noreply.github.com>
|
@Dfinity-Bjoern are we waiting for anything else on this MR ? I think everything is resolved by now. |
We might want to wait with merging this until the code changes are ready to be merged. |
I thought the way to separate between an IC platform error and a canister owner error is that reject codes 1,2, and 3 are for IC platform errors and reject codes 4 and 5 are for canister owner errors. What additional benefits does this bring for separating between canister owner errors and IC platform errors? It seems there can still be an IC platform error - reject codes 1, 2, and 3 - in the read_state call, after this call returns 202, and the spec as written in this PR still allows for canister errors - reject codes 4 and 5 - to be returned from a 200 response. If there is an additional separation that this brings, should it be done for query calls too? |
|
First I want to clarify, that the spec change is only relevant when the canister_inspect_message fails. This said, today when I am not sure I completely understand
However, please not that the spec change is under the
The spec change actually adopts the exact same behaviour as what we have for query calls. |
|
To add a bit to Rosti's reply: I think there might be a confusion because of the wording used. The goal was to make it more clear when we have IC platform errors, in the sense of replica code hitting some unexpected error case (and which are important for the operators of the network but not end users) and IC protocol-level errors, i.e. errors that are intrinsic to IC behaviour that is captured by the reject codes in the spec (and which is more useful to end users). As Rosti explained, the change actually makes things more consistent to query calls by returning the IC protocol specific errors inside the body of the HTTP reply. |
|
Hi @rumenov, I see, thanks! Before there was no way to return the reject code when canister_inspect_message fails and now with this change there is a way. Cool! I was thinking that IC-platform-errors were the reject codes 1,2, and 3, and the canister-owner-errors were reject codes 4 and 5. Thanks @dsarlis for the clarification that we are talking about a separation between the IC-protocol-level errors which are the reject codes as a whole, and between the network-operator replica errors. Thank you both. |
chore: [NET-1402] bump IC-agent version to 0.24 closes NET-1402 This MR is needed in order to upgrade IC-agent to version 0.24. The new ic-agent version is needed as part of a larger work item, where we are planning on changing the response body of rejected call requests, see the according [interface spec change](dfinity/interface-spec#143). This MR also bumps the dependencies of `k256` and `p256` to `0.13.2` and adds them as workspace dependencies. The new version of `ic-agent` requires a type `SecretKey` in some of its APIs, where `SecretKey` is introduced in the newer versions of `k256` and `p256`. An example function of such as function is [from_private_key(private_key: SecretKey)](https://docs.rs/ic-agent/latest/ic_agent/identity/struct.Secp256k1Identity.html#method.from_private_key). Closes NET-1402 See merge request dfinity-lab/public/ic!12595
feature: [NET-1335] Implement spec change IC-1462 closes NET-1335 Implements the spec change [IC-1462](dfinity/interface-spec#143). Closes NET-1335 See merge request dfinity-lab/public/ic!12635
10 participants
More context can be found at https://dfinity.atlassian.net/browse/IC-1462.
To summarize, the current problem we are facing is that IC platform errors are mixed with canister owner errors which causes confusion among engineers how different errors should be handled.
This proposal is an attempt to have have a clear separation between canister owner errors and IC platform errors.