Skip to content

Conversation

@taras
Copy link
Member

@taras taras commented Oct 4, 2025

Motivation

Problem 1: Binding SIGTERM in windows in Deno triggers this warning Windows only supports ctrl-c (SIGINT), ctrl-break (SIGBREAK), and ctrl-close (SIGUP). We don't want main to produce any warnings.

To verify that this is working on Windows, I ran tests on windows. Which lead to Problem 2: TypeError: Windows only supports ctrl-c (SIGINT), ctrl-break (SIGBREAK), and ctrl-close (SIGUP), but got SIGTERM (note the but got SIGTERM)

Approach

The solution to problem 1 is to avoid binding SIGTERM in Deno and Node using their platform flags.

The solution to problem 2 is to use ctrlc-windows instead of sending SIGINT to terminate in Windows.

The test produced stderr which caused the test to fail in pwsh. In bash, it would hang because bash is capturing command's stderr which disrupted ctrlc-windows function. In @effectionx/process we use ctrlc-windows with a process in detached mode which bypasses this problem. I used cmd shell which doesn't disrupt stderr like bash, nor fails the process when stderr is produced.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 4, 2025

Open in StackBlitz

npm i https://pkg.pr.new/thefrontside/effection@1031

commit: 9e96b4b

@taras taras requested a review from cowboyd October 4, 2025 07:15
@taras taras force-pushed the tm/no-sigterm-windows branch 2 times, most recently from 8c07835 to 443a2d9 Compare October 4, 2025 11:56
@cowboyd
Copy link
Member

cowboyd commented Oct 4, 2025

Odd. It looks like the tests don't fail on Windows, but they don't pass either.

@taras
Copy link
Member Author

taras commented Oct 5, 2025

It's wierd that looks like it succeeds, but I can reproduce it locally that in pwsh it's returning status code 1. It's defaulting to pwsh, but passes successfully on bash, so I'm just going to switch it to bash.

@taras taras force-pushed the tm/no-sigterm-windows branch from 2bb4609 to 87301b2 Compare October 5, 2025 00:50
@taras
Copy link
Member Author

taras commented Oct 5, 2025

Now it's hanging on the main.test.ts in Windows bash.

@taras taras force-pushed the tm/no-sigterm-windows branch 2 times, most recently from 36fd4a9 to ccc3d66 Compare October 5, 2025 03:28
@taras
Copy link
Member Author

taras commented Oct 5, 2025

The test produced stderr which caused the test to fail in pwsh. In bash, it would hang because bash is capturing command's stderr which disrupted ctrlc-windows function. In @effectionx/process we use ctrlc-windows with a process in detached mode which bypasses this problem. Here, I had to use cmd shell which doesn't disrupt stderr like bash, nor fail the process when stderr is produced.

@taras taras requested a review from cowboyd October 6, 2025 14:24
@taras taras force-pushed the tm/no-sigterm-windows branch from ccc3d66 to 8da04ec Compare October 6, 2025 22:37
@taras taras enabled auto-merge October 6, 2025 22:40
@taras taras force-pushed the tm/no-sigterm-windows branch from 8da04ec to 9e96b4b Compare October 6, 2025 23:01
@taras taras merged commit 3ee3873 into v3 Oct 6, 2025
8 checks passed
@taras taras deleted the tm/no-sigterm-windows branch October 6, 2025 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants