From db62c1962d0e90df20b4792c7042e48723e539d2 Mon Sep 17 00:00:00 2001 From: Jess Frazelle Date: Fri, 4 Apr 2025 16:07:48 -0700 Subject: [PATCH] ignore errors on the modeling cmds that are only for the artifact graph Signed-off-by: Jess Frazelle --- rust/kcl-lib/src/engine/conn.rs | 7 +++ rust/kcl-lib/src/engine/conn_mock.rs | 6 ++ rust/kcl-lib/src/engine/conn_wasm.rs | 6 ++ rust/kcl-lib/src/engine/mod.rs | 32 ++++++++++- rust/kcl-lib/src/std/args.rs | 11 ++++ rust/kcl-lib/src/std/extrude.rs | 76 ++++++++++++-------------- rust/kcl-lib/src/std/loft.rs | 1 - rust/kcl-lib/src/std/revolve.rs | 1 - rust/kcl-lib/src/std/sweep.rs | 1 - rust/kcl-to-core/src/conn_mock_core.rs | 6 ++ 10 files changed, 103 insertions(+), 44 deletions(-) diff --git a/rust/kcl-lib/src/engine/conn.rs b/rust/kcl-lib/src/engine/conn.rs index a8a18f6b3..3e993b6fb 100644 --- a/rust/kcl-lib/src/engine/conn.rs +++ b/rust/kcl-lib/src/engine/conn.rs @@ -51,6 +51,8 @@ pub struct EngineConnection { /// If the server sends session data, it'll be copied to here. session_data: Arc>>, + ignore_failed_responses: Arc>>, + execution_kind: Arc>, stats: EngineStats, } @@ -343,6 +345,7 @@ impl EngineConnection { batch_end: Arc::new(RwLock::new(IndexMap::new())), artifact_commands: Arc::new(RwLock::new(Vec::new())), default_planes: Default::default(), + ignore_failed_responses: Arc::new(RwLock::new(Vec::new())), session_data, execution_kind: Default::default(), stats: Default::default(), @@ -364,6 +367,10 @@ impl EngineManager for EngineConnection { self.responses.clone() } + fn ignore_failed_responses(&self) -> Arc>> { + self.ignore_failed_responses.clone() + } + fn artifact_commands(&self) -> Arc>> { self.artifact_commands.clone() } diff --git a/rust/kcl-lib/src/engine/conn_mock.rs b/rust/kcl-lib/src/engine/conn_mock.rs index 19d1d10b1..3bafe7055 100644 --- a/rust/kcl-lib/src/engine/conn_mock.rs +++ b/rust/kcl-lib/src/engine/conn_mock.rs @@ -32,6 +32,7 @@ pub struct EngineConnection { execution_kind: Arc>, /// The default planes for the scene. default_planes: Arc>>, + ignore_failed_responses: Arc>>, stats: EngineStats, } @@ -43,6 +44,7 @@ impl EngineConnection { artifact_commands: Arc::new(RwLock::new(Vec::new())), execution_kind: Default::default(), default_planes: Default::default(), + ignore_failed_responses: Arc::new(RwLock::new(Vec::new())), stats: Default::default(), }) } @@ -62,6 +64,10 @@ impl crate::engine::EngineManager for EngineConnection { Arc::new(RwLock::new(IndexMap::new())) } + fn ignore_failed_responses(&self) -> Arc>> { + self.ignore_failed_responses.clone() + } + fn stats(&self) -> &EngineStats { &self.stats } diff --git a/rust/kcl-lib/src/engine/conn_wasm.rs b/rust/kcl-lib/src/engine/conn_wasm.rs index 2242f8f83..a5ec302b9 100644 --- a/rust/kcl-lib/src/engine/conn_wasm.rs +++ b/rust/kcl-lib/src/engine/conn_wasm.rs @@ -43,6 +43,7 @@ pub struct EngineConnection { responses: Arc>>, artifact_commands: Arc>>, execution_kind: Arc>, + ignore_failed_responses: Arc>>, /// The default planes for the scene. default_planes: Arc>>, stats: EngineStats, @@ -63,6 +64,7 @@ impl EngineConnection { artifact_commands: Arc::new(RwLock::new(Vec::new())), execution_kind: Default::default(), default_planes: Default::default(), + ignore_failed_responses: Arc::new(RwLock::new(Vec::new())), stats: Default::default(), }) } @@ -156,6 +158,10 @@ impl crate::engine::EngineManager for EngineConnection { self.responses.clone() } + fn ignore_failed_responses(&self) -> Arc>> { + self.ignore_failed_responses.clone() + } + fn stats(&self) -> &EngineStats { &self.stats } diff --git a/rust/kcl-lib/src/engine/mod.rs b/rust/kcl-lib/src/engine/mod.rs index 6cef90c8e..fb373e355 100644 --- a/rust/kcl-lib/src/engine/mod.rs +++ b/rust/kcl-lib/src/engine/mod.rs @@ -90,6 +90,9 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { /// Get the command responses from the engine. fn responses(&self) -> Arc>>; + /// Ignore failed responses of these command ids. + fn ignore_failed_responses(&self) -> Arc>>; + /// Get the artifact commands that have accumulated so far. fn artifact_commands(&self) -> Arc>>; @@ -150,6 +153,7 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { async fn clear_queues(&self) { self.batch().write().await.clear(); self.batch_end().write().await.clear(); + self.ignore_failed_responses().write().await.clear(); } /// Send a modeling command and wait for the response message. @@ -296,6 +300,21 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { Ok(()) } + // Add a modeling command to the batch but don't fire it right away. + // Ignore the failure of this command. + async fn batch_modeling_cmd_ignore_failure( + &self, + id: uuid::Uuid, + source_range: SourceRange, + cmd: &ModelingCmd, + ) -> Result<(), crate::errors::KclError> { + self.ignore_failed_responses().write().await.push(id); + + self.batch_modeling_cmd(id, source_range, cmd).await?; + + Ok(()) + } + // Add a vector of modeling commands to the batch but don't fire it right away. // This allows you to force them all to be added together in the same order. // When we are running things in parallel this prevents race conditions that might come @@ -467,6 +486,7 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { if let OkWebSocketResponseData::ModelingBatch { responses } = response { let responses = responses.into_iter().map(|(k, v)| (Uuid::from(k), v)).collect(); self.parse_batch_responses(last_id.into(), id_to_source_range, responses) + .await } else { // We should never get here. Err(KclError::Engine(KclErrorDetails { @@ -657,7 +677,7 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { } } - fn parse_batch_responses( + async fn parse_batch_responses( &self, // The last response we are looking for. id: uuid::Uuid, @@ -685,6 +705,16 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static { } } BatchResponse::Failure { errors } => { + // If this was in our ignore list, we don't care about it. + if self + .ignore_failed_responses() + .read() + .await + .iter() + .any(|id| id == cmd_id) + { + continue; + } // Get the source range for the command. let source_range = id_to_source_range.get(cmd_id).cloned().ok_or_else(|| { KclError::Engine(KclErrorDetails { diff --git a/rust/kcl-lib/src/std/args.rs b/rust/kcl-lib/src/std/args.rs index b5cddd2fc..852ba87e4 100644 --- a/rust/kcl-lib/src/std/args.rs +++ b/rust/kcl-lib/src/std/args.rs @@ -370,6 +370,17 @@ impl Args { self.ctx.engine.batch_modeling_cmd(id, self.source_range, &cmd).await } + pub(crate) async fn batch_modeling_cmd_ignore_failure( + &self, + id: uuid::Uuid, + cmd: ModelingCmd, + ) -> Result<(), crate::errors::KclError> { + self.ctx + .engine + .batch_modeling_cmd_ignore_failure(id, self.source_range, &cmd) + .await + } + // Add multiple modeling commands to the batch but don't fire them right away. pub(crate) async fn batch_modeling_cmds(&self, cmds: &[ModelingCmdReq]) -> Result<(), crate::errors::KclError> { self.ctx.engine.batch_modeling_cmds(self.source_range, cmds).await diff --git a/rust/kcl-lib/src/std/extrude.rs b/rust/kcl-lib/src/std/extrude.rs index 6ab86edc0..8ac95db25 100644 --- a/rust/kcl-lib/src/std/extrude.rs +++ b/rust/kcl-lib/src/std/extrude.rs @@ -130,7 +130,6 @@ async fn inner_extrude( sketch, id.into(), length, - false, &NamedCapTags { start: tag_start.as_ref(), end: tag_end.as_ref(), @@ -155,7 +154,6 @@ pub(crate) async fn do_post_extrude<'a>( sketch: &Sketch, solid_id: ArtifactId, length: f64, - sectional: bool, named_cap_tags: &'a NamedCapTags<'a>, exec_state: &mut ExecState, args: &Args, @@ -208,45 +206,43 @@ pub(crate) async fn do_post_extrude<'a>( vec![] }; - // Face filtering attempt in order to resolve https://github.com/KittyCAD/modeling-app/issues/5328 - // We need to not run Solid3dGetOppositeEdge and Solid3dGetNextAdjacentEdge because it is too - // hard to know when they work or fail. - if !sectional { - for (curve_id, face_id) in face_infos - .iter() - .filter(|face_info| face_info.cap == ExtrusionFaceCapType::None) - .filter_map(|face_info| { - if let (Some(curve_id), Some(face_id)) = (face_info.curve_id, face_info.face_id) { - Some((curve_id, face_id)) - } else { - None - } - }) - { - // Batch these commands, because the Rust code doesn't actually care about the outcome. - // So, there's no need to await them. - // Instead, the Typescript codebases (which handles WebSocket sends when compiled via Wasm) - // uses this to build the artifact graph, which the UI needs. - args.batch_modeling_cmd( - exec_state.next_uuid(), - ModelingCmd::from(mcmd::Solid3dGetOppositeEdge { - edge_id: curve_id, - object_id: sketch.id, - face_id, - }), - ) - .await?; + for (curve_id, face_id) in face_infos + .iter() + .filter(|face_info| face_info.cap == ExtrusionFaceCapType::None) + .filter_map(|face_info| { + if let (Some(curve_id), Some(face_id)) = (face_info.curve_id, face_info.face_id) { + Some((curve_id, face_id)) + } else { + None + } + }) + { + // Batch these commands, because the Rust code doesn't actually care about the outcome. + // So, there's no need to await them. + // Instead, the Typescript codebases (which handles WebSocket sends when compiled via Wasm) + // uses this to build the artifact graph, which the UI needs. + // + // We ignore the failure here because if one of these fails, in the case of a sectional + // sweep, the whole model should not fail, this is merely for the artifact graph. + args.batch_modeling_cmd_ignore_failure( + exec_state.next_uuid(), + ModelingCmd::from(mcmd::Solid3dGetOppositeEdge { + edge_id: curve_id, + object_id: sketch.id, + face_id, + }), + ) + .await?; - args.batch_modeling_cmd( - exec_state.next_uuid(), - ModelingCmd::from(mcmd::Solid3dGetNextAdjacentEdge { - edge_id: curve_id, - object_id: sketch.id, - face_id, - }), - ) - .await?; - } + args.batch_modeling_cmd_ignore_failure( + exec_state.next_uuid(), + ModelingCmd::from(mcmd::Solid3dGetNextAdjacentEdge { + edge_id: curve_id, + object_id: sketch.id, + face_id, + }), + ) + .await?; } let Faces { diff --git a/rust/kcl-lib/src/std/loft.rs b/rust/kcl-lib/src/std/loft.rs index bb8e48db9..76b2c5ece 100644 --- a/rust/kcl-lib/src/std/loft.rs +++ b/rust/kcl-lib/src/std/loft.rs @@ -175,7 +175,6 @@ async fn inner_loft( &sketch, id.into(), 0.0, - false, &super::extrude::NamedCapTags { start: tag_start.as_ref(), end: tag_end.as_ref(), diff --git a/rust/kcl-lib/src/std/revolve.rs b/rust/kcl-lib/src/std/revolve.rs index fa1d5b86d..d194a32f8 100644 --- a/rust/kcl-lib/src/std/revolve.rs +++ b/rust/kcl-lib/src/std/revolve.rs @@ -107,7 +107,6 @@ async fn inner_revolve( sketch, id.into(), 0.0, - false, &super::extrude::NamedCapTags { start: tag_start.as_ref(), end: tag_end.as_ref(), diff --git a/rust/kcl-lib/src/std/sweep.rs b/rust/kcl-lib/src/std/sweep.rs index eaae78be6..6445b506c 100644 --- a/rust/kcl-lib/src/std/sweep.rs +++ b/rust/kcl-lib/src/std/sweep.rs @@ -211,7 +211,6 @@ async fn inner_sweep( sketch, id.into(), 0.0, - sectional, &super::extrude::NamedCapTags { start: tag_start.as_ref(), end: tag_end.as_ref(), diff --git a/rust/kcl-to-core/src/conn_mock_core.rs b/rust/kcl-to-core/src/conn_mock_core.rs index 9309c52fa..aae3afc68 100644 --- a/rust/kcl-to-core/src/conn_mock_core.rs +++ b/rust/kcl-to-core/src/conn_mock_core.rs @@ -24,6 +24,7 @@ pub struct EngineConnection { batch_end: Arc>>, core_test: Arc>, execution_kind: Arc>, + ignore_failed_responses: Arc>>, /// The default planes for the scene. default_planes: Arc>>, stats: EngineStats, @@ -39,6 +40,7 @@ impl EngineConnection { core_test: result, execution_kind: Default::default(), default_planes: Default::default(), + ignore_failed_responses: Default::default(), stats: Default::default(), }) } @@ -371,6 +373,10 @@ impl kcl_lib::EngineManager for EngineConnection { Arc::new(RwLock::new(IndexMap::new())) } + fn ignore_failed_responses(&self) -> Arc>> { + self.ignore_failed_responses.clone() + } + fn stats(&self) -> &EngineStats { &self.stats }