diff --git a/components/nimbus/src/error.rs b/components/nimbus/src/error.rs index 9372de6dc3..b0704537ba 100644 --- a/components/nimbus/src/error.rs +++ b/components/nimbus/src/error.rs @@ -21,8 +21,8 @@ pub enum NimbusError { RkvError(#[from] rkv::StoreError), #[error("IO error: {0}")] IOError(#[from] std::io::Error), - #[error("JSON Error: {0}")] - JSONError(#[from] serde_json::Error), + #[error("JSON Error: {0} — {1}")] + JSONError(String, String), #[error("EvaluationError: {0}")] EvaluationError(String), #[error("Invalid Expression - didn't evaluate to a bool")] @@ -103,6 +103,13 @@ pub enum CirrusClientError { RequestMissingParameter(String), } +#[cfg(test)] +impl From for NimbusError { + fn from(error: serde_json::Error) -> Self { + NimbusError::JSONError("test".into(), error.to_string()) + } +} + impl<'a> From> for NimbusError { fn from(eval_error: jexl_eval::error::EvaluationError<'a>) -> Self { NimbusError::EvaluationError(eval_error.to_string()) diff --git a/components/nimbus/src/sampling.rs b/components/nimbus/src/sampling.rs index 7f564dad00..c1545dfd69 100644 --- a/components/nimbus/src/sampling.rs +++ b/components/nimbus/src/sampling.rs @@ -99,7 +99,15 @@ pub(crate) fn ratio_sample(input: T, ratios: &[u32]) -> Res /// that is larger than 6 bytes (Should never occur) pub(crate) fn truncated_hash(data: T) -> Result<[u8; 6]> { let mut hasher = Sha256::new(); - let data_str = serde_json::to_string(&data)?; + let data_str = match serde_json::to_string(&data) { + Ok(v) => v, + Err(e) => { + return Err(NimbusError::JSONError( + "data_str = nimbus::sampling::truncated_hash::serde_json::to_string".into(), + e.to_string(), + )) + } + }; hasher.update(data_str.as_bytes()); Ok(hasher.finalize()[0..6].try_into()?) } diff --git a/components/nimbus/src/schema.rs b/components/nimbus/src/schema.rs index 1df9832fe8..7c73f0e67e 100644 --- a/components/nimbus/src/schema.rs +++ b/components/nimbus/src/schema.rs @@ -106,7 +106,15 @@ pub fn parse_experiments(payload: &str) -> Result> { // We first encode the response into a `serde_json::Value` // to allow us to deserialize each experiment individually, // omitting any malformed experiments - let value: Value = serde_json::from_str(payload)?; + let value: Value = match serde_json::from_str(payload) { + Ok(v) => v, + Err(e) => { + return Err(NimbusError::JSONError( + "value = nimbus::schema::parse_experiments::serde_json::from_str".into(), + e.to_string(), + )) + } + }; let data = value .get("data") .ok_or(NimbusError::InvalidExperimentFormat)?; diff --git a/components/nimbus/src/stateful/behavior.rs b/components/nimbus/src/stateful/behavior.rs index 06d46c56c9..320ce8af15 100644 --- a/components/nimbus/src/stateful/behavior.rs +++ b/components/nimbus/src/stateful/behavior.rs @@ -357,8 +357,14 @@ impl EventQueryType { self ))); } - let event = serde_json::from_value::(args.first().unwrap().clone())?; - let interval = serde_json::from_value::(args.get(1).unwrap().clone())?; + let event = match serde_json::from_value::(args.first().unwrap().clone()) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("event = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value".into(), e.to_string())) + }; + let interval = match serde_json::from_value::(args.get(1).unwrap().clone()) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("interval = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value".into(), e.to_string())) + }; let interval = Interval::from_str(&interval)?; let num_buckets = match args.get(2).unwrap().as_f64() { Some(v) => v, @@ -393,8 +399,14 @@ impl EventQueryType { self ))); } - let event = serde_json::from_value::(args.first().unwrap().clone())?; - let interval = serde_json::from_value::(args.get(1).unwrap().clone())?; + let event = match serde_json::from_value::(args.first().unwrap().clone()) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("event = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value".into(), e.to_string())) + }; + let interval = match serde_json::from_value::(args.get(1).unwrap().clone()) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("interval = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value".into(), e.to_string())) + }; let interval = Interval::from_str(&interval)?; let zero = &Value::from(0); let starting_bucket = match args.get(2).unwrap_or(zero).as_f64() { diff --git a/components/nimbus/src/stateful/nimbus_client.rs b/components/nimbus/src/stateful/nimbus_client.rs index 5f7246355c..1013cbe9c7 100644 --- a/components/nimbus/src/stateful/nimbus_client.rs +++ b/components/nimbus/src/stateful/nimbus_client.rs @@ -163,9 +163,12 @@ impl NimbusClient { .read_from_db(db)?; if let Some(recorded_context) = &self.recorded_context { - let targeting_helper = self.create_targeting_helper_with_context(serde_json::to_value( + let targeting_helper = self.create_targeting_helper_with_context(match serde_json::to_value( &state.targeting_attributes, - )?); + ) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("targeting_helper = nimbus::stateful::nimbus_client::NimbusClient::begin_initialize::serde_json::to_value".into(), e.to_string())) + }); recorded_context.execute_queries(targeting_helper.as_ref())?; state .targeting_attributes @@ -512,7 +515,10 @@ impl NimbusClient { let mut buf = String::new(); file.read_to_string(&mut buf)?; - let res = serde_json::from_str::>(&buf)?; + let res = match serde_json::from_str::>(&buf) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("res = nimbus::stateful::nimbus_client::get_creation_date_from_path::serde_json::from_str".into(), e.to_string())) + }; Ok(res) } @@ -630,7 +636,10 @@ impl NimbusClient { fn merge_additional_context(&self, context: Option) -> Result { let context = context.map(Value::Object); - let targeting = serde_json::to_value(self.get_targeting_attributes())?; + let targeting = match serde_json::to_value(self.get_targeting_attributes()) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("targeting = nimbus::stateful::nimbus_client::NimbusClient::merge_additional_context::serde_json::to_value".into(), e.to_string())) + }; let context = match context { Some(v) => v.defaults(&targeting)?, None => targeting, diff --git a/components/nimbus/src/stateful/persistence.rs b/components/nimbus/src/stateful/persistence.rs index f0644babe8..60cfbf3e98 100644 --- a/components/nimbus/src/stateful/persistence.rs +++ b/components/nimbus/src/stateful/persistence.rs @@ -157,7 +157,10 @@ impl SingleStore { key: &str, persisted_data: &T, ) -> Result<()> { - let persisted_json = serde_json::to_string(persisted_data)?; + let persisted_json = match serde_json::to_string(persisted_data) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("persisted_json = nimbus::stateful::persistence::SingleStore::put::serde_json::to_string".into(), e.to_string())) + }; self.store .put(writer, key, &rkv::Value::Json(&persisted_json))?; Ok(()) @@ -187,7 +190,10 @@ impl SingleStore { match persisted_data { Some(data) => { if let rkv::Value::Json(data) = data { - Ok(Some(serde_json::from_str::(data)?)) + Ok(Some(match serde_json::from_str::(data) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("match persisted_data nimbus::stateful::persistence::SingleStore::get::serde_json::from_str".into(), e.to_string())) + })) } else { Err(NimbusError::InvalidPersistedData) } @@ -239,7 +245,10 @@ impl SingleStore { let mut iter = self.store.iter_start(reader)?; while let Some(Ok((_, data))) = iter.next() { if let rkv::Value::Json(data) = data { - result.push(serde_json::from_str::(data)?); + result.push(match serde_json::from_str::(data) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("rkv::Value::Json(data) nimbus::stateful::persistence::SingleStore::collect_all::serde_json::from_str".into(), e.to_string())) + }); } } Ok(result) @@ -510,7 +519,10 @@ impl Database { match persisted_data { Some(data) => { if let rkv::Value::Json(data) = data { - Ok(Some(serde_json::from_str::(data)?)) + Ok(Some(match serde_json::from_str::(data) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("rkv::Value::Json(data) nimbus::stateful::persistence::Database::get::serde_json::from_str".into(), e.to_string())) + })) } else { Err(NimbusError::InvalidPersistedData) } @@ -533,7 +545,10 @@ impl Database { let mut iter = self.get_store(store_id).store.iter_start(&reader)?; while let Some(Ok((_, data))) = iter.next() { if let rkv::Value::Json(data) = data { - result.push(serde_json::from_str::(data)?); + result.push(match serde_json::from_str::(data) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("rkv::Value::Json(data) nimbus::stateful::persistence::Database::collect_all::serde_json::from_str".into(), e.to_string())) + }); } } Ok(result) diff --git a/components/nimbus/src/stateless/cirrus_client.rs b/components/nimbus/src/stateless/cirrus_client.rs index 49f3999383..a044f52740 100644 --- a/components/nimbus/src/stateless/cirrus_client.rs +++ b/components/nimbus/src/stateless/cirrus_client.rs @@ -90,7 +90,10 @@ impl CirrusClient { metrics_handler: Box, coenrolling_feature_ids: Vec, ) -> Result { - let app_context: AppContext = serde_json::from_str(&app_context)?; + let app_context: AppContext = match serde_json::from_str(&app_context) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("app_context = nimbus::stateless::cirrus_client::CirrusClient::new::serde_json::from_str".into(), e.to_string())) + }; Ok(Self { app_context, coenrolling_feature_ids, @@ -112,7 +115,10 @@ impl CirrusClient { request_context, is_user_participating, prev_enrollments, - } = serde_json::from_str(request.as_str())?; + } = match serde_json::from_str(request.as_str()) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("EnrollmentRequest { .. } = nimbus::stateless::cirrus_client::CirrusClient::handle_enrollment::serde_json::from_str".into(), e.to_string())) + }; let client_id = if let Some(client_id) = client_id { client_id } else { @@ -121,12 +127,15 @@ impl CirrusClient { )); }; - Ok(serde_json::to_string(&self.enroll( + Ok(match serde_json::to_string(&self.enroll( client_id, request_context, is_user_participating, &prev_enrollments, - )?)?) + )?) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("return nimbus::stateless::cirrus_client::CirrusClient::handle_enrollment::serde_json::to_string".into(), e.to_string())) + }) } pub(crate) fn enroll( diff --git a/components/nimbus/src/strings.rs b/components/nimbus/src/strings.rs index 5776c5dade..4691ac43a8 100644 --- a/components/nimbus/src/strings.rs +++ b/components/nimbus/src/strings.rs @@ -7,7 +7,15 @@ use serde_json::{value::Value, Map}; #[allow(dead_code)] pub fn fmt(template: &str, context: &T) -> Result { - let obj: Value = serde_json::to_value(context)?; + let obj: Value = match serde_json::to_value(context) { + Ok(v) => v, + Err(e) => { + return Err(NimbusError::JSONError( + "obj = nimbus::strings::fmt::serde_json::to_value".into(), + e.to_string(), + )) + } + }; fmt_with_value(template, &obj) } diff --git a/components/nimbus/src/tests/stateful/client/test_http_client.rs b/components/nimbus/src/tests/stateful/client/test_http_client.rs index ffe33b9d0d..0de98609c2 100644 --- a/components/nimbus/src/tests/stateful/client/test_http_client.rs +++ b/components/nimbus/src/tests/stateful/client/test_http_client.rs @@ -66,7 +66,7 @@ fn test_malformed_payload() { "#; let result = parse_experiments(payload).unwrap_err(); - assert!(matches!(result, NimbusError::JSONError(_))); + assert!(matches!(result, NimbusError::JSONError(_, _))); } // This response body includes a matching schema version, a non-matching schema version, diff --git a/components/nimbus/src/tests/stateful/test_evaluator.rs b/components/nimbus/src/tests/stateful/test_evaluator.rs index 3d98e8dcda..f2422e509c 100644 --- a/components/nimbus/src/tests/stateful/test_evaluator.rs +++ b/components/nimbus/src/tests/stateful/test_evaluator.rs @@ -158,7 +158,7 @@ fn test_event_transform_sum_cnz_avg_avgnz_parameters() { &th, ), Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: invalid type: floating point `1.0`, expected a string" + reason: "EvaluationError: Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" .to_string() }) ); @@ -168,7 +168,7 @@ fn test_event_transform_sum_cnz_avg_avgnz_parameters() { &th, ), Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: invalid type: floating point `1.0`, expected a string" + reason: "EvaluationError: Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_counting_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" .to_string() }) ); @@ -234,7 +234,7 @@ fn test_event_transform_last_seen_parameters() { &th, ), Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: invalid type: floating point `1.0`, expected a string" + reason: "EvaluationError: Custom error: JSON Error: event = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" .to_string() }) ); @@ -244,7 +244,7 @@ fn test_event_transform_last_seen_parameters() { &th, ), Some(EnrollmentStatus::Error { - reason: "EvaluationError: Custom error: JSON Error: invalid type: floating point `1.0`, expected a string" + reason: "EvaluationError: Custom error: JSON Error: interval = nimbus::stateful::behavior::EventQueryType::validate_last_seen_arguments::serde_json::from_value — invalid type: floating point `1.0`, expected a string" .to_string() }) ); diff --git a/components/nimbus/tests/test_restart.rs b/components/nimbus/tests/test_restart.rs index 23be0fa3b6..f5f4646954 100644 --- a/components/nimbus/tests/test_restart.rs +++ b/components/nimbus/tests/test_restart.rs @@ -13,6 +13,7 @@ mod test { use super::common::new_test_client_with_db; #[cfg(feature = "rkv-safe-mode")] use nimbus::error::Result; + use nimbus::NimbusError; use serde_json::json; fn experiment_target_false() -> serde_json::Value { @@ -132,12 +133,15 @@ mod test { let temp_dir = tempfile::tempdir()?; let client = new_test_client_with_db(&temp_dir)?; client.initialize()?; - let experiment_json = serde_json::to_string(&json!({ + let experiment_json = match serde_json::to_string(&json!({ "data": [ experiment_target_false(), experiment_zero_buckets(), ] - }))?; + })) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("test".into(), e.to_string())), + }; client.set_experiments_locally(experiment_json.clone())?; client.apply_pending_experiments()?; // the experiment_target_false experiment has a 'targeting' of "false", we test to ensure that @@ -180,13 +184,16 @@ mod test { let ta = client.get_targeting_attributes(); assert_eq!(ta.active_experiments, expected); - let experiment_json = serde_json::to_string(&json!({ + let experiment_json = match serde_json::to_string(&json!({ "data": [ experiment_target_false(), experiment_zero_buckets(), experiment_always_enroll(), ] - }))?; + })) { + Ok(v) => v, + Err(e) => return Err(NimbusError::JSONError("test".into(), e.to_string())), + }; client.set_experiments_locally(experiment_json)?; client.apply_pending_experiments()?;