fix(core): propagate continuous task failures to dependent tasks #33492
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for nx-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
View your CI Pipeline Execution ↗ for commit 812906d
☁️ Nx Cloud last updated this comment at |
leosvelperez
force-pushed
the
nxc-3370
branch
from
cb4bc98 to
812906d
Compare
FrozenPandaz
approved these changes
Nov 25, 2025
FrozenPandaz
approved these changes
Nov 25, 2025
|
|
||
| // we're not cleaning up, so this is an unexpected exit, fail the task | ||
| if (!this.cleaningUp) { | ||
| console.error( |
Collaborator
There was a problem hiding this comment.
This is not going to show up in the task output.. can you fix this in a followup PR please?
FrozenPandaz
pushed a commit
that referenced
this pull request
Nov 25, 2025
) ## Current Behavior When a continuous task depends on another continuous task and the dependent task exits with an error, the parent task continues running indefinitely. The task execution never terminates, leaving processes running in the background. For example, if task `a` (continuous) depends on task `b` (continuous), and task `b` exits with error code 1, task `a` will continue running even though its dependency failed. ## Expected Behavior When a continuous task exits (with any exit code), the failure should be propagated to dependent tasks: 1. The failed continuous task should be marked as failed 2. Dependent continuous tasks should be marked as skipped 3. All affected continuous tasks should be killed 4. Task execution should terminate with an error --- ## Changes Made ### 1. Restore Error Handling in Continuous Task Exit Handlers - Re-added the `cleaningUp` flag that was removed in a previous TUI-related commit - Modified `onExit` handlers for both regular and shared continuous tasks to: - Check if the task exited during normal cleanup vs. unexpectedly - Call `complete()` with 'failure' status for unexpected exits - Log error messages for debugging ### 2. Fix `cleanUpUnneededContinuousTasks()` Logic The previous implementation always added `initializingTaskIds` to the needed set, even when those tasks were already completed. This prevented dependency tasks from being killed when the top-level task exited. Fixed by: - Only adding tasks from `initializingTaskIds` if they are still incomplete - Keeping dependencies of incomplete tasks alive - This ensures continuous tasks are killed when no longer needed, whether a dependency fails or a top-level task exits ### 3. Prevent Status Overwrites Added a check in `onExit` handlers to only set status to `Stopped` if the task hasn't already been completed. This prevents the async `onExit` callback from overwriting the correct status (like 'skipped' or 'failure') with 'Stopped'. ### 4. Fix Signal Handling in `PseudoTtyProcess.kill()` The Rust pseudo-terminal defaults to SIGINT when no signal is provided, which does not reliably terminate child processes in PTY sessions. Fixed by: - Defaulting to SIGTERM in the JavaScript wrapper (rather than changing the Rust default) - The JS wrapper is the API boundary that should match Node.js semantics, where `childProcess.kill(undefined)` defaults to SIGTERM - The Rust default of SIGINT is appropriate for interactive use (Ctrl-C), while programmatic cleanup needs SIGTERM - This ensures child processes are properly killed when `runningTask.kill()` is called **File:** `packages/nx/src/tasks-runner/pseudo-terminal.ts` ## Technical Details The fix leverages the existing task failure propagation mechanism in `complete()` instead of using `process.exit(1)`, which: - Allows proper cleanup through normal execution flow - Respects the `--bail` flag configuration - Works correctly with both TUI and non-TUI modes - Maintains consistency with how other task failures are handled
Contributor
|
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Current Behavior
When a continuous task depends on another continuous task and the dependent task exits with an error, the parent task continues running indefinitely. The task execution never terminates, leaving processes running in the background.
For example, if task
a(continuous) depends on taskb(continuous), and taskbexits with error code 1, taskawill continue running even though its dependency failed.Expected Behavior
When a continuous task exits (with any exit code), the failure should be propagated to dependent tasks:
Changes Made
1. Restore Error Handling in Continuous Task Exit Handlers
cleaningUpflag that was removed in a previous TUI-related commitonExithandlers for both regular and shared continuous tasks to:complete()with 'failure' status for unexpected exits2. Fix
cleanUpUnneededContinuousTasks()LogicThe previous implementation always added
initializingTaskIdsto the needed set, even when those tasks were already completed. This prevented dependency tasks from being killed when the top-level task exited.Fixed by:
initializingTaskIdsif they are still incomplete3. Prevent Status Overwrites
Added a check in
onExithandlers to only set status toStoppedif the task hasn't already been completed. This prevents the asynconExitcallback from overwriting the correct status (like 'skipped' or 'failure') with 'Stopped'.4. Fix Signal Handling in
PseudoTtyProcess.kill()The Rust pseudo-terminal defaults to SIGINT when no signal is provided, which does not reliably terminate child processes in PTY sessions.
Fixed by:
childProcess.kill(undefined)defaults to SIGTERMrunningTask.kill()is calledFile:
packages/nx/src/tasks-runner/pseudo-terminal.tsTechnical Details
The fix leverages the existing task failure propagation mechanism in
complete()instead of usingprocess.exit(1), which:--bailflag configuration