Skip to content

Commit b8b3df9

Browse files
committed
fix: exit with non-zero status for signal-terminated processes
1 parent 98141a4 commit b8b3df9

File tree

8 files changed

+101
-20
lines changed

8 files changed

+101
-20
lines changed

crates/vite_task/src/session/event.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::time::Duration;
1+
use std::{process::ExitStatus, time::Duration};
22

33
use bstr::BString;
44
// Re-export ExecutionItemDisplay from vite_task_plan since it's the canonical definition
@@ -44,6 +44,24 @@ pub enum CacheStatus {
4444
Hit { replayed_duration: Duration },
4545
}
4646

47+
/// Convert ExitStatus to an i32 exit code.
48+
/// On Unix, if terminated by signal, returns 128 + signal_number.
49+
pub fn exit_status_to_code(status: &ExitStatus) -> i32 {
50+
#[cfg(unix)]
51+
{
52+
use std::os::unix::process::ExitStatusExt;
53+
status.code().unwrap_or_else(|| {
54+
// Process was terminated by signal, use Unix convention: 128 + signal
55+
status.signal().map(|sig| 128 + sig).unwrap_or(1)
56+
})
57+
}
58+
#[cfg(not(unix))]
59+
{
60+
// Windows always has an exit code
61+
status.code().unwrap_or(1)
62+
}
63+
}
64+
4765
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
4866
pub struct ExecutionId(u32);
4967

@@ -68,5 +86,5 @@ pub enum ExecutionEventKind {
6886
Start { display: Option<ExecutionItemDisplay>, cache_status: CacheStatus },
6987
Output { kind: OutputKind, content: BString },
7088
Error { message: String },
71-
Finish { status: Option<i32>, cache_update_status: CacheUpdateStatus },
89+
Finish { status: Option<ExitStatus>, cache_update_status: CacheUpdateStatus },
7290
}

crates/vite_task/src/session/execute/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ impl ExecutionContext<'_> {
157157
self.event_handler.handle_event(ExecutionEvent {
158158
execution_id,
159159
kind: ExecutionEventKind::Finish {
160-
status: Some(0),
160+
status: None,
161161
cache_update_status: CacheUpdateStatus::NotUpdated(
162162
CacheNotUpdatedReason::CacheDisabled,
163163
),
@@ -237,7 +237,7 @@ impl ExecutionContext<'_> {
237237
self.event_handler.handle_event(ExecutionEvent {
238238
execution_id,
239239
kind: ExecutionEventKind::Finish {
240-
status: Some(0),
240+
status: None,
241241
cache_update_status: CacheUpdateStatus::NotUpdated(
242242
CacheNotUpdatedReason::CacheHit,
243243
),
@@ -339,7 +339,7 @@ impl ExecutionContext<'_> {
339339
self.event_handler.handle_event(ExecutionEvent {
340340
execution_id,
341341
kind: ExecutionEventKind::Finish {
342-
status: result.exit_status.code(),
342+
status: Some(result.exit_status),
343343
cache_update_status,
344344
},
345345
});

crates/vite_task/src/session/reporter.rs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use std::{
44
collections::HashSet,
55
io::Write,
6+
process::ExitStatus as StdExitStatus,
67
sync::{Arc, LazyLock},
78
time::Duration,
89
};
@@ -12,7 +13,10 @@ use vite_path::AbsolutePath;
1213

1314
use super::{
1415
cache::{format_cache_status_inline, format_cache_status_summary},
15-
event::{CacheStatus, ExecutionEvent, ExecutionEventKind, ExecutionId, ExecutionItemDisplay},
16+
event::{
17+
CacheStatus, ExecutionEvent, ExecutionEventKind, ExecutionId, ExecutionItemDisplay,
18+
exit_status_to_code,
19+
},
1620
};
1721

1822
/// Wrap of `OwoColorize` that ignores style if `NO_COLOR` is set.
@@ -55,7 +59,8 @@ const CACHE_MISS_STYLE: Style = Style::new().purple();
5559
struct ExecutionInfo {
5660
display: Option<ExecutionItemDisplay>,
5761
cache_status: CacheStatus, // Non-optional, determined at Start
58-
exit_status: Option<i32>,
62+
/// Exit status from the process. None means no process was spawned (cache hit or in-process).
63+
exit_status: Option<StdExitStatus>,
5964
error_message: Option<String>,
6065
}
6166

@@ -226,12 +231,11 @@ impl<W: Write> LabeledReporter<W> {
226231
self.stats.failed += 1;
227232
}
228233

229-
fn handle_finish(&mut self, execution_id: ExecutionId, status: Option<i32>) {
234+
fn handle_finish(&mut self, execution_id: ExecutionId, status: Option<StdExitStatus>) {
230235
// Update failure statistics
231-
if let Some(s) = status {
232-
if s != 0 {
233-
self.stats.failed += 1;
234-
}
236+
// None means success (cache hit or in-process), Some checks the actual exit status
237+
if status.is_some_and(|s| !s.success()) {
238+
self.stats.failed += 1;
235239
}
236240

237241
// Update execution info exit status
@@ -403,21 +407,23 @@ impl<W: Write> LabeledReporter<W> {
403407
let _ = write!(self.writer, ": {}", command_display.style(COMMAND_STYLE));
404408

405409
// Execution result icon
406-
match exec.exit_status {
407-
Some(0) => {
410+
// None means success (cache hit or in-process), Some checks actual status
411+
match &exec.exit_status {
412+
None => {
413+
let _ = write!(self.writer, " {}", "✓".style(Style::new().green().bold()));
414+
}
415+
Some(status) if status.success() => {
408416
let _ = write!(self.writer, " {}", "✓".style(Style::new().green().bold()));
409417
}
410-
Some(code) => {
418+
Some(status) => {
419+
let code = exit_status_to_code(status);
411420
let _ = write!(
412421
self.writer,
413422
" {} {}",
414423
"✗".style(Style::new().red().bold()),
415424
format!("(exit code: {code})").style(Style::new().red())
416425
);
417426
}
418-
None => {
419-
let _ = write!(self.writer, " {}", "?".style(Style::new().bright_black()));
420-
}
421427
}
422428
let _ = writeln!(self.writer);
423429

@@ -536,11 +542,13 @@ impl<W: Write> Reporter for LabeledReporter<W> {
536542
// 1. All tasks succeed → return Ok(())
537543
// 2. Exactly one task failed → return Err with that task's exit code
538544
// 3. More than one task failed → return Err(1)
545+
// Note: None means success (cache hit or in-process)
539546
let failed_exit_codes: Vec<i32> = self
540547
.executions
541548
.iter()
542-
.filter_map(|exec| exec.exit_status)
543-
.filter(|&status| status != 0)
549+
.filter_map(|exec| exec.exit_status.as_ref())
550+
.filter(|status| !status.success())
551+
.map(exit_status_to_code)
544552
.collect();
545553

546554
match failed_exit_codes.as_slice() {
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"name": "signal-exit-test"
3+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Tests exit code behavior for signal-terminated processes
2+
# Unix-only: Windows doesn't have Unix signals, so exit codes differ
3+
4+
[[e2e]]
5+
name = "signal terminated task returns non-zero exit code"
6+
platform = "unix"
7+
steps = [
8+
"vite run abort # SIGABRT -> exit code 134",
9+
]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
source: crates/vite_task_bin/tests/e2e_snapshots/main.rs
3+
expression: e2e_outputs
4+
input_file: crates/vite_task_bin/tests/e2e_snapshots/fixtures/signal-exit
5+
---
6+
[134]> vite run abort # SIGABRT -> exit code 134
7+
$ node -e "process.kill(process.pid, 6)"
8+
9+
10+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
11+
Vite+ Task RunnerExecution Summary
12+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
13+
14+
Statistics: 1 tasks0 cache hits1 cache misses1 failed
15+
Performance: 0% cache hit rate
16+
17+
Task Details:
18+
────────────────────────────────────────────────
19+
[1] signal-exit-test#abort: $ node -e "process.kill(process.pid, 6)" ✗ (exit code: 134)
20+
Cache miss: no previous cache entry found
21+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"tasks": {
3+
"abort": {
4+
"command": "node -e \"process.kill(process.pid, 6)\""
5+
}
6+
}
7+
}

crates/vite_task_bin/tests/e2e_snapshots/main.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ struct E2e {
7676
#[serde(default)]
7777
pub cwd: RelativePathBuf,
7878
pub steps: Vec<Step>,
79+
/// Optional platform filter: "unix" or "windows". If set, test only runs on that platform.
80+
#[serde(default)]
81+
pub platform: Option<Str>,
7982
}
8083

8184
#[derive(serde::Deserialize, Default)]
@@ -165,6 +168,18 @@ async fn run_case_inner(tmpdir: &AbsolutePath, fixture_path: &Path, fixture_name
165168

166169
let mut e2e_count = 0u32;
167170
for e2e in cases_file.e2e_cases {
171+
// Skip test if platform doesn't match
172+
if let Some(platform) = &e2e.platform {
173+
let should_run = match platform.as_str() {
174+
"unix" => cfg!(unix),
175+
"windows" => cfg!(windows),
176+
other => panic!("Unknown platform '{}' in test '{}'", other, e2e.name),
177+
};
178+
if !should_run {
179+
continue;
180+
}
181+
}
182+
168183
let e2e_stage_path = tmpdir.join(format!("{}_e2e_stage_{}", fixture_name, e2e_count));
169184
e2e_count += 1;
170185
assert!(copy_dir(fixture_path, &e2e_stage_path).unwrap().is_empty());

0 commit comments

Comments
 (0)