Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion crates/vite_task/src/schedule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ pub struct PreExecutionStatus {
pub cache_status: CacheStatus,
pub display_options: DisplayOptions,
}

#[derive(Debug, Serialize, Deserialize, Clone)]
pub enum CacheStatus {
/// Cache miss with reason.
Expand Down Expand Up @@ -127,8 +128,31 @@ impl ExecutionPlan {
#[tracing::instrument(skip(self, workspace))]
pub async fn execute(self, workspace: &mut Workspace) -> Result<ExecutionSummary, Error> {
let mut execution_statuses = Vec::<ExecutionStatus>::with_capacity(self.steps.len());
let mut has_failed = false;
for step in self.steps {
execution_statuses.push(Self::execute_resolved_task(step, workspace).await?);
if has_failed {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This incorrectly skips tasks that are not yet executed but do not depend on the failed tasks.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it looks like it can't be handled simply and needs to be determined based on the task_graph

// skip executing the task and display the task name and index
let display_options = step.display_options;
execution_statuses.push(ExecutionStatus {
execution_id: Uuid::new_v4().to_string(),
pre_execution_status: PreExecutionStatus {
display_command: get_display_command(display_options, &step),
task: step,
cache_status: CacheStatus::CacheMiss(CacheMiss::NotFound),
display_options,
},
execution_result: Err(ExecutionFailure::SkippedDueToFailedDependency),
});
continue;
}

let status = Self::execute_resolved_task(step, workspace).await?;
if let Ok(exit_status) = status.execution_result {
if exit_status != 0 {
has_failed = true;
}
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The failure tracking logic only checks for non-zero exit codes but ignores Err cases. If execution_result is Err(ExecutionFailure::...), the task has failed but has_failed won't be set to true, allowing subsequent tasks to execute when they should be skipped.

Suggested change
}
}
} else {
// If execution_result is Err, treat as failure
has_failed = true;

Copilot uses AI. Check for mistakes.
}
execution_statuses.push(status);
}
Ok(ExecutionSummary { execution_statuses })
}
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/snap-tests/fail-fast/failure.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
console.log('failure');
process.exit(1);
10 changes: 10 additions & 0 deletions packages/cli/snap-tests/fail-fast/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"scripts": {
"ready": "vite run test && vite run script4",
"test": "vite run script1 && vite run script2 && vite run script3",
"script1": "echo 'success 1'",
"script2": "node failure.js",
"script3": "echo 'success 3'",
"script4": "echo 'success 4'"
}
}
61 changes: 61 additions & 0 deletions packages/cli/snap-tests/fail-fast/snap.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
[1]> vite run test # skip script3 when script2 failed
$ echo 'success 1'
success 1


$ node failure.js
failure



━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Vite+ Task Runner • Execution Summary
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Statistics: 3 tasks • 0 cache hits • 3 cache misses • 1 failed
Performance: 0% cache hit rate

Task Details:
────────────────────────────────────────────────
[1] script1: $ echo 'success 1' ✓
→ Cache miss: no previous cache entry found
·······················································
[2] script2: $ node failure.js ✗ (exit code: 1)
→ Cache miss: no previous cache entry found
·······················································
[3] test: $ vite run script3 ⊘ (skipped: dependency failed)
→ Cache miss: no previous cache entry found
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that skipped tasks still report cache status, since they don't look up cache at all.

━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

[1]> vite run ready # support nested tasks
$ echo 'success 1' (✓ cache hit, replaying)
success 1


$ node failure.js
failure




━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Vite+ Task Runner • Execution Summary
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

Statistics: 4 tasks • 1 cache hits • 3 cache misses • 1 failed
Performance: 25% cache hit rate, <variable>ms saved in total

Task Details:
────────────────────────────────────────────────
[1] script1: $ echo 'success 1' ✓
→ Cache hit - output replayed - <variable>ms saved
·······················································
[2] script2: $ node failure.js ✗ (exit code: 1)
→ Cache miss: no previous cache entry found
·······················································
[3] test: $ vite run script3 ⊘ (skipped: dependency failed)
→ Cache miss: no previous cache entry found
·······················································
[4] ready: $ vite run script4 ⊘ (skipped: dependency failed)
→ Cache miss: no previous cache entry found
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
9 changes: 9 additions & 0 deletions packages/cli/snap-tests/fail-fast/steps.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"env": {
"VITE_DISABLE_AUTO_INSTALL": "1"
},
"commands": [
"vite run test # skip script3 when script2 failed",
"vite run ready # support nested tasks"
]
}
Loading