From c673bd56b832446566dd20f340b2df4b67c896e1 Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 9 Aug 2024 14:28:10 +0200 Subject: [PATCH 1/4] properly check the internal threads for errors and or panics --- src/graph_loader.rs | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/src/graph_loader.rs b/src/graph_loader.rs index 1ae74e74..c1390235 100644 --- a/src/graph_loader.rs +++ b/src/graph_loader.rs @@ -571,7 +571,21 @@ impl GraphLoader { info!("{:?} Got all data, processing...", SystemTime::now()); for c in consumers { - let _guck = c.join(); + match c.join() { + Ok(Ok(())) => { + // The thread completed successfully and returned Ok + } + Ok(Err(e)) => { + // The thread completed but returned an error + eprintln!("Thread returned error: {:?}", e); + return Err(e); // Propagate the error + } + Err(e) => { + // The thread panicked + eprintln!("Thread panicked: {:?}", e); + return Err(GraphLoaderError::from("Thread panicked".to_string())); + } + } } } Ok(()) @@ -846,7 +860,21 @@ impl GraphLoader { std::time::SystemTime::now() ); for c in consumers { - let _guck = c.join(); + match c.join() { + Ok(Ok(())) => { + // The thread completed successfully and returned Ok + } + Ok(Err(e)) => { + // The thread completed but returned an error + eprintln!("Thread returned error: {:?}", e); + return Err(e); // Propagate the error + } + Err(e) => { + // The thread panicked + eprintln!("Thread panicked: {:?}", e); + return Err(GraphLoaderError::from("Thread panicked".to_string())); + } + } } Ok(()) } From fadfcec71212aa373f7de77418eff7cf7492363c Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 9 Aug 2024 16:17:39 +0200 Subject: [PATCH 2/4] added new thread failure behavior, adjusted tests --- src/graph_loader.rs | 12 ++++--- tests/graph_loader.rs | 73 ++++++++++++++++++++++++++++++------------- 2 files changed, 59 insertions(+), 26 deletions(-) diff --git a/src/graph_loader.rs b/src/graph_loader.rs index c1390235..95cb5ddc 100644 --- a/src/graph_loader.rs +++ b/src/graph_loader.rs @@ -582,8 +582,10 @@ impl GraphLoader { } Err(e) => { // The thread panicked - eprintln!("Thread panicked: {:?}", e); - return Err(GraphLoaderError::from("Thread panicked".to_string())); + eprintln!("Thread panicked in do_vertices: {:?}", e); + return Err(GraphLoaderError::from( + "Thread panicked in do_vertices".to_string(), + )); } } } @@ -871,8 +873,10 @@ impl GraphLoader { } Err(e) => { // The thread panicked - eprintln!("Thread panicked: {:?}", e); - return Err(GraphLoaderError::from("Thread panicked".to_string())); + eprintln!("Thread panicked in do_edges: {:?}", e); + return Err(GraphLoaderError::from( + "Thread panicked in do_edges".to_string(), + )); } } } diff --git a/tests/graph_loader.rs b/tests/graph_loader.rs index 92d7de0d..d9246694 100644 --- a/tests/graph_loader.rs +++ b/tests/graph_loader.rs @@ -255,7 +255,9 @@ async fn init_named_graph_loader_with_data() { teardown().await; } -fn get_attribute_position(field_names: &Vec, attribute: &str) -> usize { +fn get_attribute_position_from_fields(field_names: &Vec, attribute: &str) -> usize { + assert!(!field_names.is_empty()); + assert!(field_names.contains(&attribute.to_string())); field_names.iter().position(|x| x == attribute).unwrap() } @@ -300,13 +302,13 @@ async fn init_named_graph_loader_with_data_all_v_and_e_attributes_manually_set() assert_eq!(vertex.len(), 3); assert_eq!(vertex.len(), vertex_field_names.len()); - let x = &vertex[get_attribute_position(vertex_field_names, "x")] + let x = &vertex[get_attribute_position_from_fields(vertex_field_names, "x")] .as_u64() .unwrap(); - let y = &vertex[get_attribute_position(vertex_field_names, "y")] + let y = &vertex[get_attribute_position_from_fields(vertex_field_names, "y")] .as_u64() .unwrap(); - let z = &vertex[get_attribute_position(vertex_field_names, "z")] + let z = &vertex[get_attribute_position_from_fields(vertex_field_names, "z")] .as_u64() .unwrap(); let expected_x_value = (v_index + 1) as u64; @@ -358,13 +360,13 @@ async fn init_named_graph_loader_with_data_all_v_and_e_attributes_manually_set() assert_eq!(edge.len(), 3); assert_eq!(edge.len(), edge_field_names.len()); - let x = &edge[get_attribute_position(edge_field_names, "x")] + let x = &edge[get_attribute_position_from_fields(edge_field_names, "x")] .as_u64() .unwrap(); - let y = &edge[get_attribute_position(edge_field_names, "y")] + let y = &edge[get_attribute_position_from_fields(edge_field_names, "y")] .as_u64() .unwrap(); - let z = &edge[get_attribute_position(edge_field_names, "z")] + let z = &edge[get_attribute_position_from_fields(edge_field_names, "z")] .as_u64() .unwrap(); let expected_x_value = (e_index + 1) as u64; @@ -429,7 +431,7 @@ async fn init_named_graph_loader_with_data_all_v_and_e_collection_name_attribute assert_eq!(vertex.len(), vertex_field_names.len()); let collection_name = &vertex - [get_attribute_position(vertex_field_names, "@collection_name")] + [get_attribute_position_from_fields(vertex_field_names, "@collection_name")] .as_str() .unwrap(); let expected_collection_name = VERTEX_COLLECTION; @@ -472,11 +474,11 @@ async fn init_named_graph_loader_with_data_all_v_and_e_collection_name_attribute } for (_e_index, edge) in columns.iter().enumerate() { - assert_eq!(edge.len(), 3); - assert_eq!(edge.len(), edge_field_names.len()); + assert_eq!(edge.len(), 1); + assert_eq!(edge_field_names.len(), 1); let collection_name = &edge - [get_attribute_position(edge_field_names, "@collection_name")] + [get_attribute_position_from_fields(edge_field_names, "@collection_name")] .as_str() .unwrap(); let expected_collection_name = EDGE_COLLECTION; @@ -533,15 +535,28 @@ async fn init_named_graph_loader_with_data_all_v_and_e_attributes_all_by_boolean for (v_index, vertex_json_arr) in columns.iter().enumerate() { assert_eq!(vertex_json_arr.len(), 1); let vertex = &vertex_json_arr[0]; - assert_eq!(3, vertex.as_object().unwrap().len()); - - let x = &vertex[get_attribute_position(vertex_field_names, "x")] + // x, y, y including _key and _rev + assert_eq!(5, vertex.as_object().unwrap().len()); + + let x = &vertex + .as_object() + .unwrap() + .get("x") + .unwrap() .as_u64() .unwrap(); - let y = &vertex[get_attribute_position(vertex_field_names, "y")] + let y = &vertex + .as_object() + .unwrap() + .get("y") + .unwrap() .as_u64() .unwrap(); - let z = &vertex[get_attribute_position(vertex_field_names, "z")] + let z = &vertex + .as_object() + .unwrap() + .get("z") + .unwrap() .as_u64() .unwrap(); let expected_x_value = (v_index + 1) as u64; @@ -588,17 +603,31 @@ async fn init_named_graph_loader_with_data_all_v_and_e_attributes_all_by_boolean for (e_index, edge_json_arr) in columns.iter().enumerate() { assert_eq!(edge_json_arr.len(), 1); - assert_eq!(edge_json_arr.len(), edge_field_names.len()); + assert_eq!(edge_field_names.len(), 0); let edge = &edge_json_arr[0]; - assert_eq!(3, edge.as_object().unwrap().len()); - - let x = &edge[get_attribute_position(edge_field_names, "x")] + println!("Edge is {:?}", edge); + assert_eq!(6, edge.as_object().unwrap().len()); + + // x, y, z and _id, _key, _rev + let x = &edge + .as_object() + .unwrap() + .get("x") + .unwrap() .as_u64() .unwrap(); - let y = &edge[get_attribute_position(edge_field_names, "y")] + let y = &edge + .as_object() + .unwrap() + .get("y") + .unwrap() .as_u64() .unwrap(); - let z = &edge[get_attribute_position(edge_field_names, "z")] + let z = &edge + .as_object() + .unwrap() + .get("z") + .unwrap() .as_u64() .unwrap(); let expected_x_value = (e_index + 1) as u64; From 339a495bd1c99a5dedfe4dd33eb964b2e8fcd18a Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 9 Aug 2024 16:35:01 +0200 Subject: [PATCH 3/4] added forgotten id checkg --- src/graph_loader.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/graph_loader.rs b/src/graph_loader.rs index 95cb5ddc..fb45f184 100644 --- a/src/graph_loader.rs +++ b/src/graph_loader.rs @@ -754,15 +754,11 @@ impl GraphLoader { edge.as_object_mut().unwrap().remove("_to"); edge_json.push(vec![edge]); } else { - let id: &Value = &edge["_id"]; - let idstr: &String = match id { - Value::String(i) => i, - _ => { - return Err(GraphLoaderError::JsonParseError(format!( - "JSON is no object with a string _id attribute:\n{}", - edge - ))); - } + // it is not guaranteed that the _id field is present + let id = &edge["_id"]; + let idstr: Option<&String> = match id { + Value::String(i) => Some(i), + _ => None, }; // If we get here, we have to extract the field @@ -770,7 +766,11 @@ impl GraphLoader { // to edge_json: let get_value = |v: &Value, field: &str| -> Value { if field == "@collection_name" { - Value::String(collection_name_from_id(idstr)) + if let Some(id) = idstr { + Value::String(collection_name_from_id(id)) + } else { + Value::String("n/A - _id is missing".to_string()) + } } else { v[field].clone() } From 6ba4a1b62d81af5ad444bdedb9002ccdeecf600a Mon Sep 17 00:00:00 2001 From: hkernbach Date: Fri, 9 Aug 2024 16:40:57 +0200 Subject: [PATCH 4/4] remove dbg print --- tests/graph_loader.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/graph_loader.rs b/tests/graph_loader.rs index d9246694..46c60694 100644 --- a/tests/graph_loader.rs +++ b/tests/graph_loader.rs @@ -605,7 +605,6 @@ async fn init_named_graph_loader_with_data_all_v_and_e_attributes_all_by_boolean assert_eq!(edge_json_arr.len(), 1); assert_eq!(edge_field_names.len(), 0); let edge = &edge_json_arr[0]; - println!("Edge is {:?}", edge); assert_eq!(6, edge.as_object().unwrap().len()); // x, y, z and _id, _key, _rev