Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows shims show Terminate batch job (Y/N)? on ctrl-c #269

Closed
rotu opened this issue Oct 30, 2023 · 15 comments
Closed

Windows shims show Terminate batch job (Y/N)? on ctrl-c #269

rotu opened this issue Oct 30, 2023 · 15 comments
Assignees
Labels
bug Something isn't working

Comments

@rotu
Copy link
Contributor

rotu commented Oct 30, 2023

What version?

0.21.0

Which command?

No response

What happened?

When launching a shim on Windows, interrupting with ctrl-c can result in the confusing output "Terminate batch job (Y/N)?".

To reproduce:

node -e "setTimeout(new Function, 30000);"

and stop the program with ctrl c.

Any logs?

No response

Operating system?

Windows

Architecture?

x64

@rotu rotu added the bug Something isn't working label Oct 30, 2023
@rotu
Copy link
Contributor Author

rotu commented Oct 30, 2023

Current viable workarounds:

  1. Use proto run directly: proto run node -- -e "setTimeout(new Function, 30000);"
  2. (if using cmd.exe) use DOSKEY to set up the shims instead:
doskey /exename=cmd.exe npx=proto run node --bin npx.cmd -- $*
  1. (if using powershell) use a Function instead:
function node { proto run node -- @args }
  1. Edit the shim to add an exit statement when node finishes, instead of returning control to the script:
proto.exe run node --  %* & EXIT

@milesj
Copy link
Contributor

milesj commented Nov 1, 2023

I don't know windows very well, is the exit strategy really the best way to handle this?

@rotu
Copy link
Contributor Author

rotu commented Nov 1, 2023

I don't know windows very well, is the exit strategy really the best way to handle this?

I don't know Windows very well either. I think these are all limited workarounds, and that the best way to handle this is to not use cmd shims but to use exe executables instead.

The exit hack is a bad user experience if you're calling node from a cmd shell (since CALL node would terminates the shell when node ends) but great if you're using powershell or calling it from another executable.


Windows itself has some facility for shell aliases under Settings -> Apps -> Advanced app settings -> App execution aliases, which seem to get installed to C:\Users\dan\AppData\Local\Microsoft\WindowsApps. I think this is in UWP AppExecutionAlias.

But it looks like a lot of platform-specific stuff that may or may not be worth it, and is not as user-hackable as shims.

@rotu
Copy link
Contributor Author

rotu commented Nov 1, 2023

I've come up with an even better way of suppressing the prompt. It's ugly and I hate it, but it seems pretty effective:

setlocal enabledelayedexpansion 
proto.exe run node -- %* & set RC=!errorlevel! & call;
EXIT /b %RC%

On ctrl-c, the errorlevel gets set, but not immediately. That is, when doing

foo && echo success %ERRORLEVEL% || echo failure %ERRORLEVEL%
echo RC %ERRORLEVEL%

even though the return code of foo affects the branch taken, the error level is not yet updated. Fortunately, enabledelayedexpansion with !varname! allows us to get this value correctly. Then call; clobbers the exit code before cmd reacts to it, thus suppressing the prompt. Finally, we exit the batch script with the captured error code.

EDIT: this misbehaves with npx for some reason I don't understand. I should probably see what the npx kicker looks like after #268 before any more effort on this.

@milesj
Copy link
Contributor

milesj commented Nov 29, 2023

Figured this out here #308

@rotu
Copy link
Contributor Author

rotu commented Nov 29, 2023

Figured this out here #308

I have my doubts that script-based shims can overcome all the idiosyncrasies of cmd and PowerShell. That said, if anyone can make it work, you can!

@milesj
Copy link
Contributor

milesj commented Nov 29, 2023

Actually stole it from here: pnpm/cmd-shim#39

Looks like npm/pnpm/yarn are starting to use it too.

@milesj
Copy link
Contributor

milesj commented Nov 29, 2023

Ok out.

@milesj milesj closed this as completed Nov 29, 2023
@rotu
Copy link
Contributor Author

rotu commented Nov 29, 2023

None of these are quite dealbreakers but here are issues I currently attribute to the use of shims:

  • deno and deno.exe resolve to different versions (various shims versus .proto\bin\deno.exe)
  • Start-Process deno opens deno.ps1 in an editor versus Start-Process deno.exe which invokes the executable.
  • Another is that, depending which kicker gets called, my PowerShell profile Profile.ps1 may or may not run.

Ok out.

Awesome! I tried out the fix and it definitely fixes this issue (and I think also fixes the more annoying bug where a node subprocess/server will stay alive after interrupting the parent).

@milesj
Copy link
Contributor

milesj commented Nov 29, 2023

deno and deno.exe resolve to different versions (various shims versus .proto\bin\deno.exe)

This is expected. Shims will dynamically change versions, while the exe is manually pinned to a version. We can't make the exe dynamic, so if you don't want the dynamic part, just don't use the shims in PATH.

Start-Process deno opens deno.ps1 in an editor versus Start-Process deno.exe which invokes the executable.

Do you have .ps1 in your PATHEXT variable? It should not exist by default. Sure it's not hitting deno.cmd?

@milesj
Copy link
Contributor

milesj commented Nov 29, 2023

In the long run, I want to ditch ps1/cmd shims, and somehow generate .exe files on demand. They would be simple wrappers for spawning a child process.

I'm pretty sure this is how chocolatey works.

@rotu
Copy link
Contributor Author

rotu commented Nov 30, 2023

We can't make the exe dynamic, so if you don't want the dynamic part, just don't use the shims in PATH.

It's the opposite - I do want the executable to be dynamic. But proto.exe is in .proto/bin, so if I want to get deno.exe out of my path, I have to manually clean out the unwanted executables from the .proto\bin folder.

Start-Process deno opens deno.ps1 in an editor versus Start-Process deno.exe which invokes the executable.

Do you have .ps1 in your PATHEXT variable? It should not exist by default. Sure it's not hitting deno.cmd?

Whether or not I put .ps1 in my PATHEXT variable, this is the case. Powershell has different handling so that ps1 files get put ahead of normal executable resolution when using the call operator/&, the dot sourcing operator/., and Invoke-Expression/iex. But scripts are still not executables, and Windows tries to make sure you never forget it!

image

@milesj
Copy link
Contributor

milesj commented Nov 30, 2023

It's the opposite - I do want the executable to be dynamic. But proto.exe is in .proto/bin, so if I want to get deno.exe out of my path, I have to manually clean out the unwanted executables from the .proto\bin folder.

You could move the proto binary somewhere else and just ignore that path.

Whether or not I put .ps1 in my PATHEXT variable, this is the case.

Why is windows so dumb 😢

@rotu
Copy link
Contributor Author

rotu commented Nov 30, 2023

You could move the proto binary somewhere else and just ignore that path.

I plan to remove that path from my $env:PATH and add an App Paths registry entry. This may or may not work.

Why is windows so dumb 😢

I bet it's because this way MS didn't have to worry about name collisions between Cmdlets and the pre-existing morass of executables and WSH scripts. Fun fact: when you say "pwsh" out loud, it's a fair approximation of the sound you get when you break windows!

@rotu
Copy link
Contributor Author

rotu commented Nov 30, 2023

The registry entry didn't work great. Wound up moving the executable to the shims folder and dropping the bin folder from my PATH

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants