From 72f99e8379f005eaf6a20c865b2ba9e5e755bbc0 Mon Sep 17 00:00:00 2001 From: branchseer Date: Sat, 14 Feb 2026 23:11:25 +0800 Subject: [PATCH] refactor: replace task_call_stack with recursive Error::NestPlan --- crates/vite_task/src/session/mod.rs | 9 +- ...ypo in task script fails without list.snap | 2 +- crates/vite_task_plan/src/context.rs | 58 +-------- crates/vite_task_plan/src/error.rs | 114 +++++------------- crates/vite_task_plan/src/lib.rs | 17 +-- crates/vite_task_plan/src/plan.rs | 105 +++++++--------- 6 files changed, 83 insertions(+), 222 deletions(-) diff --git a/crates/vite_task/src/session/mod.rs b/crates/vite_task/src/session/mod.rs index 7975a100..69f407ec 100644 --- a/crates/vite_task/src/session/mod.rs +++ b/crates/vite_task/src/session/mod.rs @@ -21,7 +21,7 @@ use vite_task_graph::{ loader::UserConfigLoader, }; use vite_task_plan::{ - ExecutionPlan, TaskGraphLoader, TaskPlanErrorKind, + ExecutionPlan, TaskGraphLoader, plan_request::{PlanRequest, ScriptCommand, SyntheticPlanRequest}, prepend_path_env, }; @@ -484,16 +484,15 @@ impl<'a> Session<'a> { let plan_request = match command.into_plan_request(&cwd) { Ok(plan_request) => plan_request, Err(crate::cli::CLITaskQueryError::MissingTaskSpecifier) => { - return Err(TaskPlanErrorKind::MissingTaskSpecifier.with_empty_call_stack()); + return Err(vite_task_plan::Error::MissingTaskSpecifier); } Err(error) => { - return Err(TaskPlanErrorKind::ParsePlanRequestError { + return Err(vite_task_plan::Error::ParsePlanRequest { error: error.into(), program: Str::from("vp"), args: Arc::default(), cwd: Arc::clone(&cwd), - } - .with_empty_call_stack()); + }); } }; let plan = ExecutionPlan::plan( diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/task-select/snapshots/typo in task script fails without list.snap b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/task-select/snapshots/typo in task script fails without list.snap index cb58fd0d..a0820dc1 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/task-select/snapshots/typo in task script fails without list.snap +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/task-select/snapshots/typo in task script fails without list.snap @@ -3,7 +3,7 @@ source: crates/vite_task_bin/tests/e2e_snapshots/main.rs expression: e2e_outputs --- [1]> vp run run-typo-task -Error: Failed to plan execution, task call stack: task-select-test#run-typo-task +Error: Failed to plan tasks from `vp run nonexistent-xyz` in task task-select-test#run-typo-task Caused by: 0: Failed to query tasks from task graph diff --git a/crates/vite_task_plan/src/context.rs b/crates/vite_task_plan/src/context.rs index 3b233bc0..0f67a05f 100644 --- a/crates/vite_task_plan/src/context.rs +++ b/crates/vite_task_plan/src/context.rs @@ -1,9 +1,9 @@ -use std::{env::JoinPathsError, ffi::OsStr, fmt::Display, ops::Range, sync::Arc}; +use std::{env::JoinPathsError, ffi::OsStr, ops::Range, sync::Arc}; use rustc_hash::FxHashMap; use vite_path::AbsolutePath; use vite_str::Str; -use vite_task_graph::{IndexedTaskGraph, TaskNodeIndex, display::TaskDisplay}; +use vite_task_graph::{IndexedTaskGraph, TaskNodeIndex}; use crate::{PlanRequestParser, path_env::prepend_path_env}; @@ -41,46 +41,6 @@ pub struct PlanContext<'a> { indexed_task_graph: &'a IndexedTaskGraph, } -/// A human-readable frame in the task call stack. -#[derive(Debug, Clone)] -pub struct TaskCallStackFrameDisplay { - pub task_display: TaskDisplay, - - #[expect(dead_code, reason = "to be used in terminal error display")] - pub command_span: Range, -} - -impl Display for TaskCallStackFrameDisplay { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - // TODO: display command_span - write!(f, "{}", self.task_display) - } -} - -/// A human-readable display of the task call stack. -#[derive(Default, Debug, Clone)] -pub struct TaskCallStackDisplay { - frames: Arc<[TaskCallStackFrameDisplay]>, -} - -impl TaskCallStackDisplay { - pub fn is_empty(&self) -> bool { - self.frames.is_empty() - } -} - -impl Display for TaskCallStackDisplay { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for (i, frame) in self.frames.iter().enumerate() { - if i > 0 { - write!(f, " -> ")?; - } - write!(f, "{frame}")?; - } - Ok(()) - } -} - impl<'a> PlanContext<'a> { pub fn new( workspace_path: &'a Arc, @@ -104,20 +64,6 @@ impl<'a> PlanContext<'a> { &self.envs } - /// Get a human-readable display of the current task call stack. - pub fn display_call_stack(&self) -> TaskCallStackDisplay { - TaskCallStackDisplay { - frames: self - .task_call_stack - .iter() - .map(|(idx, span)| TaskCallStackFrameDisplay { - task_display: self.indexed_task_graph.display_task(*idx), - command_span: span.clone(), - }) - .collect(), - } - } - /// Check if adding the given task node index would create a recursion in the call stack. pub fn check_recursion( &self, diff --git a/crates/vite_task_plan/src/error.rs b/crates/vite_task_plan/src/error.rs index e29faceb..6e94937d 100644 --- a/crates/vite_task_plan/src/error.rs +++ b/crates/vite_task_plan/src/error.rs @@ -3,15 +3,13 @@ reason = "Arc is used for non-UTF-8 path data in error types" )] use std::path::Path; -use std::{env::JoinPathsError, ffi::OsStr, fmt::Display, sync::Arc}; +use std::{env::JoinPathsError, ffi::OsStr, sync::Arc}; use vite_path::{AbsolutePath, relative::InvalidPathDataError}; use vite_str::Str; +use vite_task_graph::display::TaskDisplay; -use crate::{ - context::{PlanContext, TaskCallStackDisplay, TaskRecursionError}, - envs::ResolveEnvError, -}; +use crate::{context::TaskRecursionError, envs::ResolveEnvError}; #[derive(Debug, thiserror::Error)] pub enum CdCommandError { @@ -30,7 +28,7 @@ pub struct WhichError { #[source] pub error: which::Error, } -impl Display for WhichError { +impl std::fmt::Display for WhichError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, @@ -66,7 +64,7 @@ pub enum PathType { Program, PackagePath, } -impl Display for PathType { +impl std::fmt::Display for PathType { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::Cwd => write!(f, "current working directory"), @@ -84,18 +82,26 @@ pub struct PathFingerprintError { pub kind: PathFingerprintErrorKind, } -/// Errors that can occur when planning a specific execution from a task . +/// Errors that can occur when planning a specific execution from a task. #[derive(Debug, thiserror::Error)] -pub enum TaskPlanErrorKind { +pub enum Error { + #[error("Failed to plan tasks from `{command}` in task {task_display}")] + NestPlan { + task_display: TaskDisplay, + command: Str, + #[source] + error: Box, + }, + #[error("Failed to load task graph")] - TaskGraphLoadError( + TaskGraphLoad( #[source] #[from] vite_task_graph::TaskGraphLoadError, ), #[error("Failed to execute 'cd' command")] - CdCommandError( + CdCommand( #[source] #[from] CdCommandError, @@ -105,10 +111,10 @@ pub enum TaskPlanErrorKind { ProgramNotFound(#[from] WhichError), #[error(transparent)] - PathFingerprintError(#[from] PathFingerprintError), + PathFingerprint(#[from] PathFingerprintError), #[error("Failed to query tasks from task graph")] - TaskQueryError( + TaskQuery( #[source] #[from] vite_task_graph::query::TaskQueryError, @@ -118,7 +124,7 @@ pub enum TaskPlanErrorKind { TaskRecursionDetected(#[from] TaskRecursionError), #[error("Invalid vite task command: {program} with args {args:?} under cwd {cwd:?}")] - ParsePlanRequestError { + ParsePlanRequest { program: Str, args: Arc<[Str]>, cwd: Arc, @@ -127,100 +133,38 @@ pub enum TaskPlanErrorKind { }, #[error("Failed to add node_modules/.bin to PATH environment variable")] - AddNodeModulesBinPathError { + AddNodeModulesBinPath { #[source] join_paths_error: JoinPathsError, }, #[error("Failed to resolve environment variables")] - ResolveEnvError(#[source] ResolveEnvError), + ResolveEnv(#[source] ResolveEnvError), #[error("No task specifier provided for 'run' command")] MissingTaskSpecifier, } -#[derive(Debug, thiserror::Error)] -pub struct Error { - task_call_stack: TaskCallStackDisplay, - - #[source] - kind: TaskPlanErrorKind, -} - -impl Display for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "Failed to plan execution")?; - if !self.task_call_stack.is_empty() { - write!(f, ", task call stack: {}", self.task_call_stack)?; - } - Ok(()) - } -} - -impl TaskPlanErrorKind { - #[must_use] - pub fn with_empty_call_stack(self) -> Error { - Error { task_call_stack: TaskCallStackDisplay::default(), kind: self } - } -} - impl Error { #[must_use] pub const fn is_missing_task_specifier(&self) -> bool { - matches!(self.kind, TaskPlanErrorKind::MissingTaskSpecifier) + matches!(self, Self::MissingTaskSpecifier) } /// If this error represents a top-level task-not-found lookup failure, /// returns the task name that the user typed. /// - /// Returns `None` if the error occurred in a nested task (non-empty call stack), + /// Returns `None` if the error occurred in a nested task (wrapped in `NestPlan`), /// since nested task errors should propagate as-is rather than triggering /// interactive task selection. #[must_use] pub fn task_not_found_name(&self) -> Option<&str> { - if !self.task_call_stack.is_empty() { - return None; - } - match &self.kind { - TaskPlanErrorKind::TaskQueryError( - vite_task_graph::query::TaskQueryError::SpecifierLookupError { specifier, .. }, - ) => Some(specifier.task_name.as_str()), - _ => None, - } - } -} - -#[expect( - clippy::result_large_err, - reason = "Error wraps TaskPlanErrorKind with call stack for diagnostics" -)] -pub trait TaskPlanErrorKindResultExt { - type Ok; - /// Attach the current task call stack from the planning context to the error. - fn with_plan_context(self, context: &PlanContext<'_>) -> Result; - - /// Attach an empty task call stack to the error. - fn with_empty_call_stack(self) -> Result; -} - -impl TaskPlanErrorKindResultExt for Result { - type Ok = T; - - /// Attach the current task call stack from the planning context to the error. - fn with_plan_context(self, context: &PlanContext<'_>) -> Result { - match self { - Ok(value) => Ok(value), - Err(kind) => { - let task_call_stack = context.display_call_stack(); - Err(Error { task_call_stack, kind }) - } - } - } - - fn with_empty_call_stack(self) -> Result { match self { - Ok(value) => Ok(value), - Err(kind) => Err(kind.with_empty_call_stack()), + Self::TaskQuery(vite_task_graph::query::TaskQueryError::SpecifierLookupError { + specifier, + .. + }) => Some(specifier.task_name.as_str()), + _ => None, } } } diff --git a/crates/vite_task_plan/src/lib.rs b/crates/vite_task_plan/src/lib.rs index b1221cc7..756f8069 100644 --- a/crates/vite_task_plan/src/lib.rs +++ b/crates/vite_task_plan/src/lib.rs @@ -11,8 +11,7 @@ pub mod plan_request; use std::{collections::BTreeMap, ffi::OsStr, fmt::Debug, sync::Arc}; use context::PlanContext; -use error::TaskPlanErrorKindResultExt; -pub use error::{Error, TaskPlanErrorKind}; +pub use error::Error; use execution_graph::ExecutionGraph; use in_process::InProcessExecution; pub use path_env::{get_path_env, prepend_path_env}; @@ -216,11 +215,7 @@ impl ExecutionPlan { ) -> Result { let root_node = match plan_request { PlanRequest::Query(query_plan_request) => { - let indexed_task_graph = task_graph_loader - .load_task_graph() - .await - .map_err(TaskPlanErrorKind::TaskGraphLoadError) - .with_empty_call_stack()?; + let indexed_task_graph = task_graph_loader.load_task_graph().await?; let context = PlanContext::new( workspace_path, @@ -240,8 +235,7 @@ impl ExecutionPlan { None, cwd, ParentCacheConfig::None, - ) - .with_empty_call_stack()?; + )?; ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(execution)) } }; @@ -252,7 +246,7 @@ impl ExecutionPlan { /// /// # Errors /// Returns an error if the program is not found or path fingerprinting fails. - #[expect(clippy::result_large_err, reason = "Error contains task call stack for diagnostics")] + #[expect(clippy::result_large_err, reason = "Error is large for diagnostics")] pub fn plan_synthetic( workspace_path: &Arc, cwd: &Arc, @@ -267,8 +261,7 @@ impl ExecutionPlan { Some(execution_cache_key), cwd, ParentCacheConfig::None, - ) - .with_empty_call_stack()?; + )?; Ok(Self { root_node: ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(execution)) }) } } diff --git a/crates/vite_task_plan/src/plan.rs b/crates/vite_task_plan/src/plan.rs index d173759e..e7d63dc2 100644 --- a/crates/vite_task_plan/src/plan.rs +++ b/crates/vite_task_plan/src/plan.rs @@ -29,10 +29,7 @@ use crate::{ SpawnCommand, SpawnExecution, TaskExecution, cache_metadata::{CacheMetadata, ExecutionCacheKey, ProgramFingerprint, SpawnFingerprint}, envs::EnvFingerprints, - error::{ - CdCommandError, Error, PathFingerprintError, PathFingerprintErrorKind, PathType, - TaskPlanErrorKind, TaskPlanErrorKindResultExt, - }, + error::{CdCommandError, Error, PathFingerprintError, PathFingerprintErrorKind, PathType}, execution_graph::{ExecutionGraph, ExecutionNodeIndex}, in_process::InProcessExecution, path_env::get_path_env, @@ -66,10 +63,7 @@ async fn plan_task_as_execution_node( mut context: PlanContext<'_>, ) -> Result { // Check for recursions in the task call stack. - context - .check_recursion(task_node_index) - .map_err(TaskPlanErrorKind::TaskRecursionDetected) - .with_plan_context(&context)?; + context.check_recursion(task_node_index)?; let task_node = &context.indexed_task_graph().task_graph()[task_node_index]; let command_str = task_node.resolved_config.command.as_str(); @@ -78,10 +72,7 @@ async fn plan_task_as_execution_node( // Prepend {package_path}/node_modules/.bin to PATH let package_node_modules_bin_path = package_path.join("node_modules").join(".bin"); if let Err(join_paths_error) = context.prepend_path(&package_node_modules_bin_path) { - // Push the current task frame with full command span (the path was added for every and_item of the command) before returning the error - context.push_stack_frame(task_node_index, 0..command_str.len()); - return Err(TaskPlanErrorKind::AddNodeModulesBinPathError { join_paths_error }) - .with_plan_context(&context); + return Err(Error::AddNodeModulesBinPath { join_paths_error }); } let mut items = Vec::::new(); @@ -115,16 +106,12 @@ async fn plan_task_as_execution_node( )] let cd_target: Cow<'_, Path> = match args.as_slice() { // No args, go to home directory - [] => home_dir() - .ok_or_else(|| { - TaskPlanErrorKind::CdCommandError(CdCommandError::NoHomeDirectory) - }) - .with_plan_context(&context)? - .into(), + [] => { + home_dir().ok_or(Error::CdCommand(CdCommandError::NoHomeDirectory))?.into() + } [dir] => Path::new(dir.as_str()).into(), _ => { - return Err(TaskPlanErrorKind::CdCommandError(CdCommandError::ToManyArgs)) - .with_plan_context(&context); + return Err(Error::CdCommand(CdCommandError::ToManyArgs)); } }; cwd = cwd.join(cd_target.as_ref()).into(); @@ -163,13 +150,10 @@ async fn plan_task_as_execution_node( and_item_index: index, extra_args: Arc::clone(&extra_args), package_path: strip_prefix_for_cache(package_path, context.workspace_path()) - .map_err(|kind| { - TaskPlanErrorKind::PathFingerprintError(PathFingerprintError { - kind, - path_type: PathType::PackagePath, - }) - }) - .with_plan_context(&context)?, + .map_err(|kind| PathFingerprintError { + kind, + path_type: PathType::PackagePath, + })?, }; // Try to parse the args of an and_item to a plan request like `run -r build` @@ -180,24 +164,28 @@ async fn plan_task_as_execution_node( envs, cwd: Arc::clone(&cwd), }; - let plan_request = context - .callbacks() - .get_plan_request(&mut script_command) - .await - .map_err(|error| TaskPlanErrorKind::ParsePlanRequestError { - program: script_command.program.clone(), - args: Arc::clone(&script_command.args), - cwd: Arc::clone(&script_command.cwd), - error, - }) - .with_plan_context(&context)?; + let plan_request = + context.callbacks().get_plan_request(&mut script_command).await.map_err( + |error| Error::ParsePlanRequest { + program: script_command.program.clone(), + args: Arc::clone(&script_command.args), + cwd: Arc::clone(&script_command.cwd), + error, + }, + )?; let execution_item_kind: ExecutionItemKind = match plan_request { // Expand task query like `vp run -r build` Some(PlanRequest::Query(query_plan_request)) => { // Add prefix envs to the context context.add_envs(and_item.envs.iter()); - let execution_graph = plan_query_request(query_plan_request, context).await?; + let execution_graph = plan_query_request(query_plan_request, context) + .await + .map_err(|error| Error::NestPlan { + task_display: task_node.task_display.clone(), + command: Str::from(&command_str[add_item_span.clone()]), + error: Box::new(error), + })?; ExecutionItemKind::Expanded(execution_graph) } // Synthetic task (from CommandHandler) @@ -217,8 +205,7 @@ async fn plan_task_as_execution_node( Some(task_execution_cache_key), &cwd, parent_cache_config, - ) - .with_plan_context(&context)?; + )?; ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(spawn_execution)) } // Normal 3rd party tool command (like `tsc --noEmit`), using potentially mutated script_command @@ -227,9 +214,7 @@ async fn plan_task_as_execution_node( &OsStr::new(&script_command.program).into(), &script_command.envs, &script_command.cwd, - ) - .map_err(TaskPlanErrorKind::ProgramNotFound) - .with_plan_context(&context)?; + )?; let spawn_execution = plan_spawn_execution( context.workspace_path(), Some(task_execution_cache_key), @@ -238,8 +223,7 @@ async fn plan_task_as_execution_node( &script_command.envs, program_path, script_command.args, - ) - .with_plan_context(&context)?; + )?; ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(spawn_execution)) } }; @@ -286,21 +270,17 @@ async fn plan_task_as_execution_node( and_item_index: 0, extra_args: Arc::clone(context.extra_args()), package_path: strip_prefix_for_cache(package_path, context.workspace_path()) - .map_err(|kind| { - TaskPlanErrorKind::PathFingerprintError(PathFingerprintError { - kind, - path_type: PathType::PackagePath, - }) - }) - .with_plan_context(&context)?, + .map_err(|kind| PathFingerprintError { + kind, + path_type: PathType::PackagePath, + })?, }), &BTreeMap::new(), &task_node.resolved_config.resolved_options, context.envs(), Arc::clone(&*SHELL_PROGRAM_PATH), SHELL_ARGS.iter().map(|s| Str::from(*s)).chain(std::iter::once(script)).collect(), - ) - .with_plan_context(&context)?; + )?; items.push(ExecutionItem { execution_item_display, kind: ExecutionItemKind::Leaf(LeafExecutionKind::Spawn(spawn_execution)), @@ -379,7 +359,7 @@ fn resolve_synthetic_cache_config( } } -#[expect(clippy::result_large_err, reason = "TaskPlanErrorKind is large for diagnostics")] +#[expect(clippy::result_large_err, reason = "Error is large for diagnostics")] pub fn plan_synthetic_request( workspace_path: &Arc, prefix_envs: &BTreeMap, @@ -387,10 +367,10 @@ pub fn plan_synthetic_request( execution_cache_key: Option, cwd: &Arc, parent_cache_config: ParentCacheConfig, -) -> Result { +) -> Result { let SyntheticPlanRequest { program, args, cache_config, envs } = synthetic_plan_request; - let program_path = which(&program, &envs, cwd).map_err(TaskPlanErrorKind::ProgramNotFound)?; + let program_path = which(&program, &envs, cwd)?; let resolved_cache_config = resolve_synthetic_cache_config(parent_cache_config, cache_config, cwd); let resolved_options = @@ -424,7 +404,7 @@ fn strip_prefix_for_cache( } } -#[expect(clippy::result_large_err, reason = "TaskPlanErrorKind is large for diagnostics")] +#[expect(clippy::result_large_err, reason = "Error is large for diagnostics")] #[expect( clippy::needless_pass_by_value, reason = "program_path ownership is needed for Arc construction" @@ -437,7 +417,7 @@ fn plan_spawn_execution( envs: &FxHashMap, Arc>, program_path: Arc, args: Arc<[Str]>, -) -> Result { +) -> Result { // all envs available in the current context let mut all_envs = envs.clone(); let cwd = Arc::clone(&resolved_task_options.cwd); @@ -447,7 +427,7 @@ fn plan_spawn_execution( // Resolve envs according cache configs let mut env_fingerprints = EnvFingerprints::resolve(&mut all_envs, &cache_config.env_config) - .map_err(TaskPlanErrorKind::ResolveEnvError)?; + .map_err(Error::ResolveEnv)?; // Add prefix envs to fingerprinted envs env_fingerprints @@ -529,8 +509,7 @@ pub async fn plan_query_request( let task_node_index_graph = context .indexed_task_graph() .query_tasks(query_plan_request.query) - .map_err(TaskPlanErrorKind::TaskQueryError) - .with_plan_context(&context)?; + .map_err(Error::TaskQuery)?; let mut execution_node_indices_by_task_index = FxHashMap::::with_capacity_and_hasher(