fix: preserve signal exit code on Unix#2154
Open
liangmiQwQ wants to merge 12 commits into
Open
Conversation
The N-API command path used ExitStatus::code directly, so signal-terminated children were collapsed to exit code 1. Reuse a shared shell-compatible conversion and guard SIGILL as exit code 132.
Process exit statuses were converted independently across Rust and Node runners, so signal-terminated children became generic failures or even successes. Centralize Rust conversion, map Node signal names to 128 + signal, and cover SIGILL in both runner modes.
Let the snapshot runner and vpt helper depend on vite_shared so both use the canonical Unix signal-to-exit-code conversion instead of maintaining duplicate implementations.
✅ Deploy Preview for viteplus-preview canceled.
|
Collaborator
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
liangmiQwQ
changed the title
…e-signal-exit-code # Conflicts: # crates/vite_shared/src/lib.rs
liangmiQwQ
commented
Jul 16, 2026
|
|
||
| // Keep output stable when another dependency enables serde_json's | ||
| // workspace-wide `preserve_order` feature. | ||
| root.sort_all_objects(); |
Collaborator
Author
There was a problem hiding this comment.
This is because this PR adds vite_shared crate as a dependency. And vite_shared enables serde_json/preserve_order feature, while Cargo is merging this change, json object will no longer be sorted, causes unrelated snapshots to differ.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In the current codebase, there are a lot of places where we simply use
result.status.code().unwrap_or(1)to handle the exit code.However, according to Rust std docs, if the process is terminated by a signal, the
status.code()will becomeNoneon Unix. We should readExitStatusto know the signal and get the real exit code.This PR fixes them.
Partially related to #2041.