From 7762e4a59968e9236c38f05ae3e6940791a4a70b Mon Sep 17 00:00:00 2001 From: Mike Voronov Date: Thu, 2 Sep 2021 18:01:21 +0300 Subject: [PATCH] Treat call service result de errors as local service errors (#134) --- Cargo.lock | 20 +++++++++ air/Cargo.toml | 2 + .../execution_step/air/call/resolved_call.rs | 33 +++++++++++---- air/src/execution_step/errors.rs | 41 ++++--------------- air/src/preparation_step/errors.rs | 19 +++++---- air/tests/test_module/instructions/call.rs | 2 +- .../test_module/integration/join_behaviour.rs | 4 +- .../integration/streams_early_exit.rs | 2 +- 8 files changed, 72 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5d2e0d50..18a591c2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -33,6 +33,8 @@ dependencies = [ "pretty_assertions", "serde", "serde_json", + "strum", + "strum_macros", "thiserror", "wasm-bindgen", ] @@ -1818,6 +1820,24 @@ dependencies = [ "precomputed-hash", ] +[[package]] +name = "strum" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aaf86bbcfd1fa9670b7a129f64fc0c9fcbbfe4f1bc4210e9e98fe71ffc12cde2" + +[[package]] +name = "strum_macros" +version = "0.21.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d06aaeeee809dbc59eb4556183dd927df67db1540de5be8d3ec0b6636358a5ec" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "subtle" version = "2.4.1" diff --git a/air/Cargo.toml b/air/Cargo.toml index ce30700c..d94d1e9c 100644 --- a/air/Cargo.toml +++ b/air/Cargo.toml @@ -25,6 +25,8 @@ jsonpath_lib-fl = "=0.3.7" boolinator = "2.4.0" log = "0.4.11" thiserror = "1.0.23" +strum = "0.21" +strum_macros = "0.21" # Keep 0.2.65 until this is resolved https://github.com/rustwasm/wasm-pack/issues/886 wasm-bindgen = "=0.2.65" diff --git a/air/src/execution_step/air/call/resolved_call.rs b/air/src/execution_step/air/call/resolved_call.rs index f98271da..f00aafc8 100644 --- a/air/src/execution_step/air/call/resolved_call.rs +++ b/air/src/execution_step/air/call/resolved_call.rs @@ -103,16 +103,12 @@ impl<'i> ResolvedCall<'i> { exec_ctx: &mut ExecutionCtx<'i>, trace_ctx: &mut TraceHandler, ) -> ExecutionResult<()> { - use ExecutionError::CallServiceResultDeError as DeError; - // check that service call succeeded - let service_result = handle_service_error(service_result, trace_ctx)?; - - let result: JValue = serde_json::from_str(&service_result.result).map_err(|e| DeError(service_result, e))?; - let result = Rc::new(result); + let call_service_result = handle_service_error(service_result, trace_ctx)?; + // try to get service result from call service result + let result = try_to_service_result(call_service_result, trace_ctx)?; let trace_pos = trace_ctx.trace_pos(); - let executed_result = ResolvedCallResult::new(result, self.tetraplet.clone(), trace_pos); let new_call_result = set_local_result(executed_result, &self.output, exec_ctx)?; trace_ctx.meet_call_end(new_call_result); @@ -191,3 +187,26 @@ fn handle_service_error( Err(error) } + +fn try_to_service_result( + service_result: CallServiceResult, + trace_ctx: &mut TraceHandler, +) -> ExecutionResult> { + use CallResult::CallServiceFailed; + + match serde_json::from_str(&service_result.result) { + Ok(result) => Ok(Rc::new(result)), + Err(e) => { + let error_msg = format!( + "call_service result '{0}' can't be serialized or deserialized with an error: {1}", + service_result.result, e + ); + let error_msg = Rc::new(error_msg); + + let error = CallServiceFailed(i32::MAX, error_msg.clone()); + trace_ctx.meet_call_end(error); + + Err(Rc::new(ExecutionError::LocalServiceError(i32::MAX, error_msg.clone()))) + } + } +} diff --git a/air/src/execution_step/errors.rs b/air/src/execution_step/errors.rs index d0313da2..5590d14e 100644 --- a/air/src/execution_step/errors.rs +++ b/air/src/execution_step/errors.rs @@ -24,22 +24,20 @@ use super::trace_handler::MergerApResult; use super::trace_handler::TraceHandlerError; use super::ResolvedCallResult; use super::Stream; -use crate::build_targets::CallServiceResult; use crate::JValue; use jsonpath_lib::JsonPathError; -use serde_json::Error as SerdeJsonError; +use strum::IntoEnumIterator; +use strum_macros::EnumDiscriminants; +use strum_macros::EnumIter; use thiserror::Error as ThisError; use std::rc::Rc; /// Errors arised while executing AIR script. -#[derive(ThisError, Debug)] +#[derive(ThisError, EnumDiscriminants, Debug)] +#[strum_discriminants(derive(EnumIter))] pub(crate) enum ExecutionError { - /// Errors occurred while parsing returned by call_service value. - #[error("call_service result '{0}' can't be serialized or deserialized with an error: {1}")] - CallServiceResultDeError(CallServiceResult, SerdeJsonError), - /// Semantic errors in a call instructions. #[error("call should have service id specified by peer part or function part")] IncorrectCallTriplet, @@ -137,32 +135,11 @@ impl From for Rc { impl ExecutionError { pub(crate) fn to_error_code(&self) -> u32 { - use ExecutionError::*; + let mut errors = ExecutionErrorDiscriminants::iter(); + let actual_error_type = ExecutionErrorDiscriminants::from(self); - match self { - CallServiceResultDeError(..) => 1, - IncorrectCallTriplet => 2, - LocalServiceError(..) => 3, - VariableNotFound(_) => 4, - MultipleVariablesFound(_) => 5, - JValueJsonPathError(..) => 6, - GenerationStreamJsonPathError(..) => 7, - IncompatibleJValueType(..) => 8, - IncompatibleAValueType(..) => 9, - MultipleValuesInJsonPath(_) => 10, - FoldStateNotFound(_) => 11, - MultipleFoldStates(_) => 12, - IterableShadowing(_) => 13, - MatchWithoutXorError => 14, - MismatchWithoutXorError => 15, - FlatteningError(_) => 16, - JsonPathVariableTypeError(_) => 17, - StreamJsonPathError(..) => 18, - StreamDontHaveSuchGeneration(..) => 19, - ApResultNotCorrespondToInstr(_) => 20, - EmptyStreamJsonPathError(_) => 21, - TraceError(_) => 22, - } + // unwrap is safe here because errors are guaranteed to contain all errors variants + errors.position(|et| et == actual_error_type).unwrap() as _ } } diff --git a/air/src/preparation_step/errors.rs b/air/src/preparation_step/errors.rs index c6255c0f..2478785c 100644 --- a/air/src/preparation_step/errors.rs +++ b/air/src/preparation_step/errors.rs @@ -14,14 +14,19 @@ * limitations under the License. */ +use air_interpreter_data::DATA_FORMAT_VERSION; + use serde_json::Error as SerdeJsonError; +use strum::IntoEnumIterator; +use strum_macros::EnumDiscriminants; +use strum_macros::EnumIter; use thiserror::Error as ThisError; -use air_interpreter_data::DATA_FORMAT_VERSION; use std::env::VarError; /// Errors happened during the interpreter preparation_step step. -#[derive(Debug, ThisError)] +#[derive(Debug, EnumDiscriminants, ThisError)] +#[strum_discriminants(derive(EnumIter))] pub enum PreparationError { /// Error occurred while parsing AIR script #[error("air can't be parsed:\n{0}")] @@ -39,12 +44,10 @@ pub enum PreparationError { impl PreparationError { pub(crate) fn to_error_code(&self) -> u32 { - use PreparationError::*; + let mut errors = PreparationErrorDiscriminants::iter(); + let actual_error_type = PreparationErrorDiscriminants::from(self); - match self { - AIRParseError(_) => 1, - DataDeFailed(..) => 2, - CurrentPeerIdEnvError(_) => 3, - } + // unwrap is safe here because errors are guaranteed to contain all errors variants + errors.position(|et| et == actual_error_type).unwrap() as _ } } diff --git a/air/tests/test_module/instructions/call.rs b/air/tests/test_module/instructions/call.rs index 065eec04..dc1d555c 100644 --- a/air/tests/test_module/instructions/call.rs +++ b/air/tests/test_module/instructions/call.rs @@ -120,7 +120,7 @@ fn duplicate_variables() { let result = call_vm!(vm, "asd", script, "", ""); - assert_eq!(result.ret_code, 1005); + assert_eq!(result.ret_code, 1003); assert!(result.next_peer_pks.is_empty()); } diff --git a/air/tests/test_module/integration/join_behaviour.rs b/air/tests/test_module/integration/join_behaviour.rs index 4ded2e17..94608c28 100644 --- a/air/tests/test_module/integration/join_behaviour.rs +++ b/air/tests/test_module/integration/join_behaviour.rs @@ -176,7 +176,7 @@ fn dont_wait_on_json_path_on_scalars() { let init_peer_id = "asd"; let result = call_vm!(set_variable_vm, init_peer_id, &script, "", ""); let array_result = call_vm!(array_consumer, init_peer_id, &script, "", result.data.clone()); - assert_eq!(array_result.ret_code, 1006); + assert_eq!(array_result.ret_code, 1004); assert_eq!( array_result.error_message, r#"variable with path '$.[5]' not found in '[1,2,3,4,5]' with an error: 'json value not set'"# @@ -195,7 +195,7 @@ fn dont_wait_on_json_path_on_scalars() { let init_peer_id = "asd"; let result = call_vm!(set_variable_vm, init_peer_id, &script, "", ""); let object_result = call_vm!(object_consumer, init_peer_id, script, "", result.data); - assert_eq!(object_result.ret_code, 1006); + assert_eq!(object_result.ret_code, 1004); assert_eq!( object_result.error_message, r#"variable with path '$.non_exist_path' not found in '{"err_msg":"","is_authenticated":1,"ret_code":0}' with an error: 'json value not set'"# diff --git a/air/tests/test_module/integration/streams_early_exit.rs b/air/tests/test_module/integration/streams_early_exit.rs index 911ed231..759c2eec 100644 --- a/air/tests/test_module/integration/streams_early_exit.rs +++ b/air/tests/test_module/integration/streams_early_exit.rs @@ -104,7 +104,7 @@ fn par_early_exit() { ]; let setter_3_malicious_data = raw_data_from_trace(setter_3_malicious_trace); let init_result_5 = call_vm!(init, "", &script, init_result_3.data.clone(), setter_3_malicious_data); - assert_eq!(init_result_5.ret_code, 1022); + assert_eq!(init_result_5.ret_code, 1018); let actual_trace = trace_from_result(&init_result_5); let expected_trace = trace_from_result(&init_result_3);