Skip to content

Commit

Permalink
fix(watch): await persistent run task instead of abort (#9152)
Browse files Browse the repository at this point in the history
### Description

This PR fixes an issue where the TUI was getting events from a previous
run after an `update_tasks` call causing it to exit.

One can tell this is happening by inspecting the logs:
```
2024-09-16T13:17:16.509-0400 [DEBUG] turborepo_ui::tui::app: updating task list: ["docs#dev", "web#dev"]
...
2024-09-16T13:17:16.512-0400 [DEBUG] turborepo_ui::tui::app: finishing task web#dev
```
The TUI is receiving that `web#dev` finish event from the previous run
which confuses the TUI as web#dev hasn't been started yet in this run.
This appears to only happen for tasks that don't exit within a 500ms of
receiving `SIGINT` and need to be forcibly killed.

### Testing Instructions

Use `create-turbo` to create a basic repro. Change one of the dev
scripts to have a long running `SIGINT` handler:
```
"dev": "trap 'sleep 5 && echo fin' SIGINT; next dev --turbo"
```

Observe that editing root `package.json` (just adding/changing
`"description"` will do) while `turbo watch dev` is running results in a
crash:
<img width="749" alt="Screenshot 2024-09-16 at 1 36 03 PM"
src="https://github.com/user-attachments/assets/387c0453-6ec7-43a9-92e9-daf5080a6b6f">

Checkout the changes in this PR and try the same thing. You should
observe that `turbo_dev --skip-infer watch dev` no longer crashes on a
root `package.json` change.
<img width="584" alt="Screenshot 2024-09-16 at 1 37 25 PM"
src="https://github.com/user-attachments/assets/6b9aaa68-da7b-415d-8f19-dffe45f3de62">
You can see that the `SIGINT` handler has not run as there is no `fin`
displayed in the output on the restart.
  • Loading branch information
chris-olszewski committed Sep 16, 2024
1 parent 8749399 commit c3b7be9
Showing 1 changed file with 3 additions and 1 deletion.
4 changes: 3 additions & 1 deletion crates/turborepo-lib/src/run/watch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,9 @@ impl WatchClient {
{
// Shut down the tasks for the run
stopper.stop().await;
run_task.abort();
// Run should exit shortly after we stop all child tasks, wait for it to finish
// to ensure all messages are flushed.
let _ = run_task.await;
}
if let Some(sender) = &self.ui_sender {
let task_names = self.run.engine.tasks_with_command(&self.run.pkg_dep_graph);
Expand Down

0 comments on commit c3b7be9

Please sign in to comment.