Fix a couple of panics (#5900)

* Ensure batches in the engine are cleared between scenes

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Avoid panicking reading arguments out of bounds

Signed-off-by: Nick Cameron <nrc@ncameron.org>

---------

Signed-off-by: Nick Cameron <nrc@ncameron.org>
This commit is contained in:
Nick Cameron
2025-03-20 15:59:16 +13:00
committed by GitHub
parent 9ddb4e629f
commit 79be72c5f0
4 changed files with 97 additions and 34 deletions

View File

@ -147,6 +147,11 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
source_range: SourceRange,
) -> Result<(), crate::errors::KclError>;
async fn clear_queues(&self) {
self.batch().write().await.clear();
self.batch_end().write().await.clear();
}
/// Send a modeling command and wait for the response message.
async fn inner_send_modeling_cmd(
&self,
@ -161,6 +166,9 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
id_generator: &mut IdGenerator,
source_range: SourceRange,
) -> Result<(), crate::errors::KclError> {
// Clear any batched commands leftover from previous scenes.
self.clear_queues().await;
self.batch_modeling_cmd(
uuid::Uuid::new_v4(),
source_range,

View File

@ -96,7 +96,7 @@ impl ExecutorContext {
module_id: ModuleId,
path: &ModulePath,
) -> Result<(Option<KclValue>, EnvironmentRef, Vec<String>), KclError> {
crate::log::log(format!("enter module {path} {}", exec_state.stack()));
crate::log::log(format!("enter module {path} {} {exec_kind:?}", exec_state.stack()));
let old_units = exec_state.length_unit();
let original_execution = self.engine.replace_execution_kind(exec_kind).await;
@ -1192,6 +1192,7 @@ impl Node<CallExpression> {
format!("`{fn_name}` is deprecated, see the docs for a recommended replacement"),
));
}
let op = if func.feature_tree_operation() {
let op_labeled_args = func
.args(false)

View File

@ -724,10 +724,17 @@ impl ExecutorContext {
.map_err(KclErrorWithOutputs::no_outputs)?;
let default_planes = self.engine.get_default_planes().read().await.clone();
let env_ref = self
let result = self
.execute_and_build_graph(&program.ast, exec_state, preserve_mem)
.await
.map_err(|e| {
.await;
crate::log::log(format!(
"Post interpretation KCL memory stats: {:#?}",
exec_state.stack().memory.stats
));
crate::log::log(format!("Engine stats: {:?}", self.engine.stats()));
let env_ref = result.map_err(|e| {
let module_id_to_module_path: IndexMap<ModuleId, ModulePath> = exec_state
.global
.path_to_source_id
@ -746,12 +753,6 @@ impl ExecutorContext {
)
})?;
crate::log::log(format!(
"Post interpretation KCL memory stats: {:#?}",
exec_state.stack().memory.stats
));
crate::log::log(format!("Engine stats: {:?}", self.engine.stats()));
if !self.is_mock() {
let mut mem = exec_state.stack().deep_clone();
mem.restore_env(env_ref);
@ -786,6 +787,10 @@ impl ExecutorContext {
)
.await;
// If we errored out and early-returned, there might be commands which haven't been executed
// and should be dropped.
self.engine.clear_queues().await;
// Move the artifact commands and responses to simplify cache management
// and error creation.
exec_state
@ -1589,6 +1594,18 @@ const bracket = startSketchOn(XY)
parse_execute(ast).await.unwrap();
}
#[tokio::test(flavor = "multi_thread")]
async fn test_bad_arg_count_std() {
let ast = "startSketchOn(XY)
|> startProfileAt([0, 0], %)
|> profileStartX()";
assert!(parse_execute(ast)
.await
.unwrap_err()
.message()
.contains("Expected a sketch argument"));
}
#[tokio::test(flavor = "multi_thread")]
async fn test_unary_operator_not_succeeds() {
let ast = r#"

View File

@ -589,13 +589,19 @@ impl Args {
}
pub(crate) fn get_sketches(&self, exec_state: &mut ExecState) -> Result<(Vec<Sketch>, Sketch), KclError> {
let sarg = self.args[0]
let Some(arg0) = self.args.first() else {
return Err(KclError::Semantic(KclErrorDetails {
message: "Expected a sketch argument".to_owned(),
source_ranges: vec![self.source_range],
}));
};
let sarg = arg0
.value
.coerce(&RuntimeType::Array(PrimitiveType::Sketch, ArrayLen::None), exec_state)
.ok_or(KclError::Type(KclErrorDetails {
message: format!(
"Expected an array of sketches, found {}",
self.args[0].value.human_friendly_type()
arg0.value.human_friendly_type()
),
source_ranges: vec![self.source_range],
}))?;
@ -603,11 +609,18 @@ impl Args {
KclValue::HomArray { value, .. } => value.iter().map(|v| v.as_sketch().unwrap().clone()).collect(),
_ => unreachable!(),
};
let sarg = self.args[1]
let Some(arg1) = self.args.get(1) else {
return Err(KclError::Semantic(KclErrorDetails {
message: "Expected a second sketch argument".to_owned(),
source_ranges: vec![self.source_range],
}));
};
let sarg = arg1
.value
.coerce(&RuntimeType::Primitive(PrimitiveType::Sketch), exec_state)
.ok_or(KclError::Type(KclErrorDetails {
message: format!("Expected a sketch, found {}", self.args[1].value.human_friendly_type()),
message: format!("Expected a sketch, found {}", arg1.value.human_friendly_type()),
source_ranges: vec![self.source_range],
}))?;
let sketch = match sarg {
@ -619,11 +632,17 @@ impl Args {
}
pub(crate) fn get_sketch(&self, exec_state: &mut ExecState) -> Result<Sketch, KclError> {
let sarg = self.args[0]
let Some(arg0) = self.args.first() else {
return Err(KclError::Semantic(KclErrorDetails {
message: "Expected a sketch argument".to_owned(),
source_ranges: vec![self.source_range],
}));
};
let sarg = arg0
.value
.coerce(&RuntimeType::Primitive(PrimitiveType::Sketch), exec_state)
.ok_or(KclError::Type(KclErrorDetails {
message: format!("Expected a sketch, found {}", self.args[0].value.human_friendly_type()),
message: format!("Expected a sketch, found {}", arg0.value.human_friendly_type()),
source_ranges: vec![self.source_range],
}))?;
match sarg {
@ -658,13 +677,19 @@ impl Args {
T: serde::de::DeserializeOwned + FromArgs<'a>,
{
let data: T = FromArgs::from_args(self, 0)?;
let sarg = self.args[1]
let Some(arg1) = self.args.get(1) else {
return Err(KclError::Semantic(KclErrorDetails {
message: "Expected one or more sketches for second argument".to_owned(),
source_ranges: vec![self.source_range],
}));
};
let sarg = arg1
.value
.coerce(&RuntimeType::Array(PrimitiveType::Sketch, ArrayLen::None), exec_state)
.ok_or(KclError::Type(KclErrorDetails {
message: format!(
"Expected an array of sketches for second argument, found {}",
self.args[1].value.human_friendly_type()
"Expected one or more sketches for second argument, found {}",
arg1.value.human_friendly_type()
),
source_ranges: vec![self.source_range],
}))?;
@ -683,13 +708,19 @@ impl Args {
T: serde::de::DeserializeOwned + FromKclValue<'a> + Sized,
{
let data: T = FromArgs::from_args(self, 0)?;
let sarg = self.args[1]
let Some(arg1) = self.args.get(1) else {
return Err(KclError::Semantic(KclErrorDetails {
message: "Expected a sketch for second argument".to_owned(),
source_ranges: vec![self.source_range],
}));
};
let sarg = arg1
.value
.coerce(&RuntimeType::Primitive(PrimitiveType::Sketch), exec_state)
.ok_or(KclError::Type(KclErrorDetails {
message: format!(
"Expected a sketch for second argument, found {}",
self.args[1].value.human_friendly_type()
arg1.value.human_friendly_type()
),
source_ranges: vec![self.source_range],
}))?;
@ -713,13 +744,19 @@ impl Args {
T: serde::de::DeserializeOwned + FromKclValue<'a> + Sized,
{
let data: T = FromArgs::from_args(self, 0)?;
let sarg = self.args[1]
let Some(arg1) = self.args.get(1) else {
return Err(KclError::Semantic(KclErrorDetails {
message: "Expected a solid for second argument".to_owned(),
source_ranges: vec![self.source_range],
}));
};
let sarg = arg1
.value
.coerce(&RuntimeType::Primitive(PrimitiveType::Solid), exec_state)
.ok_or(KclError::Type(KclErrorDetails {
message: format!(
"Expected a solid for second argument, found {}",
self.args[1].value.human_friendly_type()
arg1.value.human_friendly_type()
),
source_ranges: vec![self.source_range],
}))?;