Skip to content

Commit

Permalink
Merge branch 'alin/fix-stopped-error-code' into 'master'
Browse files Browse the repository at this point in the history 
fix: Consolidate mapping of `StateError` to `ErrorCode` / `RejectCode`

Have a single source of truth for mapping `StateError` to `ErrorCode` (and from
that into a `RejectCode`). This ensures consistency with the `ErrorCodes`
produced by execution (e.g. so the same error code is output when attempting to
enqueue a request on a stopped canister as when trying to execute an already
enqueued one). 

See merge request dfinity-lab/public/ic!16305
  • Loading branch information
@alin-at-dfinity
alin-at-dfinity committed Nov 24, 2023
2 parents 4ac5712 + 51b2a96 commit 8c9e85f
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 51 deletions.
4 changes: 2 additions & 2 deletions packages/pocket-ic/src/lib.rs
View file
Expand Up @@ -935,7 +935,7 @@ pub enum TryFromError {
pub enum ErrorCode {
SubnetOversubscribed = 101,
MaxNumberOfCanistersReached = 102,
CanisterOutputQueueFull = 201,
CanisterQueueFull = 201,
IngressMessageTimeout = 202,
CanisterQueueNotEmpty = 203,
IngressHistoryFull = 204,
Expand Down Expand Up @@ -991,7 +991,7 @@ impl TryFrom<u64> for ErrorCode {
match err {
101 => Ok(ErrorCode::SubnetOversubscribed),
102 => Ok(ErrorCode::MaxNumberOfCanistersReached),
201 => Ok(ErrorCode::CanisterOutputQueueFull),
201 => Ok(ErrorCode::CanisterQueueFull),
202 => Ok(ErrorCode::IngressMessageTimeout),
203 => Ok(ErrorCode::CanisterQueueNotEmpty),
204 => Ok(ErrorCode::IngressHistoryFull),
Expand Down
2 changes: 1 addition & 1 deletion rs/execution_environment/src/history.rs
View file
Expand Up @@ -248,7 +248,7 @@ fn dashboard_label_value_from(code: ErrorCode) -> &'static str {
CanisterContractViolation => "Canister Contract Violation",
CanisterInvalidWasm => "Canister Invalid WASM",
CanisterDidNotReply => "Canister Did Not Reply",
CanisterOutputQueueFull => "Canister Output Queue Full",
CanisterQueueFull => "Canister Queue Full",
CanisterQueueNotEmpty => "Canister Queues Not Empty",
CanisterOutOfMemory => "Canister Out Of Memory",
CanisterStopped => "Canister Stopped",
Expand Down
19 changes: 4 additions & 15 deletions rs/messaging/src/routing/stream_handler.rs
View file
Expand Up @@ -2,7 +2,7 @@ use crate::message_routing::LatencyMetrics;
use ic_base_types::NumBytes;
use ic_certification_version::CertificationVersion;
use ic_config::execution_environment::Config as HypervisorConfig;
use ic_error_types::RejectCode;
use ic_error_types::{ErrorCode, RejectCode};
use ic_logger::{debug, error, fatal, trace, ReplicaLogger};
use ic_metrics::{
buckets::{add_bucket, decimal_buckets},
Expand Down Expand Up @@ -644,8 +644,9 @@ impl StreamHandlerImpl {
// Critical error, responses should always be inducted successfully.
error!(
self.log,
"{}: Inducting response failed: {:?}",
"{}: Inducting response failed: {} {:?}",
CRITICAL_ERROR_INDUCT_RESPONSE_FAILED,
err,
response
);
self.metrics.critical_error_induct_response_failed.inc()
Expand Down Expand Up @@ -885,19 +886,7 @@ fn generate_reject_response(

/// Maps a `StateError` resulting from a failed induction to a `RejectCode`.
fn reject_code_for_state_error(err: &StateError) -> RejectCode {
match err {
StateError::QueueFull { .. } => RejectCode::SysTransient,
StateError::CanisterOutOfCycles(_) => RejectCode::SysTransient,
StateError::CanisterNotFound(_) => RejectCode::DestinationInvalid,
StateError::CanisterStopped(_) => RejectCode::CanisterReject,
StateError::CanisterStopping(_) => RejectCode::CanisterReject,
StateError::InvariantBroken(_) => RejectCode::SysTransient,
StateError::UnknownSubnetMethod(_) => RejectCode::CanisterReject,
StateError::NonMatchingResponse { .. } => RejectCode::SysFatal,
StateError::InvalidSubnetPayload => RejectCode::CanisterReject,
StateError::OutOfMemory { .. } => RejectCode::SysTransient,
StateError::BitcoinNonMatchingResponse { .. } => RejectCode::SysTransient,
}
ErrorCode::from(err).into()
}

/// Ensures that the given signals are valid (strictly increasing, before
Expand Down
4 changes: 2 additions & 2 deletions rs/messaging/src/routing/stream_handler/tests.rs
View file
Expand Up @@ -632,7 +632,7 @@ fn induct_loopback_stream_with_memory_limit_impl(
let msg = loopback_stream.messages().iter().nth(1).unwrap().1;
expected_loopback_stream.push(generate_reject_response(
msg.clone(),
RejectCode::SysTransient,
RejectCode::CanisterError,
StateError::OutOfMemory {
requested: NumBytes::new(MAX_RESPONSE_COUNT_BYTES as u64),
available: MAX_RESPONSE_COUNT_BYTES as i64 / 2,
Expand Down Expand Up @@ -2420,7 +2420,7 @@ fn induct_stream_slices_with_memory_limit_impl(
expected_stream.increment_signals_end();
expected_stream.push(generate_reject_response(
request1,
RejectCode::SysTransient,
RejectCode::CanisterError,
StateError::OutOfMemory {
requested: NumBytes::new(MAX_RESPONSE_COUNT_BYTES as u64),
available: MAX_RESPONSE_COUNT_BYTES as i64 / 2,
Expand Down
38 changes: 12 additions & 26 deletions rs/messaging/src/scheduling/valid_set_rule.rs
View file
Expand Up @@ -16,7 +16,7 @@ use ic_replicated_state::{
replicated_state::{
LABEL_VALUE_CANISTER_NOT_FOUND, LABEL_VALUE_CANISTER_OUT_OF_CYCLES,
LABEL_VALUE_CANISTER_STOPPED, LABEL_VALUE_CANISTER_STOPPING,
LABEL_VALUE_INVALID_SUBNET_PAYLOAD, LABEL_VALUE_QUEUE_FULL,
LABEL_VALUE_INGRESS_HISTORY_FULL, LABEL_VALUE_INVALID_SUBNET_PAYLOAD,
LABEL_VALUE_UNKNOWN_SUBNET_METHOD,
},
ReplicatedState, StateError,
Expand Down Expand Up @@ -88,7 +88,7 @@ impl VsrMetrics {
LABEL_VALUE_SUCCESS,
LABEL_VALUE_DUPLICATE,
LABEL_VALUE_CANISTER_NOT_FOUND,
LABEL_VALUE_QUEUE_FULL,
LABEL_VALUE_INGRESS_HISTORY_FULL,
LABEL_VALUE_CANISTER_STOPPED,
LABEL_VALUE_CANISTER_STOPPING,
LABEL_VALUE_CANISTER_OUT_OF_CYCLES,
Expand Down Expand Up @@ -172,29 +172,15 @@ impl ValidSetRuleImpl {
LABEL_VALUE_SUCCESS
}
Err(err) => {
let error_code = match err {
StateError::CanisterNotFound(canister_id) => {
error!(
self.log,
"Failed to induct message: canister does not exist";
messaging.message_id => format!("{}", message_id),
messaging.canister_id => format!("{}", canister_id),
);
ErrorCode::CanisterNotFound
}
StateError::CanisterStopped(_) => ErrorCode::CanisterStopped,
StateError::CanisterStopping(_) => ErrorCode::CanisterStopping,
StateError::CanisterOutOfCycles { .. } => ErrorCode::CanisterOutOfCycles,
StateError::UnknownSubnetMethod(_) => ErrorCode::CanisterOutOfCycles,
StateError::InvalidSubnetPayload => ErrorCode::InvalidManagementPayload,
StateError::QueueFull { .. } => ErrorCode::IngressHistoryFull,
StateError::OutOfMemory { .. }
| StateError::InvariantBroken { .. }
| StateError::NonMatchingResponse { .. }
| StateError::BitcoinNonMatchingResponse { .. } => {
unreachable!("Unexpected error: {}", err)
}
};
if let StateError::CanisterNotFound(canister_id) = &err {
error!(
self.log,
"Failed to induct message: canister does not exist";
messaging.message_id => format!("{}", message_id),
messaging.canister_id => format!("{}", canister_id),
);
}
let error_code = ErrorCode::from(&err);
self.ingress_history_writer.set_status(
state,
message_id,
Expand Down Expand Up @@ -261,7 +247,7 @@ impl ValidSetRuleImpl {
if state.metadata.own_subnet_type != SubnetType::System
&& state.metadata.ingress_history.len() >= self.ingress_history_max_messages
{
return Err(StateError::QueueFull {
return Err(StateError::IngressHistoryFull {
capacity: self.ingress_history_max_messages,
});
}
Expand Down
2 changes: 1 addition & 1 deletion rs/messaging/src/scheduling/valid_set_rule/test.rs
View file
Expand Up @@ -938,7 +938,7 @@ fn ingress_history_max_messages_impl(subnet_type: SubnetType) {
assert_inducted_ingress_messages_eq(
metric_vec(&[
(&[(LABEL_STATUS, LABEL_VALUE_SUCCESS)], 3),
(&[(LABEL_STATUS, LABEL_VALUE_QUEUE_FULL)], 1),
(&[(LABEL_STATUS, LABEL_VALUE_INGRESS_HISTORY_FULL)], 1),
]),
&metrics_registry,
);
Expand Down
34 changes: 34 additions & 0 deletions rs/replicated_state/src/replicated_state.rs
View file
Expand Up @@ -60,6 +60,9 @@ pub enum StateError {
/// Message enqueuing failed due to full in/out queue.
QueueFull { capacity: usize },

/// Message enqueuing failed due to full ingress history.
IngressHistoryFull { capacity: usize },

/// Canister is stopped, not accepting any messages.
CanisterStopped(CanisterId),

Expand Down Expand Up @@ -214,6 +217,7 @@ impl PeekableOutputIterator for OutputIterator<'_> {

pub const LABEL_VALUE_CANISTER_NOT_FOUND: &str = "CanisterNotFound";
pub const LABEL_VALUE_QUEUE_FULL: &str = "QueueFull";
pub const LABEL_VALUE_INGRESS_HISTORY_FULL: &str = "IngressHistoryFull";
pub const LABEL_VALUE_CANISTER_STOPPED: &str = "CanisterStopped";
pub const LABEL_VALUE_CANISTER_STOPPING: &str = "CanisterStopping";
pub const LABEL_VALUE_CANISTER_OUT_OF_CYCLES: &str = "CanisterOutOfCycles";
Expand All @@ -231,6 +235,7 @@ impl StateError {
match self {
StateError::CanisterNotFound(_) => LABEL_VALUE_CANISTER_NOT_FOUND,
StateError::QueueFull { .. } => LABEL_VALUE_QUEUE_FULL,
StateError::IngressHistoryFull { .. } => LABEL_VALUE_INGRESS_HISTORY_FULL,
StateError::CanisterStopped(_) => LABEL_VALUE_CANISTER_STOPPED,
StateError::CanisterStopping(_) => LABEL_VALUE_CANISTER_STOPPING,
StateError::CanisterOutOfCycles(_) => LABEL_VALUE_CANISTER_OUT_OF_CYCLES,
Expand All @@ -257,6 +262,9 @@ impl std::fmt::Display for StateError {
StateError::QueueFull { capacity } => {
write!(f, "Maximum queue capacity {} reached", capacity)
}
StateError::IngressHistoryFull { capacity } => {
write!(f, "Maximum ingress history capacity {} reached", capacity)
}
StateError::CanisterStopped(canister_id) => {
write!(f, "Canister {} is stopped", canister_id)
}
Expand Down Expand Up @@ -301,6 +309,32 @@ impl std::fmt::Display for StateError {
}
}

impl From<&StateError> for ErrorCode {
fn from(err: &StateError) -> Self {
match err {
StateError::CanisterNotFound(_) => ErrorCode::CanisterNotFound,
StateError::CanisterStopped(_) => ErrorCode::CanisterStopped,
StateError::CanisterStopping(_) => ErrorCode::CanisterStopping,
StateError::CanisterOutOfCycles { .. } => ErrorCode::CanisterOutOfCycles,
StateError::UnknownSubnetMethod(_) => ErrorCode::CanisterMethodNotFound,
StateError::InvalidSubnetPayload => ErrorCode::InvalidManagementPayload,
StateError::QueueFull { .. } => ErrorCode::CanisterQueueFull,
StateError::IngressHistoryFull { .. } => ErrorCode::IngressHistoryFull,
StateError::OutOfMemory { .. } => ErrorCode::CanisterOutOfMemory,

// These errors cannot happen when pushing a request or ingress:
//
// * `InvariantBroken` is only produced by `check_invariants()`; and
// * `.*NonMatchingResponse` is only produced for responses.
StateError::InvariantBroken { .. }
| StateError::NonMatchingResponse { .. }
| StateError::BitcoinNonMatchingResponse { .. } => {
unreachable!("Not a user error: {}", err)
}
}
}
}

/// Represents the memory taken in bytes by various resources.
///
/// Should be used in cases where the deterministic state machine needs to
Expand Down
8 changes: 4 additions & 4 deletions rs/types/error_types/src/lib.rs
View file
Expand Up @@ -64,7 +64,7 @@ impl From<ErrorCode> for RejectCode {
match err {
SubnetOversubscribed => SysFatal,
MaxNumberOfCanistersReached => SysFatal,
CanisterOutputQueueFull => SysTransient,
CanisterQueueFull => SysTransient,
IngressMessageTimeout => SysTransient,
CanisterQueueNotEmpty => SysTransient,
IngressHistoryFull => SysTransient,
Expand Down Expand Up @@ -129,7 +129,7 @@ impl From<ErrorCode> for RejectCode {
pub enum ErrorCode {
SubnetOversubscribed = 101,
MaxNumberOfCanistersReached = 102,
CanisterOutputQueueFull = 201,
CanisterQueueFull = 201,
IngressMessageTimeout = 202,
CanisterQueueNotEmpty = 203,
IngressHistoryFull = 204,
Expand Down Expand Up @@ -186,7 +186,7 @@ impl TryFrom<u64> for ErrorCode {
match err {
101 => Ok(ErrorCode::SubnetOversubscribed),
102 => Ok(ErrorCode::MaxNumberOfCanistersReached),
201 => Ok(ErrorCode::CanisterOutputQueueFull),
201 => Ok(ErrorCode::CanisterQueueFull),
202 => Ok(ErrorCode::IngressMessageTimeout),
203 => Ok(ErrorCode::CanisterQueueNotEmpty),
204 => Ok(ErrorCode::IngressHistoryFull),
Expand Down Expand Up @@ -306,7 +306,7 @@ impl UserError {
ErrorCode::CanisterWasmEngineError | ErrorCode::QueryCallGraphInternal => true,
ErrorCode::SubnetOversubscribed
| ErrorCode::MaxNumberOfCanistersReached
| ErrorCode::CanisterOutputQueueFull
| ErrorCode::CanisterQueueFull
| ErrorCode::IngressMessageTimeout
| ErrorCode::StopCanisterRequestTimeout
| ErrorCode::CanisterQueueNotEmpty
Expand Down

0 comments on commit 8c9e85f

Please sign in to comment.