From 1de5377b4054c0ed19fe95a1837c628f50f0d843 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 16:51:59 -0400 Subject: [PATCH 1/9] test: Add retries to tests for engine flakiness --- src/cmd_alias.rs | 1 + src/cmd_auth.rs | 1 + src/cmd_completion.rs | 1 + src/cmd_config.rs | 2 + src/cmd_file.rs | 1 + src/cmd_generate.rs | 2 + src/cmd_kcl.rs | 28 +-- src/cmd_say.rs | 1 + src/cmd_user.rs | 1 + src/context.rs | 491 ++++++++++++++++++++++++++++++++++++++++-- src/tests.rs | 1 + 11 files changed, 490 insertions(+), 40 deletions(-) diff --git a/src/cmd_alias.rs b/src/cmd_alias.rs index f65977cd..111e93d1 100644 --- a/src/cmd_alias.rs +++ b/src/cmd_alias.rs @@ -432,6 +432,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let cmd_alias = crate::cmd_alias::CmdAlias { subcmd: t.cmd }; diff --git a/src/cmd_auth.rs b/src/cmd_auth.rs index 63c609e5..d0f9cb08 100644 --- a/src/cmd_auth.rs +++ b/src/cmd_auth.rs @@ -614,6 +614,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; if let Some(h) = &t.host { ctx.override_host = Some(h.clone()); diff --git a/src/cmd_completion.rs b/src/cmd_completion.rs index 3742cad4..660240f4 100644 --- a/src/cmd_completion.rs +++ b/src/cmd_completion.rs @@ -139,6 +139,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; cmd.run(&mut ctx).await.unwrap(); diff --git a/src/cmd_config.rs b/src/cmd_config.rs index d6967b22..17c354cd 100644 --- a/src/cmd_config.rs +++ b/src/cmd_config.rs @@ -265,6 +265,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let cmd_config = crate::cmd_config::CmdConfig { subcmd: t.cmd }; @@ -301,6 +302,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let mut cmd_config = crate::cmd_config::CmdConfig { diff --git a/src/cmd_file.rs b/src/cmd_file.rs index c9f7a991..e56b3c7a 100644 --- a/src/cmd_file.rs +++ b/src/cmd_file.rs @@ -868,6 +868,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let cmd_file = crate::cmd_file::CmdFile { subcmd: t.cmd }; diff --git a/src/cmd_generate.rs b/src/cmd_generate.rs index a93205cf..e29fb75f 100644 --- a/src/cmd_generate.rs +++ b/src/cmd_generate.rs @@ -163,6 +163,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let cmd = crate::cmd_generate::CmdGenerateMarkdown { dir: "".to_string() }; @@ -192,6 +193,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let cmd = crate::cmd_generate::CmdGenerateMarkdown { dir: "".to_string() }; diff --git a/src/cmd_kcl.rs b/src/cmd_kcl.rs index 4bc8e5ce..abe1aae0 100644 --- a/src/cmd_kcl.rs +++ b/src/cmd_kcl.rs @@ -157,25 +157,17 @@ impl crate::cmd::Command for CmdKclExport { .map_err(|err| kcl_error_fmt::into_miette_for_parse(&filepath.display().to_string(), &code, err))?; let meta_settings = program.meta_settings()?.unwrap_or_default(); let units: UnitLength = meta_settings.default_length_units.to_kcmc(); + let output_format = get_output_format(&self.output_format, units, self.deterministic); - let client = ctx.api_client("")?; - let ectx = kcl_lib::ExecutorContext::new(&client, settings).await?; - let mut state = kcl_lib::ExecState::new(&ectx); - let session_data = ectx - .run(&program, &mut state) - .await - .map_err(|err| kcl_error_fmt::into_miette(err, &code))? - .1; - kcl_error_fmt::check_exec_state_issues( - &mut ctx.io.err_out, - &filepath.display().to_string(), - &code, - &state, - self.run_options.issue_check(), - )?; - - let files = ectx - .export(get_output_format(&self.output_format, units, self.deterministic)) + let (files, session_data) = ctx + .run_kcl_then_export( + &filepath.display().to_string(), + &code, + &program, + settings, + self.run_options.issue_check(), + output_format, + ) .await?; // Save the files to our export directory. diff --git a/src/cmd_say.rs b/src/cmd_say.rs index f2bef2fb..54cbff5e 100644 --- a/src/cmd_say.rs +++ b/src/cmd_say.rs @@ -116,6 +116,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let cmd_say = crate::cmd_say::CmdSay { input: t.cmd.input }; diff --git a/src/cmd_user.rs b/src/cmd_user.rs index 90782278..47a88e3e 100644 --- a/src/cmd_user.rs +++ b/src/cmd_user.rs @@ -85,6 +85,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let cmd_user = crate::cmd_user::CmdUser { subcmd: t.cmd }; diff --git a/src/context.rs b/src/context.rs index fe03b9d6..367856f5 100644 --- a/src/context.rs +++ b/src/context.rs @@ -5,7 +5,11 @@ use futures::StreamExt; use kcl_lib::engine_connection::EngineManager; use kcmc::{each_cmd as mcmd, websocket::OkWebSocketResponseData}; use kittycad::types::{ApiCallStatus, AsyncApiCallOutput, TextToCad, TextToCadCreateBody, TextToCadMultiFileIteration}; -use kittycad_modeling_cmds::{self as kcmc, ModelingCmd, output::TakeSnapshot, websocket::ModelingSessionData}; +use kittycad_modeling_cmds::{ + self as kcmc, ModelingCmd, + output::TakeSnapshot, + websocket::{ModelingSessionData, RawFile}, +}; use tokio_tungstenite::{WebSocketStream, tungstenite::protocol::Role}; use crate::{cmd_kcl, config::Config, config_file::get_env_var, kcl_error_fmt, types::FormatOutput}; @@ -16,6 +20,130 @@ pub struct Context<'a> { pub debug: bool, // If set, override the host used when commands don't specify one. pub(crate) override_host: Option, + #[cfg(test)] + pub(crate) kcl_retry_config: Option, +} + +#[cfg(test)] +#[derive(Debug, Clone)] +pub(crate) struct RetryConfig { + pub(crate) retries: usize, + pub(crate) print_retries: bool, +} + +#[cfg(test)] +impl Default for RetryConfig { + fn default() -> Self { + Self { + retries: 2, + print_retries: true, + } + } +} + +#[cfg(test)] +enum KclExecutionError { + Setup(anyhow::Error), + Execution(Box), + Kcl(Box), + Issue { err: anyhow::Error, stderr: Vec }, +} + +#[cfg(test)] +type KclRetryResult = std::result::Result<(T, Vec), KclExecutionError>; + +#[cfg(test)] +type KclProgramRun = ( + kcl_lib::ExecutorContext, + kcl_lib::ExecState, + Option, +); + +#[cfg(test)] +impl KclExecutionError { + fn stderr(&self) -> Option<&[u8]> { + match self { + Self::Issue { stderr, .. } => Some(stderr), + _ => None, + } + } + + fn into_anyhow(self, filename: &str, code: &str) -> anyhow::Error { + match self { + Self::Setup(err) => err, + Self::Execution(err) => kcl_error_fmt::into_miette(*err, code), + Self::Kcl(err) => kcl_error_fmt::into_miette_for_parse(filename, code, *err), + Self::Issue { err, .. } => err, + } + } +} + +#[cfg(test)] +impl std::fmt::Display for KclExecutionError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Setup(err) => write!(formatter, "{err}"), + Self::Execution(err) => write!(formatter, "{err}"), + Self::Kcl(err) => write!(formatter, "{err}"), + Self::Issue { err, .. } => write!(formatter, "{err}"), + } + } +} + +#[cfg(test)] +impl kcl_lib::IsRetryable for KclExecutionError { + fn is_retryable(&self) -> bool { + match self { + Self::Setup(_) => false, + Self::Execution(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), + Self::Kcl(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), + Self::Issue { .. } => false, + } + } +} + +#[cfg(test)] +async fn execute_with_retries(config: &RetryConfig, mut execute: F) -> std::result::Result +where + F: FnMut() -> Fut, + Fut: std::future::Future>, + E: kcl_lib::IsRetryable + std::fmt::Display, +{ + let mut retries_remaining = config.retries; + loop { + let exec_result = execute().await; + + if retries_remaining > 0 + && let Err(error) = &exec_result + && error.is_retryable() + { + if config.print_retries { + eprintln!("Execute got {error}; retrying..."); + } + retries_remaining -= 1; + continue; + } + + return exec_result; + } +} + +#[cfg(test)] +async fn run_kcl_program_once_with_client( + client: &kittycad::Client, + program: &kcl_lib::Program, + settings: kcl_lib::ExecutorSettings, +) -> std::result::Result { + let ctx = kcl_lib::ExecutorContext::new(client, settings) + .await + .map_err(KclExecutionError::Setup)?; + let mut state = kcl_lib::ExecState::new(&ctx); + let session_data = ctx + .run(program, &mut state) + .await + .map_err(|err| KclExecutionError::Execution(Box::new(err)))? + .1; + Ok((ctx, state, session_data)) } impl<'a> Context<'a> { @@ -92,6 +220,8 @@ impl<'a> Context<'a> { io, debug: false, override_host: None, + #[cfg(test)] + kcl_retry_config: None, } } @@ -223,6 +353,27 @@ impl<'a> Context<'a> { Ok(engine) } + #[cfg(test)] + fn finish_kcl_retry_result( + &mut self, + filename: &str, + code: &str, + result: std::result::Result<(T, Vec), KclExecutionError>, + ) -> Result { + match result { + Ok((value, stderr)) => { + self.io.err_out.write_all(&stderr)?; + Ok(value) + } + Err(err) => { + if let Some(stderr) = err.stderr() { + self.io.err_out.write_all(stderr)?; + } + Err(err.into_anyhow(filename, code)) + } + } + } + pub async fn send_kcl_modeling_cmd( &mut self, hostname: &str, @@ -237,13 +388,65 @@ impl<'a> Context<'a> { let program = kcl_lib::Program::parse_no_errs(code) .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - let ctx = kcl_lib::ExecutorContext::new(&client, cmd_kcl::with_heartbeats(settings)).await?; - let mut state = kcl_lib::ExecState::new(&ctx); - let session_data = ctx - .run(&program, &mut state) - .await - .map_err(|err| kcl_error_fmt::into_miette(err, code))? - .1; + let settings = cmd_kcl::with_heartbeats(settings); + #[cfg(test)] + if let Some(retry_config) = self.kcl_retry_config.clone() { + let result: KclRetryResult<(OkWebSocketResponseData, Option)> = + execute_with_retries(&retry_config, || { + let settings = settings.clone(); + let cmd = cmd.clone(); + let client = &client; + let program = &program; + async move { + let (ctx, state, session_data) = + run_kcl_program_once_with_client(client, program, settings).await?; + + let mut stderr = Vec::new(); + if let Err(err) = + kcl_error_fmt::check_exec_state_issues(&mut stderr, filename, code, &state, issue_check) + { + return Err(KclExecutionError::Issue { err, stderr }); + } + + let batch_context = kcl_lib::EngineBatchContext::new(); + + ctx.engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &ModelingCmd::from( + mcmd::ZoomToFit::builder() + .animated(false) + .object_ids(Default::default()) + .padding(0.1) + .build(), + ), + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + + let resp = ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + Ok(((resp, session_data), stderr)) + } + }) + .await; + + return self.finish_kcl_retry_result(filename, code, result); + } + + let (ctx, state, session_data) = self + .run_kcl_program_with_client(&client, code, &program, settings) + .await?; kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check)?; let batch_context = kcl_lib::EngineBatchContext::new(); @@ -291,13 +494,53 @@ impl<'a> Context<'a> { let program = kcl_lib::Program::parse_no_errs(code) .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - let ctx = kcl_lib::ExecutorContext::new(&client, cmd_kcl::with_heartbeats(settings)).await?; - let mut state = kcl_lib::ExecState::new(&ctx); - let session_data = ctx - .run(&program, &mut state) - .await - .map_err(|err| kcl_error_fmt::into_miette(err, code))? - .1; + let settings = cmd_kcl::with_heartbeats(settings); + #[cfg(test)] + if let Some(retry_config) = self.kcl_retry_config.clone() { + let result: KclRetryResult<(Vec, Option)> = + execute_with_retries(&retry_config, || { + let settings = settings.clone(); + let cmds = cmds.clone(); + let client = &client; + let program = &program; + async move { + let (ctx, state, session_data) = + run_kcl_program_once_with_client(client, program, settings).await?; + + let mut stderr = Vec::new(); + if let Err(err) = + kcl_error_fmt::check_exec_state_issues(&mut stderr, filename, code, &state, issue_check) + { + return Err(KclExecutionError::Issue { err, stderr }); + } + + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut responses = Vec::with_capacity(cmds.len()); + for cmd in cmds { + let resp = ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + responses.push(resp); + } + + Ok(((responses, session_data), stderr)) + } + }) + .await; + + return self.finish_kcl_retry_result(filename, code, result); + } + + let (ctx, state, session_data) = self + .run_kcl_program_with_client(&client, code, &program, settings) + .await?; kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check)?; let batch_context = kcl_lib::EngineBatchContext::new(); @@ -335,13 +578,59 @@ impl<'a> Context<'a> { let program = kcl_lib::Program::parse_no_errs(code) .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - let ctx = kcl_lib::ExecutorContext::new(&client, cmd_kcl::with_heartbeats(settings)).await?; - let mut state = kcl_lib::ExecState::new(&ctx); - let session_data = ctx - .run(&program, &mut state) - .await - .map_err(|err| kcl_error_fmt::into_miette(err, code))? - .1; + let settings = cmd_kcl::with_heartbeats(settings); + #[cfg(test)] + if let Some(retry_config) = self.kcl_retry_config.clone() { + let result: KclRetryResult<(Vec, Option)> = + execute_with_retries(&retry_config, || { + let settings = settings.clone(); + let snapshot_cmds = snapshot_cmds.clone(); + let client = &client; + let program = &program; + async move { + let (ctx, state, session_data) = + run_kcl_program_once_with_client(client, program, settings).await?; + + let mut stderr = Vec::new(); + if let Err(err) = + kcl_error_fmt::check_exec_state_issues(&mut stderr, filename, code, &state, issue_check) + { + return Err(KclExecutionError::Issue { err, stderr }); + } + + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut snapshot_resps = Vec::new(); + for snapshot_cmd in snapshot_cmds { + let resp = ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &snapshot_cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + if let OkWebSocketResponseData::Modeling { + modeling_response: + kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), + } = resp + { + snapshot_resps.push(snap); + } + } + + Ok(((snapshot_resps, session_data), stderr)) + } + }) + .await; + + return self.finish_kcl_retry_result(filename, code, result); + } + + let (ctx, state, session_data) = self + .run_kcl_program_with_client(&client, code, &program, settings) + .await?; kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check)?; let batch_context = kcl_lib::EngineBatchContext::new(); @@ -368,6 +657,98 @@ impl<'a> Context<'a> { Ok((snapshot_resps, session_data)) } + pub(crate) async fn run_kcl_then_export( + &mut self, + filename: &str, + code: &str, + program: &kcl_lib::Program, + settings: kcl_lib::ExecutorSettings, + issue_check: kcl_error_fmt::KclIssueCheck, + output_format: kittycad_modeling_cmds::format::OutputFormat3d, + ) -> Result<(Vec, Option)> { + let client = self.api_client("")?; + + #[cfg(test)] + if let Some(retry_config) = self.kcl_retry_config.clone() { + let result: KclRetryResult<(Vec, Option)> = + execute_with_retries(&retry_config, || { + let settings = settings.clone(); + let output_format = output_format.clone(); + let client = &client; + async move { + let (ctx, state, session_data) = + run_kcl_program_once_with_client(client, program, settings).await?; + + let mut stderr = Vec::new(); + if let Err(err) = + kcl_error_fmt::check_exec_state_issues(&mut stderr, filename, code, &state, issue_check) + { + return Err(KclExecutionError::Issue { err, stderr }); + } + + let files = ctx + .export(output_format) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + Ok(((files, session_data), stderr)) + } + }) + .await; + + return self.finish_kcl_retry_result(filename, code, result); + } + + let (ctx, state, session_data) = self + .run_kcl_program_with_client(&client, code, program, settings) + .await?; + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check)?; + + let files = ctx + .export(output_format) + .await + .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; + + Ok((files, session_data)) + } + + async fn run_kcl_program_with_client( + &self, + client: &kittycad::Client, + code: &str, + program: &kcl_lib::Program, + settings: kcl_lib::ExecutorSettings, + ) -> Result<( + kcl_lib::ExecutorContext, + kcl_lib::ExecState, + Option, + )> { + #[cfg(test)] + if let Some(retry_config) = &self.kcl_retry_config { + let result: std::result::Result< + ( + kcl_lib::ExecutorContext, + kcl_lib::ExecState, + Option, + ), + KclExecutionError, + > = execute_with_retries(retry_config, || { + run_kcl_program_once_with_client(client, program, settings.clone()) + }) + .await; + + return result.map_err(|err| err.into_anyhow("", code)); + } + + let ctx = kcl_lib::ExecutorContext::new(client, settings).await?; + let mut state = kcl_lib::ExecState::new(&ctx); + let session_data = ctx + .run(program, &mut state) + .await + .map_err(|err| kcl_error_fmt::into_miette(err, code))? + .1; + Ok((ctx, state, session_data)) + } + /// Create and poll a plain text-to-CAD job. /// /// `show_reasoning` is accepted to keep the command-facing API aligned with @@ -1048,6 +1429,27 @@ mod test { use super::*; + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum RetryTestError { + Retryable, + Fatal, + } + + impl std::fmt::Display for RetryTestError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Retryable => write!(formatter, "retryable"), + Self::Fatal => write!(formatter, "fatal"), + } + } + } + + impl kcl_lib::IsRetryable for RetryTestError { + fn is_retryable(&self) -> bool { + matches!(self, Self::Retryable) + } + } + pub struct TestItem { name: String, zoo_pager_env: String, @@ -1284,6 +1686,49 @@ mod test { assert_eq!(md, "Hello world"); } + #[tokio::test] + async fn execute_with_retries_retries_retryable_errors() { + let retry_config = RetryConfig { + retries: 2, + print_retries: false, + }; + let mut attempts = 0; + + let result: std::result::Result<&str, RetryTestError> = execute_with_retries(&retry_config, || { + attempts += 1; + let attempt = attempts; + async move { + if attempt < 3 { + Err(RetryTestError::Retryable) + } else { + Ok("ok") + } + } + }) + .await; + + assert_eq!(result, Ok("ok")); + assert_eq!(attempts, 3); + } + + #[tokio::test] + async fn execute_with_retries_does_not_retry_fatal_errors() { + let retry_config = RetryConfig { + retries: 2, + print_retries: false, + }; + let mut attempts = 0; + + let result: std::result::Result<(), RetryTestError> = execute_with_retries(&retry_config, || { + attempts += 1; + async { Err(RetryTestError::Fatal) } + }) + .await; + + assert_eq!(result, Err(RetryTestError::Fatal)); + assert_eq!(attempts, 1); + } + #[test] fn resolve_host_prefers_explicit_then_global() { let mut config = crate::config::new_blank_config().unwrap(); @@ -1294,6 +1739,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; // No override: falls back to default host in config (which will be DEFAULT_HOST initially) @@ -1327,6 +1773,7 @@ mod test { io, debug: false, override_host: None, + kcl_retry_config: None, }; let (files, filepath) = ctx diff --git a/src/tests.rs b/src/tests.rs index aee0566a..ce3052b7 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -294,6 +294,7 @@ async fn run_test_item_with_config(config: &mut TestConfig, item: TestItem) { io, debug: false, override_host: None, + kcl_retry_config: Some(crate::context::RetryConfig::default()), }; let _cwd_guard = CurrentDirGuard::change_to(item.current_directory.as_deref()) From afd12b139c163a8b232b61caba56b7ef6b9fa068 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 17:13:33 -0400 Subject: [PATCH 2/9] Remove unneeded stderr buffering --- src/context.rs | 340 +++++++++++++++++++++++-------------------------- 1 file changed, 157 insertions(+), 183 deletions(-) diff --git a/src/context.rs b/src/context.rs index 367856f5..07f179a6 100644 --- a/src/context.rs +++ b/src/context.rs @@ -46,12 +46,9 @@ enum KclExecutionError { Setup(anyhow::Error), Execution(Box), Kcl(Box), - Issue { err: anyhow::Error, stderr: Vec }, + Issue(anyhow::Error), } -#[cfg(test)] -type KclRetryResult = std::result::Result<(T, Vec), KclExecutionError>; - #[cfg(test)] type KclProgramRun = ( kcl_lib::ExecutorContext, @@ -61,19 +58,12 @@ type KclProgramRun = ( #[cfg(test)] impl KclExecutionError { - fn stderr(&self) -> Option<&[u8]> { - match self { - Self::Issue { stderr, .. } => Some(stderr), - _ => None, - } - } - fn into_anyhow(self, filename: &str, code: &str) -> anyhow::Error { match self { Self::Setup(err) => err, Self::Execution(err) => kcl_error_fmt::into_miette(*err, code), Self::Kcl(err) => kcl_error_fmt::into_miette_for_parse(filename, code, *err), - Self::Issue { err, .. } => err, + Self::Issue(err) => err, } } } @@ -85,7 +75,7 @@ impl std::fmt::Display for KclExecutionError { Self::Setup(err) => write!(formatter, "{err}"), Self::Execution(err) => write!(formatter, "{err}"), Self::Kcl(err) => write!(formatter, "{err}"), - Self::Issue { err, .. } => write!(formatter, "{err}"), + Self::Issue(err) => write!(formatter, "{err}"), } } } @@ -97,7 +87,7 @@ impl kcl_lib::IsRetryable for KclExecutionError { Self::Setup(_) => false, Self::Execution(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), Self::Kcl(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), - Self::Issue { .. } => false, + Self::Issue(_) => false, } } } @@ -128,6 +118,26 @@ where } } +#[cfg(test)] +fn should_retry_kcl_attempt( + config: &RetryConfig, + retries_remaining: &mut usize, + result: &std::result::Result, +) -> bool { + if *retries_remaining > 0 + && let Err(error) = result + && kcl_lib::IsRetryable::is_retryable(error) + { + if config.print_retries { + eprintln!("Execute got {error}; retrying..."); + } + *retries_remaining -= 1; + true + } else { + false + } +} + #[cfg(test)] async fn run_kcl_program_once_with_client( client: &kittycad::Client, @@ -353,27 +363,6 @@ impl<'a> Context<'a> { Ok(engine) } - #[cfg(test)] - fn finish_kcl_retry_result( - &mut self, - filename: &str, - code: &str, - result: std::result::Result<(T, Vec), KclExecutionError>, - ) -> Result { - match result { - Ok((value, stderr)) => { - self.io.err_out.write_all(&stderr)?; - Ok(value) - } - Err(err) => { - if let Some(stderr) = err.stderr() { - self.io.err_out.write_all(stderr)?; - } - Err(err.into_anyhow(filename, code)) - } - } - } - pub async fn send_kcl_modeling_cmd( &mut self, hostname: &str, @@ -391,57 +380,53 @@ impl<'a> Context<'a> { let settings = cmd_kcl::with_heartbeats(settings); #[cfg(test)] if let Some(retry_config) = self.kcl_retry_config.clone() { - let result: KclRetryResult<(OkWebSocketResponseData, Option)> = - execute_with_retries(&retry_config, || { - let settings = settings.clone(); - let cmd = cmd.clone(); - let client = &client; - let program = &program; - async move { - let (ctx, state, session_data) = - run_kcl_program_once_with_client(client, program, settings).await?; - - let mut stderr = Vec::new(); - if let Err(err) = - kcl_error_fmt::check_exec_state_issues(&mut stderr, filename, code, &state, issue_check) - { - return Err(KclExecutionError::Issue { err, stderr }); - } - - let batch_context = kcl_lib::EngineBatchContext::new(); - - ctx.engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &ModelingCmd::from( - mcmd::ZoomToFit::builder() - .animated(false) - .object_ids(Default::default()) - .padding(0.1) - .build(), - ), - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - - let resp = ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &cmd, - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - Ok(((resp, session_data), stderr)) - } - }) + let mut retries_remaining = retry_config.retries; + loop { + let result = async { + let (ctx, state, session_data) = + run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check) + .map_err(KclExecutionError::Issue)?; + + let batch_context = kcl_lib::EngineBatchContext::new(); + + ctx.engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &ModelingCmd::from( + mcmd::ZoomToFit::builder() + .animated(false) + .object_ids(Default::default()) + .padding(0.1) + .build(), + ), + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + + let resp = ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + Ok((resp, session_data)) + } .await; - return self.finish_kcl_retry_result(filename, code, result); + if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { + continue; + } + + return result.map_err(|err| err.into_anyhow(filename, code)); + } } let (ctx, state, session_data) = self @@ -497,45 +482,41 @@ impl<'a> Context<'a> { let settings = cmd_kcl::with_heartbeats(settings); #[cfg(test)] if let Some(retry_config) = self.kcl_retry_config.clone() { - let result: KclRetryResult<(Vec, Option)> = - execute_with_retries(&retry_config, || { - let settings = settings.clone(); - let cmds = cmds.clone(); - let client = &client; - let program = &program; - async move { - let (ctx, state, session_data) = - run_kcl_program_once_with_client(client, program, settings).await?; - - let mut stderr = Vec::new(); - if let Err(err) = - kcl_error_fmt::check_exec_state_issues(&mut stderr, filename, code, &state, issue_check) - { - return Err(KclExecutionError::Issue { err, stderr }); - } - - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut responses = Vec::with_capacity(cmds.len()); - for cmd in cmds { - let resp = ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &cmd, - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - responses.push(resp); - } - - Ok(((responses, session_data), stderr)) + let mut retries_remaining = retry_config.retries; + loop { + let result = async { + let (ctx, state, session_data) = + run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check) + .map_err(KclExecutionError::Issue)?; + + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut responses = Vec::with_capacity(cmds.len()); + for cmd in cmds.clone() { + let resp = ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + responses.push(resp); } - }) + + Ok((responses, session_data)) + } .await; - return self.finish_kcl_retry_result(filename, code, result); + if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { + continue; + } + + return result.map_err(|err| err.into_anyhow(filename, code)); + } } let (ctx, state, session_data) = self @@ -581,51 +562,47 @@ impl<'a> Context<'a> { let settings = cmd_kcl::with_heartbeats(settings); #[cfg(test)] if let Some(retry_config) = self.kcl_retry_config.clone() { - let result: KclRetryResult<(Vec, Option)> = - execute_with_retries(&retry_config, || { - let settings = settings.clone(); - let snapshot_cmds = snapshot_cmds.clone(); - let client = &client; - let program = &program; - async move { - let (ctx, state, session_data) = - run_kcl_program_once_with_client(client, program, settings).await?; - - let mut stderr = Vec::new(); - if let Err(err) = - kcl_error_fmt::check_exec_state_issues(&mut stderr, filename, code, &state, issue_check) + let mut retries_remaining = retry_config.retries; + loop { + let result = async { + let (ctx, state, session_data) = + run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check) + .map_err(KclExecutionError::Issue)?; + + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut snapshot_resps = Vec::new(); + for snapshot_cmd in snapshot_cmds.clone() { + let resp = ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &snapshot_cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + if let OkWebSocketResponseData::Modeling { + modeling_response: + kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), + } = resp { - return Err(KclExecutionError::Issue { err, stderr }); - } - - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut snapshot_resps = Vec::new(); - for snapshot_cmd in snapshot_cmds { - let resp = ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &snapshot_cmd, - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - if let OkWebSocketResponseData::Modeling { - modeling_response: - kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), - } = resp - { - snapshot_resps.push(snap); - } + snapshot_resps.push(snap); } - - Ok(((snapshot_resps, session_data), stderr)) } - }) + + Ok((snapshot_resps, session_data)) + } .await; - return self.finish_kcl_retry_result(filename, code, result); + if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { + continue; + } + + return result.map_err(|err| err.into_anyhow(filename, code)); + } } let (ctx, state, session_data) = self @@ -670,32 +647,29 @@ impl<'a> Context<'a> { #[cfg(test)] if let Some(retry_config) = self.kcl_retry_config.clone() { - let result: KclRetryResult<(Vec, Option)> = - execute_with_retries(&retry_config, || { - let settings = settings.clone(); - let output_format = output_format.clone(); - let client = &client; - async move { - let (ctx, state, session_data) = - run_kcl_program_once_with_client(client, program, settings).await?; - - let mut stderr = Vec::new(); - if let Err(err) = - kcl_error_fmt::check_exec_state_issues(&mut stderr, filename, code, &state, issue_check) - { - return Err(KclExecutionError::Issue { err, stderr }); - } + let mut retries_remaining = retry_config.retries; + loop { + let result = async { + let (ctx, state, session_data) = + run_kcl_program_once_with_client(&client, program, settings.clone()).await?; - let files = ctx - .export(output_format) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - Ok(((files, session_data), stderr)) - } - }) + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check) + .map_err(KclExecutionError::Issue)?; + + let files = ctx + .export(output_format.clone()) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + Ok((files, session_data)) + } .await; - return self.finish_kcl_retry_result(filename, code, result); + if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { + continue; + } + + return result.map_err(|err| err.into_anyhow(filename, code)); + } } let (ctx, state, session_data) = self From 626cd6d2a4c543f7d0754b0ff4a15ce8d1363629 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 17:34:31 -0400 Subject: [PATCH 3/9] Change tuple to struct --- src/context.rs | 119 ++++++++++++++++++++++++++++++------------------- 1 file changed, 72 insertions(+), 47 deletions(-) diff --git a/src/context.rs b/src/context.rs index 07f179a6..4956a0a7 100644 --- a/src/context.rs +++ b/src/context.rs @@ -50,11 +50,11 @@ enum KclExecutionError { } #[cfg(test)] -type KclProgramRun = ( - kcl_lib::ExecutorContext, - kcl_lib::ExecState, - Option, -); +struct KclProgramRun { + exec_ctx: kcl_lib::ExecutorContext, + exec_state: kcl_lib::ExecState, + session_data: Option, +} #[cfg(test)] impl KclExecutionError { @@ -153,7 +153,11 @@ async fn run_kcl_program_once_with_client( .await .map_err(|err| KclExecutionError::Execution(Box::new(err)))? .1; - Ok((ctx, state, session_data)) + Ok(KclProgramRun { + exec_ctx: ctx, + exec_state: state, + session_data, + }) } impl<'a> Context<'a> { @@ -383,15 +387,21 @@ impl<'a> Context<'a> { let mut retries_remaining = retry_config.retries; loop { let result = async { - let (ctx, state, session_data) = - run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check) - .map_err(KclExecutionError::Issue)?; + let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Issue)?; let batch_context = kcl_lib::EngineBatchContext::new(); - ctx.engine + run.exec_ctx + .engine .send_modeling_cmd( &batch_context, uuid::Uuid::new_v4(), @@ -407,7 +417,8 @@ impl<'a> Context<'a> { .await .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - let resp = ctx + let resp = run + .exec_ctx .engine .send_modeling_cmd( &batch_context, @@ -417,7 +428,7 @@ impl<'a> Context<'a> { ) .await .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - Ok((resp, session_data)) + Ok((resp, run.session_data)) } .await; @@ -485,16 +496,22 @@ impl<'a> Context<'a> { let mut retries_remaining = retry_config.retries; loop { let result = async { - let (ctx, state, session_data) = - run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check) - .map_err(KclExecutionError::Issue)?; + let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Issue)?; let batch_context = kcl_lib::EngineBatchContext::new(); let mut responses = Vec::with_capacity(cmds.len()); for cmd in cmds.clone() { - let resp = ctx + let resp = run + .exec_ctx .engine .send_modeling_cmd( &batch_context, @@ -507,7 +524,7 @@ impl<'a> Context<'a> { responses.push(resp); } - Ok((responses, session_data)) + Ok((responses, run.session_data)) } .await; @@ -565,16 +582,22 @@ impl<'a> Context<'a> { let mut retries_remaining = retry_config.retries; loop { let result = async { - let (ctx, state, session_data) = - run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check) - .map_err(KclExecutionError::Issue)?; + let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Issue)?; let batch_context = kcl_lib::EngineBatchContext::new(); let mut snapshot_resps = Vec::new(); for snapshot_cmd in snapshot_cmds.clone() { - let resp = ctx + let resp = run + .exec_ctx .engine .send_modeling_cmd( &batch_context, @@ -593,7 +616,7 @@ impl<'a> Context<'a> { } } - Ok((snapshot_resps, session_data)) + Ok((snapshot_resps, run.session_data)) } .await; @@ -650,17 +673,23 @@ impl<'a> Context<'a> { let mut retries_remaining = retry_config.retries; loop { let result = async { - let (ctx, state, session_data) = - run_kcl_program_once_with_client(&client, program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check) - .map_err(KclExecutionError::Issue)?; + let run = run_kcl_program_once_with_client(&client, program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Issue)?; - let files = ctx + let files = run + .exec_ctx .export(output_format.clone()) .await .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - Ok((files, session_data)) + Ok((files, run.session_data)) } .await; @@ -698,19 +727,15 @@ impl<'a> Context<'a> { )> { #[cfg(test)] if let Some(retry_config) = &self.kcl_retry_config { - let result: std::result::Result< - ( - kcl_lib::ExecutorContext, - kcl_lib::ExecState, - Option, - ), - KclExecutionError, - > = execute_with_retries(retry_config, || { - run_kcl_program_once_with_client(client, program, settings.clone()) - }) - .await; + let result: std::result::Result = + execute_with_retries(retry_config, || { + run_kcl_program_once_with_client(client, program, settings.clone()) + }) + .await; - return result.map_err(|err| err.into_anyhow("", code)); + return result + .map(|run| (run.exec_ctx, run.exec_state, run.session_data)) + .map_err(|err| err.into_anyhow("", code)); } let ctx = kcl_lib::ExecutorContext::new(client, settings).await?; From 2cc662b6568a00c1ad6548e10ffdb70869416013 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 17:42:54 -0400 Subject: [PATCH 4/9] Collapse enum variants --- src/context.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/context.rs b/src/context.rs index 4956a0a7..d2d8f71e 100644 --- a/src/context.rs +++ b/src/context.rs @@ -43,10 +43,9 @@ impl Default for RetryConfig { #[cfg(test)] enum KclExecutionError { - Setup(anyhow::Error), Execution(Box), Kcl(Box), - Issue(anyhow::Error), + Other(anyhow::Error), } #[cfg(test)] @@ -60,10 +59,9 @@ struct KclProgramRun { impl KclExecutionError { fn into_anyhow(self, filename: &str, code: &str) -> anyhow::Error { match self { - Self::Setup(err) => err, Self::Execution(err) => kcl_error_fmt::into_miette(*err, code), Self::Kcl(err) => kcl_error_fmt::into_miette_for_parse(filename, code, *err), - Self::Issue(err) => err, + Self::Other(err) => err, } } } @@ -72,10 +70,9 @@ impl KclExecutionError { impl std::fmt::Display for KclExecutionError { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Setup(err) => write!(formatter, "{err}"), Self::Execution(err) => write!(formatter, "{err}"), Self::Kcl(err) => write!(formatter, "{err}"), - Self::Issue(err) => write!(formatter, "{err}"), + Self::Other(err) => write!(formatter, "{err}"), } } } @@ -84,10 +81,9 @@ impl std::fmt::Display for KclExecutionError { impl kcl_lib::IsRetryable for KclExecutionError { fn is_retryable(&self) -> bool { match self { - Self::Setup(_) => false, Self::Execution(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), Self::Kcl(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), - Self::Issue(_) => false, + Self::Other(_) => false, } } } @@ -146,7 +142,7 @@ async fn run_kcl_program_once_with_client( ) -> std::result::Result { let ctx = kcl_lib::ExecutorContext::new(client, settings) .await - .map_err(KclExecutionError::Setup)?; + .map_err(KclExecutionError::Other)?; let mut state = kcl_lib::ExecState::new(&ctx); let session_data = ctx .run(program, &mut state) @@ -396,7 +392,7 @@ impl<'a> Context<'a> { &run.exec_state, issue_check, ) - .map_err(KclExecutionError::Issue)?; + .map_err(KclExecutionError::Other)?; let batch_context = kcl_lib::EngineBatchContext::new(); @@ -505,7 +501,7 @@ impl<'a> Context<'a> { &run.exec_state, issue_check, ) - .map_err(KclExecutionError::Issue)?; + .map_err(KclExecutionError::Other)?; let batch_context = kcl_lib::EngineBatchContext::new(); let mut responses = Vec::with_capacity(cmds.len()); @@ -591,7 +587,7 @@ impl<'a> Context<'a> { &run.exec_state, issue_check, ) - .map_err(KclExecutionError::Issue)?; + .map_err(KclExecutionError::Other)?; let batch_context = kcl_lib::EngineBatchContext::new(); let mut snapshot_resps = Vec::new(); @@ -682,7 +678,7 @@ impl<'a> Context<'a> { &run.exec_state, issue_check, ) - .map_err(KclExecutionError::Issue)?; + .map_err(KclExecutionError::Other)?; let files = run .exec_ctx From 1da05ef471d8b704c1f000e614d5242ed29369ce Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 17:54:30 -0400 Subject: [PATCH 5/9] Remove logic duplication --- src/context.rs | 140 +++++++++++++++++++++++++------------------------ 1 file changed, 72 insertions(+), 68 deletions(-) diff --git a/src/context.rs b/src/context.rs index d2d8f71e..db0a6a9a 100644 --- a/src/context.rs +++ b/src/context.rs @@ -99,14 +99,7 @@ where loop { let exec_result = execute().await; - if retries_remaining > 0 - && let Err(error) = &exec_result - && error.is_retryable() - { - if config.print_retries { - eprintln!("Execute got {error}; retrying..."); - } - retries_remaining -= 1; + if should_retry_kcl_attempt(config, &mut retries_remaining, &exec_result) { continue; } @@ -115,11 +108,14 @@ where } #[cfg(test)] -fn should_retry_kcl_attempt( +fn should_retry_kcl_attempt( config: &RetryConfig, retries_remaining: &mut usize, - result: &std::result::Result, -) -> bool { + result: &std::result::Result, +) -> bool +where + E: kcl_lib::IsRetryable + std::fmt::Display, +{ if *retries_remaining > 0 && let Err(error) = result && kcl_lib::IsRetryable::is_retryable(error) @@ -382,7 +378,10 @@ impl<'a> Context<'a> { if let Some(retry_config) = self.kcl_retry_config.clone() { let mut retries_remaining = retry_config.retries; loop { - let result = async { + let result: std::result::Result< + (OkWebSocketResponseData, Option), + KclExecutionError, + > = async { let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; kcl_error_fmt::check_exec_state_issues( @@ -491,7 +490,10 @@ impl<'a> Context<'a> { if let Some(retry_config) = self.kcl_retry_config.clone() { let mut retries_remaining = retry_config.retries; loop { - let result = async { + let result: std::result::Result< + (Vec, Option), + KclExecutionError, + > = async { let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; kcl_error_fmt::check_exec_state_issues( @@ -577,44 +579,45 @@ impl<'a> Context<'a> { if let Some(retry_config) = self.kcl_retry_config.clone() { let mut retries_remaining = retry_config.retries; loop { - let result = async { - let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues( - &mut self.io.err_out, - filename, - code, - &run.exec_state, - issue_check, - ) - .map_err(KclExecutionError::Other)?; - - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut snapshot_resps = Vec::new(); - for snapshot_cmd in snapshot_cmds.clone() { - let resp = run - .exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &snapshot_cmd, - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - if let OkWebSocketResponseData::Modeling { - modeling_response: - kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), - } = resp - { - snapshot_resps.push(snap); + let result: std::result::Result<(Vec, Option), KclExecutionError> = + async { + let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Other)?; + + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut snapshot_resps = Vec::new(); + for snapshot_cmd in snapshot_cmds.clone() { + let resp = run + .exec_ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &snapshot_cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + if let OkWebSocketResponseData::Modeling { + modeling_response: + kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), + } = resp + { + snapshot_resps.push(snap); + } } - } - Ok((snapshot_resps, run.session_data)) - } - .await; + Ok((snapshot_resps, run.session_data)) + } + .await; if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { continue; @@ -668,26 +671,27 @@ impl<'a> Context<'a> { if let Some(retry_config) = self.kcl_retry_config.clone() { let mut retries_remaining = retry_config.retries; loop { - let result = async { - let run = run_kcl_program_once_with_client(&client, program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues( - &mut self.io.err_out, - filename, - code, - &run.exec_state, - issue_check, - ) - .map_err(KclExecutionError::Other)?; + let result: std::result::Result<(Vec, Option), KclExecutionError> = + async { + let run = run_kcl_program_once_with_client(&client, program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Other)?; - let files = run - .exec_ctx - .export(output_format.clone()) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - Ok((files, run.session_data)) - } - .await; + let files = run + .exec_ctx + .export(output_format.clone()) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + Ok((files, run.session_data)) + } + .await; if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { continue; From 764a02652819194aad0174f2eb6105aa078c4195 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 17:59:28 -0400 Subject: [PATCH 6/9] Change to use the named struct instead of the tuple --- src/context.rs | 56 ++++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/src/context.rs b/src/context.rs index db0a6a9a..a964a661 100644 --- a/src/context.rs +++ b/src/context.rs @@ -48,7 +48,6 @@ enum KclExecutionError { Other(anyhow::Error), } -#[cfg(test)] struct KclProgramRun { exec_ctx: kcl_lib::ExecutorContext, exec_state: kcl_lib::ExecState, @@ -435,15 +434,16 @@ impl<'a> Context<'a> { } } - let (ctx, state, session_data) = self + let run = self .run_kcl_program_with_client(&client, code, &program, settings) .await?; - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check)?; + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &run.exec_state, issue_check)?; let batch_context = kcl_lib::EngineBatchContext::new(); // Zoom on the object. - ctx.engine + run.exec_ctx + .engine .send_modeling_cmd( &batch_context, uuid::Uuid::new_v4(), @@ -458,7 +458,8 @@ impl<'a> Context<'a> { ) .await?; - let resp = ctx + let resp = run + .exec_ctx .engine .send_modeling_cmd( &batch_context, @@ -468,7 +469,7 @@ impl<'a> Context<'a> { ) .await .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - Ok((resp, session_data)) + Ok((resp, run.session_data)) } pub(crate) async fn run_kcl_then_modeling_cmds( @@ -534,15 +535,16 @@ impl<'a> Context<'a> { } } - let (ctx, state, session_data) = self + let run = self .run_kcl_program_with_client(&client, code, &program, settings) .await?; - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check)?; + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &run.exec_state, issue_check)?; let batch_context = kcl_lib::EngineBatchContext::new(); let mut responses = Vec::with_capacity(cmds.len()); for cmd in cmds { - let resp = ctx + let resp = run + .exec_ctx .engine .send_modeling_cmd( &batch_context, @@ -555,7 +557,7 @@ impl<'a> Context<'a> { responses.push(resp); } - Ok((responses, session_data)) + Ok((responses, run.session_data)) } /// Run the given KCL program, then after, run the given extra modeling commands. @@ -627,15 +629,16 @@ impl<'a> Context<'a> { } } - let (ctx, state, session_data) = self + let run = self .run_kcl_program_with_client(&client, code, &program, settings) .await?; - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check)?; + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &run.exec_state, issue_check)?; let batch_context = kcl_lib::EngineBatchContext::new(); let mut snapshot_resps = Vec::new(); for snapshot_cmd in snapshot_cmds { - let resp = ctx + let resp = run + .exec_ctx .engine .send_modeling_cmd( &batch_context, @@ -653,7 +656,7 @@ impl<'a> Context<'a> { } } - Ok((snapshot_resps, session_data)) + Ok((snapshot_resps, run.session_data)) } pub(crate) async fn run_kcl_then_export( @@ -701,17 +704,18 @@ impl<'a> Context<'a> { } } - let (ctx, state, session_data) = self + let run = self .run_kcl_program_with_client(&client, code, program, settings) .await?; - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &state, issue_check)?; + kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &run.exec_state, issue_check)?; - let files = ctx + let files = run + .exec_ctx .export(output_format) .await .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - Ok((files, session_data)) + Ok((files, run.session_data)) } async fn run_kcl_program_with_client( @@ -720,11 +724,7 @@ impl<'a> Context<'a> { code: &str, program: &kcl_lib::Program, settings: kcl_lib::ExecutorSettings, - ) -> Result<( - kcl_lib::ExecutorContext, - kcl_lib::ExecState, - Option, - )> { + ) -> Result { #[cfg(test)] if let Some(retry_config) = &self.kcl_retry_config { let result: std::result::Result = @@ -733,9 +733,7 @@ impl<'a> Context<'a> { }) .await; - return result - .map(|run| (run.exec_ctx, run.exec_state, run.session_data)) - .map_err(|err| err.into_anyhow("", code)); + return result.map_err(|err| err.into_anyhow("", code)); } let ctx = kcl_lib::ExecutorContext::new(client, settings).await?; @@ -745,7 +743,11 @@ impl<'a> Context<'a> { .await .map_err(|err| kcl_error_fmt::into_miette(err, code))? .1; - Ok((ctx, state, session_data)) + Ok(KclProgramRun { + exec_ctx: ctx, + exec_state: state, + session_data, + }) } /// Create and poll a plain text-to-CAD job. From 157f4bdf48ddd135604d177db6ad37d1b092d821 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 18:25:48 -0400 Subject: [PATCH 7/9] Reduce duplicated code --- src/context.rs | 421 ++++++++++++++++++------------------------------- 1 file changed, 151 insertions(+), 270 deletions(-) diff --git a/src/context.rs b/src/context.rs index a964a661..b45f2e56 100644 --- a/src/context.rs +++ b/src/context.rs @@ -24,11 +24,19 @@ pub struct Context<'a> { pub(crate) kcl_retry_config: Option, } -#[cfg(test)] #[derive(Debug, Clone)] pub(crate) struct RetryConfig { - pub(crate) retries: usize, - pub(crate) print_retries: bool, + retries: usize, + print_retries: bool, +} + +impl RetryConfig { + fn no_retries() -> Self { + Self { + retries: 0, + print_retries: false, + } + } } #[cfg(test)] @@ -41,7 +49,6 @@ impl Default for RetryConfig { } } -#[cfg(test)] enum KclExecutionError { Execution(Box), Kcl(Box), @@ -54,7 +61,6 @@ struct KclProgramRun { session_data: Option, } -#[cfg(test)] impl KclExecutionError { fn into_anyhow(self, filename: &str, code: &str) -> anyhow::Error { match self { @@ -65,7 +71,6 @@ impl KclExecutionError { } } -#[cfg(test)] impl std::fmt::Display for KclExecutionError { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -76,7 +81,6 @@ impl std::fmt::Display for KclExecutionError { } } -#[cfg(test)] impl kcl_lib::IsRetryable for KclExecutionError { fn is_retryable(&self) -> bool { match self { @@ -106,7 +110,6 @@ where } } -#[cfg(test)] fn should_retry_kcl_attempt( config: &RetryConfig, retries_remaining: &mut usize, @@ -129,7 +132,6 @@ where } } -#[cfg(test)] async fn run_kcl_program_once_with_client( client: &kittycad::Client, program: &kcl_lib::Program, @@ -152,6 +154,17 @@ async fn run_kcl_program_once_with_client( } impl<'a> Context<'a> { + fn kcl_retry_config(&self) -> RetryConfig { + #[cfg(test)] + { + self.kcl_retry_config.clone().unwrap_or_else(RetryConfig::no_retries) + } + #[cfg(not(test))] + { + RetryConfig::no_retries() + } + } + fn resolve_api_host_and_baseurl(&self, hostname: &str) -> Result<(String, String)> { let host = if !hostname.is_empty() { hostname.to_string() @@ -373,14 +386,11 @@ impl<'a> Context<'a> { .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; let settings = cmd_kcl::with_heartbeats(settings); - #[cfg(test)] - if let Some(retry_config) = self.kcl_retry_config.clone() { - let mut retries_remaining = retry_config.retries; - loop { - let result: std::result::Result< - (OkWebSocketResponseData, Option), - KclExecutionError, - > = async { + let retry_config = self.kcl_retry_config(); + let mut retries_remaining = retry_config.retries; + loop { + let result: std::result::Result<(OkWebSocketResponseData, Option), KclExecutionError> = + async { let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; kcl_error_fmt::check_exec_state_issues( @@ -426,50 +436,12 @@ impl<'a> Context<'a> { } .await; - if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { - continue; - } - - return result.map_err(|err| err.into_anyhow(filename, code)); + if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { + continue; } - } - - let run = self - .run_kcl_program_with_client(&client, code, &program, settings) - .await?; - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &run.exec_state, issue_check)?; - - let batch_context = kcl_lib::EngineBatchContext::new(); - // Zoom on the object. - run.exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &ModelingCmd::from( - mcmd::ZoomToFit::builder() - .animated(false) - .object_ids(Default::default()) - .padding(0.1) - .build(), - ), - ) - .await?; - - let resp = run - .exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &cmd, - ) - .await - .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - Ok((resp, run.session_data)) + return result.map_err(|err| err.into_anyhow(filename, code)); + } } pub(crate) async fn run_kcl_then_modeling_cmds( @@ -487,77 +459,51 @@ impl<'a> Context<'a> { .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; let settings = cmd_kcl::with_heartbeats(settings); - #[cfg(test)] - if let Some(retry_config) = self.kcl_retry_config.clone() { - let mut retries_remaining = retry_config.retries; - loop { - let result: std::result::Result< - (Vec, Option), - KclExecutionError, - > = async { - let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues( - &mut self.io.err_out, - filename, - code, - &run.exec_state, - issue_check, - ) - .map_err(KclExecutionError::Other)?; - - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut responses = Vec::with_capacity(cmds.len()); - for cmd in cmds.clone() { - let resp = run - .exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &cmd, - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - responses.push(resp); - } - - Ok((responses, run.session_data)) - } - .await; + let retry_config = self.kcl_retry_config(); + let mut retries_remaining = retry_config.retries; + loop { + let result: std::result::Result< + (Vec, Option), + KclExecutionError, + > = async { + let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Other)?; - if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { - continue; + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut responses = Vec::with_capacity(cmds.len()); + for cmd in cmds.clone() { + let resp = run + .exec_ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + responses.push(resp); } - return result.map_err(|err| err.into_anyhow(filename, code)); + Ok((responses, run.session_data)) } - } + .await; - let run = self - .run_kcl_program_with_client(&client, code, &program, settings) - .await?; - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &run.exec_state, issue_check)?; + if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { + continue; + } - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut responses = Vec::with_capacity(cmds.len()); - for cmd in cmds { - let resp = run - .exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &cmd, - ) - .await - .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - responses.push(resp); + return result.map_err(|err| err.into_anyhow(filename, code)); } - - Ok((responses, run.session_data)) } /// Run the given KCL program, then after, run the given extra modeling commands. @@ -577,86 +523,55 @@ impl<'a> Context<'a> { .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; let settings = cmd_kcl::with_heartbeats(settings); - #[cfg(test)] - if let Some(retry_config) = self.kcl_retry_config.clone() { - let mut retries_remaining = retry_config.retries; - loop { - let result: std::result::Result<(Vec, Option), KclExecutionError> = - async { - let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues( - &mut self.io.err_out, - filename, - code, - &run.exec_state, - issue_check, - ) - .map_err(KclExecutionError::Other)?; - - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut snapshot_resps = Vec::new(); - for snapshot_cmd in snapshot_cmds.clone() { - let resp = run - .exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &snapshot_cmd, - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - if let OkWebSocketResponseData::Modeling { - modeling_response: - kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), - } = resp - { - snapshot_resps.push(snap); - } - } + let retry_config = self.kcl_retry_config(); + let mut retries_remaining = retry_config.retries; + loop { + let result: std::result::Result<(Vec, Option), KclExecutionError> = + async { + let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - Ok((snapshot_resps, run.session_data)) + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Other)?; + + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut snapshot_resps = Vec::new(); + for snapshot_cmd in snapshot_cmds.clone() { + let resp = run + .exec_ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &snapshot_cmd, + ) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + if let OkWebSocketResponseData::Modeling { + modeling_response: + kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), + } = resp + { + snapshot_resps.push(snap); + } } - .await; - if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { - continue; + Ok((snapshot_resps, run.session_data)) } + .await; - return result.map_err(|err| err.into_anyhow(filename, code)); + if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { + continue; } - } - - let run = self - .run_kcl_program_with_client(&client, code, &program, settings) - .await?; - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &run.exec_state, issue_check)?; - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut snapshot_resps = Vec::new(); - for snapshot_cmd in snapshot_cmds { - let resp = run - .exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &snapshot_cmd, - ) - .await - .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - if let OkWebSocketResponseData::Modeling { - modeling_response: kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), - } = resp - { - snapshot_resps.push(snap); - } + return result.map_err(|err| err.into_anyhow(filename, code)); } - - Ok((snapshot_resps, run.session_data)) } pub(crate) async fn run_kcl_then_export( @@ -669,85 +584,36 @@ impl<'a> Context<'a> { output_format: kittycad_modeling_cmds::format::OutputFormat3d, ) -> Result<(Vec, Option)> { let client = self.api_client("")?; - - #[cfg(test)] - if let Some(retry_config) = self.kcl_retry_config.clone() { - let mut retries_remaining = retry_config.retries; - loop { - let result: std::result::Result<(Vec, Option), KclExecutionError> = - async { - let run = run_kcl_program_once_with_client(&client, program, settings.clone()).await?; - - kcl_error_fmt::check_exec_state_issues( - &mut self.io.err_out, - filename, - code, - &run.exec_state, - issue_check, - ) - .map_err(KclExecutionError::Other)?; - - let files = run - .exec_ctx - .export(output_format.clone()) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - Ok((files, run.session_data)) - } - .await; - - if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { - continue; - } - - return result.map_err(|err| err.into_anyhow(filename, code)); + let retry_config = self.kcl_retry_config(); + let mut retries_remaining = retry_config.retries; + loop { + let result: std::result::Result<(Vec, Option), KclExecutionError> = async { + let run = run_kcl_program_once_with_client(&client, program, settings.clone()).await?; + + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecutionError::Other)?; + + let files = run + .exec_ctx + .export(output_format.clone()) + .await + .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + Ok((files, run.session_data)) } - } - - let run = self - .run_kcl_program_with_client(&client, code, program, settings) - .await?; - kcl_error_fmt::check_exec_state_issues(&mut self.io.err_out, filename, code, &run.exec_state, issue_check)?; - - let files = run - .exec_ctx - .export(output_format) - .await - .map_err(|err| kcl_error_fmt::into_miette_for_parse(filename, code, err))?; - - Ok((files, run.session_data)) - } + .await; - async fn run_kcl_program_with_client( - &self, - client: &kittycad::Client, - code: &str, - program: &kcl_lib::Program, - settings: kcl_lib::ExecutorSettings, - ) -> Result { - #[cfg(test)] - if let Some(retry_config) = &self.kcl_retry_config { - let result: std::result::Result = - execute_with_retries(retry_config, || { - run_kcl_program_once_with_client(client, program, settings.clone()) - }) - .await; + if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { + continue; + } - return result.map_err(|err| err.into_anyhow("", code)); + return result.map_err(|err| err.into_anyhow(filename, code)); } - - let ctx = kcl_lib::ExecutorContext::new(client, settings).await?; - let mut state = kcl_lib::ExecState::new(&ctx); - let session_data = ctx - .run(program, &mut state) - .await - .map_err(|err| kcl_error_fmt::into_miette(err, code))? - .1; - Ok(KclProgramRun { - exec_ctx: ctx, - exec_state: state, - session_data, - }) } /// Create and poll a plain text-to-CAD job. @@ -1730,6 +1596,21 @@ mod test { assert_eq!(attempts, 1); } + #[tokio::test] + async fn execute_with_retries_no_retries_does_not_retry_retryable_errors() { + let retry_config = RetryConfig::no_retries(); + let mut attempts = 0; + + let result: std::result::Result<(), RetryTestError> = execute_with_retries(&retry_config, || { + attempts += 1; + async { Err(RetryTestError::Retryable) } + }) + .await; + + assert_eq!(result, Err(RetryTestError::Retryable)); + assert_eq!(attempts, 1); + } + #[test] fn resolve_host_prefers_explicit_then_global() { let mut config = crate::config::new_blank_config().unwrap(); From 86ad9ddbe73c55e9ee9cf3d6b6c75276597dc2da Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 21:14:13 -0400 Subject: [PATCH 8/9] Change order and rename to be clearer --- src/context.rs | 181 ++++++++++++++++++++++++------------------------- 1 file changed, 89 insertions(+), 92 deletions(-) diff --git a/src/context.rs b/src/context.rs index 4d4a98ab..048f2f24 100644 --- a/src/context.rs +++ b/src/context.rs @@ -65,43 +65,43 @@ impl Default for RetryConfig { } } -enum KclExecutionError { - Execution(Box), - Kcl(Box), - Other(anyhow::Error), -} - struct KclProgramRun { exec_ctx: kcl_lib::ExecutorContext, exec_state: kcl_lib::ExecState, session_data: Option, } -impl KclExecutionError { +enum KclExecError { + Err(Box), + WithOutputs(Box), + Other(anyhow::Error), +} + +impl KclExecError { fn into_anyhow(self, filename: &str, code: &str) -> anyhow::Error { match self { - Self::Execution(err) => kcl_error_fmt::into_miette(*err, code), - Self::Kcl(err) => kcl_error_fmt::into_miette_for_parse(filename, code, *err), + Self::Err(err) => kcl_error_fmt::into_miette_for_parse(filename, code, *err), + Self::WithOutputs(err) => kcl_error_fmt::into_miette(*err, code), Self::Other(err) => err, } } } -impl std::fmt::Display for KclExecutionError { +impl std::fmt::Display for KclExecError { fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::Execution(err) => write!(formatter, "{err}"), - Self::Kcl(err) => write!(formatter, "{err}"), + Self::Err(err) => write!(formatter, "{err}"), + Self::WithOutputs(err) => write!(formatter, "{err}"), Self::Other(err) => write!(formatter, "{err}"), } } } -impl kcl_lib::IsRetryable for KclExecutionError { +impl kcl_lib::IsRetryable for KclExecError { fn is_retryable(&self) -> bool { match self { - Self::Execution(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), - Self::Kcl(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), + Self::Err(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), + Self::WithOutputs(err) => kcl_lib::IsRetryable::is_retryable(err.as_ref()), Self::Other(_) => false, } } @@ -152,15 +152,15 @@ async fn run_kcl_program_once_with_client( client: &kittycad::Client, program: &kcl_lib::Program, settings: kcl_lib::ExecutorSettings, -) -> std::result::Result { +) -> std::result::Result { let ctx = kcl_lib::ExecutorContext::new(client, settings) .await - .map_err(KclExecutionError::Other)?; + .map_err(KclExecError::Other)?; let mut state = kcl_lib::ExecState::new(&ctx); let session_data = ctx .run(program, &mut state) .await - .map_err(|err| KclExecutionError::Execution(Box::new(err)))? + .map_err(|err| KclExecError::WithOutputs(Box::new(err)))? .1; Ok(KclProgramRun { exec_ctx: ctx, @@ -593,7 +593,7 @@ impl<'a> Context<'a> { let retry_config = self.kcl_retry_config(); let mut retries_remaining = retry_config.retries; loop { - let result: std::result::Result<(OkWebSocketResponseData, Option), KclExecutionError> = + let result: std::result::Result<(OkWebSocketResponseData, Option), KclExecError> = async { let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; @@ -604,7 +604,7 @@ impl<'a> Context<'a> { &run.exec_state, issue_check, ) - .map_err(KclExecutionError::Other)?; + .map_err(KclExecError::Other)?; let batch_context = kcl_lib::EngineBatchContext::new(); @@ -623,7 +623,7 @@ impl<'a> Context<'a> { ), ) .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + .map_err(|err| KclExecError::Err(Box::new(err)))?; let resp = run .exec_ctx @@ -635,7 +635,7 @@ impl<'a> Context<'a> { &cmd, ) .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + .map_err(|err| KclExecError::Err(Box::new(err)))?; Ok((resp, run.session_data)) } .await; @@ -672,41 +672,39 @@ impl<'a> Context<'a> { let retry_config = self.kcl_retry_config(); let mut retries_remaining = retry_config.retries; loop { - let result: std::result::Result< - (Vec, Option), - KclExecutionError, - > = async { - let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + let result: std::result::Result<(Vec, Option), KclExecError> = + async { + let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - kcl_error_fmt::check_exec_state_issues( - &mut self.io.err_out, - filename, - code, - &run.exec_state, - issue_check, - ) - .map_err(KclExecutionError::Other)?; + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecError::Other)?; - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut responses = Vec::with_capacity(cmds.len()); - for cmd in cmds.clone() { - let resp = run - .exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &cmd, - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - responses.push(resp); - } + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut responses = Vec::with_capacity(cmds.len()); + for cmd in cmds.clone() { + let resp = run + .exec_ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &cmd, + ) + .await + .map_err(|err| KclExecError::Err(Box::new(err)))?; + responses.push(resp); + } - Ok((responses, run.session_data)) - } - .await; + Ok((responses, run.session_data)) + } + .await; if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { continue; @@ -760,45 +758,44 @@ impl<'a> Context<'a> { let retry_config = self.kcl_retry_config(); let mut retries_remaining = retry_config.retries; loop { - let result: std::result::Result<(Vec, Option), KclExecutionError> = - async { - let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; + let result: std::result::Result<(Vec, Option), KclExecError> = async { + let run = run_kcl_program_once_with_client(&client, &program, settings.clone()).await?; - kcl_error_fmt::check_exec_state_issues( - &mut self.io.err_out, - filename, - code, - &run.exec_state, - issue_check, - ) - .map_err(KclExecutionError::Other)?; + kcl_error_fmt::check_exec_state_issues( + &mut self.io.err_out, + filename, + code, + &run.exec_state, + issue_check, + ) + .map_err(KclExecError::Other)?; - let batch_context = kcl_lib::EngineBatchContext::new(); - let mut snapshot_resps = Vec::new(); - for snapshot_cmd in snapshot_cmds.clone() { - let resp = run - .exec_ctx - .engine - .send_modeling_cmd( - &batch_context, - uuid::Uuid::new_v4(), - kcl_lib::SourceRange::default(), - &snapshot_cmd, - ) - .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; - if let OkWebSocketResponseData::Modeling { - modeling_response: - kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), - } = resp - { - snapshot_resps.push(snap); - } + let batch_context = kcl_lib::EngineBatchContext::new(); + let mut snapshot_resps = Vec::new(); + for snapshot_cmd in snapshot_cmds.clone() { + let resp = run + .exec_ctx + .engine + .send_modeling_cmd( + &batch_context, + uuid::Uuid::new_v4(), + kcl_lib::SourceRange::default(), + &snapshot_cmd, + ) + .await + .map_err(|err| KclExecError::Err(Box::new(err)))?; + if let OkWebSocketResponseData::Modeling { + modeling_response: + kittycad_modeling_cmds::ok_response::OkModelingCmdResponse::TakeSnapshot(snap), + } = resp + { + snapshot_resps.push(snap); } - - Ok((snapshot_resps, run.session_data)) } - .await; + + Ok((snapshot_resps, run.session_data)) + } + .await; if should_retry_kcl_attempt(&retry_config, &mut retries_remaining, &result) { continue; @@ -821,7 +818,7 @@ impl<'a> Context<'a> { let retry_config = self.kcl_retry_config(); let mut retries_remaining = retry_config.retries; loop { - let result: std::result::Result<(Vec, Option), KclExecutionError> = async { + let result: std::result::Result<(Vec, Option), KclExecError> = async { let run = run_kcl_program_once_with_client(&client, program, settings.clone()).await?; kcl_error_fmt::check_exec_state_issues( @@ -831,13 +828,13 @@ impl<'a> Context<'a> { &run.exec_state, issue_check, ) - .map_err(KclExecutionError::Other)?; + .map_err(KclExecError::Other)?; let files = run .exec_ctx .export(output_format.clone()) .await - .map_err(|err| KclExecutionError::Kcl(Box::new(err)))?; + .map_err(|err| KclExecError::Err(Box::new(err)))?; Ok((files, run.session_data)) } .await; From e58a31c1c16a15534fd0850c404f6d81bc23d8e9 Mon Sep 17 00:00:00 2001 From: Jonathan Tran Date: Tue, 7 Jul 2026 21:36:33 -0400 Subject: [PATCH 9/9] Add comments --- src/context.rs | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/context.rs b/src/context.rs index 048f2f24..4bc370c3 100644 --- a/src/context.rs +++ b/src/context.rs @@ -36,10 +36,12 @@ pub struct Context<'a> { pub debug: bool, // If set, override the host used when commands don't specify one. pub(crate) override_host: Option, + /// Overrides retry behavior for KCL execution tests. #[cfg(test)] pub(crate) kcl_retry_config: Option, } +/// Controls how retryable KCL execution failures are retried. #[derive(Debug, Clone)] pub(crate) struct RetryConfig { retries: usize, @@ -47,6 +49,8 @@ pub(crate) struct RetryConfig { } impl RetryConfig { + /// Returns retry settings that make each KCL execution attempt run only + /// once. fn no_retries() -> Self { Self { retries: 0, @@ -57,6 +61,7 @@ impl RetryConfig { #[cfg(test)] impl Default for RetryConfig { + /// Returns the retry settings used by tests that exercise retry handling. fn default() -> Self { Self { retries: 2, @@ -65,19 +70,25 @@ impl Default for RetryConfig { } } +/// Result data captured from a single KCL program execution attempt. struct KclProgramRun { exec_ctx: kcl_lib::ExecutorContext, exec_state: kcl_lib::ExecState, session_data: Option, } +/// Error type used to preserve KCL diagnostics across retry attempts. enum KclExecError { + /// A KCL error that should be formatted with filename and source context. Err(Box), + /// A KCL execution error that already includes command outputs. WithOutputs(Box), + /// Any non-KCL error from setup or issue checking. Other(anyhow::Error), } impl KclExecError { + /// Converts the stored error into the diagnostic form expected by callers. fn into_anyhow(self, filename: &str, code: &str) -> anyhow::Error { match self { Self::Err(err) => kcl_error_fmt::into_miette_for_parse(filename, code, *err), @@ -107,6 +118,8 @@ impl kcl_lib::IsRetryable for KclExecError { } } +/// Runs an async operation until it succeeds, fails fatally, or exhausts +/// retries. #[cfg(test)] async fn execute_with_retries(config: &RetryConfig, mut execute: F) -> std::result::Result where @@ -126,6 +139,8 @@ where } } +/// Adjusts the retries remaining and returns whether a failed KCL attempt +/// should be retried. This handles all the book-keeping of retrying. fn should_retry_kcl_attempt( config: &RetryConfig, retries_remaining: &mut usize, @@ -148,6 +163,8 @@ where } } +/// Runs one KCL execution attempt and returns state needed for follow-up +/// commands. async fn run_kcl_program_once_with_client( client: &kittycad::Client, program: &kcl_lib::Program, @@ -156,20 +173,21 @@ async fn run_kcl_program_once_with_client( let ctx = kcl_lib::ExecutorContext::new(client, settings) .await .map_err(KclExecError::Other)?; - let mut state = kcl_lib::ExecState::new(&ctx); + let mut exec_state = kcl_lib::ExecState::new(&ctx); let session_data = ctx - .run(program, &mut state) + .run(program, &mut exec_state) .await .map_err(|err| KclExecError::WithOutputs(Box::new(err)))? .1; Ok(KclProgramRun { exec_ctx: ctx, - exec_state: state, + exec_state, session_data, }) } impl<'a> Context<'a> { + /// Returns the retry settings to use for local KCL execution. fn kcl_retry_config(&self) -> RetryConfig { #[cfg(test)] { @@ -608,6 +626,7 @@ impl<'a> Context<'a> { let batch_context = kcl_lib::EngineBatchContext::new(); + // Zoom on the object. run.exec_ctx .engine .send_modeling_cmd( @@ -805,6 +824,7 @@ impl<'a> Context<'a> { } } + /// Runs KCL, checks execution issues, and exports the resulting files. pub(crate) async fn run_kcl_then_export( &mut self, filename: &str, @@ -1629,9 +1649,12 @@ mod test { use super::*; + /// Test error used to verify retryable and fatal retry paths. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum RetryTestError { + /// Error variant that should be retried. Retryable, + /// Error variant that should stop retrying. Fatal, } @@ -1886,6 +1909,8 @@ mod test { assert_eq!(md, "Hello world"); } + /// Verifies that retryable failures are retried until the operation + /// succeeds. #[tokio::test] async fn execute_with_retries_retries_retryable_errors() { let retry_config = RetryConfig { @@ -1911,6 +1936,7 @@ mod test { assert_eq!(attempts, 3); } + /// Verifies that fatal failures are returned without retrying. #[tokio::test] async fn execute_with_retries_does_not_retry_fatal_errors() { let retry_config = RetryConfig { @@ -1929,6 +1955,8 @@ mod test { assert_eq!(attempts, 1); } + /// Verifies that retryable failures are not retried when retries are + /// disabled. #[tokio::test] async fn execute_with_retries_no_retries_does_not_retry_retryable_errors() { let retry_config = RetryConfig::no_retries();