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

"unstable_dev" doesn't kill app server on exit (Windows) #6322

Closed
1 task done
trevor-hackett opened this issue May 5, 2023 · 10 comments · Fixed by #6395
Closed
1 task done

"unstable_dev" doesn't kill app server on exit (Windows) #6322

trevor-hackett opened this issue May 5, 2023 · 10 comments · Fixed by #6395

Comments

@trevor-hackett
Copy link

trevor-hackett commented May 5, 2023

What version of Remix are you using?

1.16.0

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

On a Windows machine, create a new Remix app with unstable_dev enabled. (ex: npx create-remix@latest --template https://github.com/remix-run/remix/tree/templates_v2_dev/templates/remix my-app)

Run the app. (npm run dev)

Kill the process (ex: CTRL-C)

Expected Behavior

Dev server and app server should exit

Actual Behavior

Dev server exits but app server process continues to run and has to be killed separately.

@trevor-hackett
Copy link
Author

trevor-hackett commented May 5, 2023

Looks like it is just the way execa and NodeJS works by default on Windows. The spawned process doesn't exit when the process is killed. To get this to work, windowsHide must be disabled.

See:

Despite it saying that a new console would open, I haven't seen another console open with this disabled.

@pcattori
Copy link
Contributor

pcattori commented May 5, 2023

possibly fixed by #6289 which is available in nightly

@trevor-hackett
Copy link
Author

trevor-hackett commented May 5, 2023

This isn't about the development loop (restart app server on changes).

This is when you kill the development process altogether. The child process (app server) is not killed with the dev server. The app server process is left running and you can't start up a new process (npm run dev) until you manually kill the node process

@trevor-hackett
Copy link
Author

Just to confirm. I installed the latest nightly and it is still happening.

For more context this is what I'm doing:

> dev
> remix dev

💿 Building...
💿 Built in 537ms
Waiting for app server (07d97bd0)
> remix-serve build\index.js
Remix dev server ready
Remix App Server started at http://localhost:3000 (http://192.168.169.228:3000)
App server took 278ms
Terminate batch job (Y/N)? y
PS D:\dev\my-app22> npm run dev

> dev
> remix dev

💿 Building...
💿 Built in 528ms
Waiting for app server (07d97bd0)
> remix-serve build\index.js
Remix dev server ready
Error: listen EADDRINUSE: address already in use :::3000
    at Server.setupListenHandle [as _listen2] (node:net:1740:16)
    at listenInCluster (node:net:1788:12)
    at Server.listen (node:net:1876:7)
    at Function.listen (D:\dev\my-app22\node_modules\.pnpm\[email protected]\node_modules\express\lib\application.js:635:24)
    at Object.<anonymous> (D:\dev\my-app22\node_modules\.pnpm\@[email protected]\node_modules\@remix-run\serve\dist\cli.js:48:84)
    at Module._compile (node:internal/modules/cjs/loader:1254:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1308:10)
    at Module.load (node:internal/modules/cjs/loader:1117:32)
    at Function.Module._load (node:internal/modules/cjs/loader:958:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)

Adding windowsHide: false to this fixes the issue:

let newAppServer = execa.command(command, {
stdio: "pipe",
env: {
NODE_ENV: "development",
PATH:
bin + (process.platform === "win32" ? ";" : ":") + process.env.PATH,
REMIX_DEV_HTTP_ORIGIN: stringifyOrigin(httpOrigin),
},
});

Example:

 let newAppServer = execa.command(command, { 
   stdio: "pipe", 
   env: { 
     NODE_ENV: "development", 
     PATH: 
       bin + (process.platform === "win32" ? ";" : ":") + process.env.PATH, 
     REMIX_DEV_HTTP_ORIGIN: stringifyOrigin(httpOrigin), 
   }, 
   windowsHide: false
 }); 
PS D:\dev\my-app22> npm run dev

> dev
> remix dev

💿 Building...
💿 Built in 528ms
Waiting for app server (07d97bd0)
> remix-serve build\index.js
Remix dev server ready
Remix App Server started at http://localhost:3000 (http://192.168.169.228:3000)
App server took 272ms
Terminate batch job (Y/N)? 
^C
PS D:\dev\my-app22> npm run dev

> dev
> remix dev

💿 Building...
💿 Built in 531ms
Waiting for app server (07d97bd0)
> remix-serve build\index.js
Remix dev server ready
Remix App Server started at http://localhost:3000 (http://192.168.169.228:3000)
App server took 268ms

@pcattori
Copy link
Contributor

pcattori commented May 6, 2023

Thanks for the great detective work @trevor-hackett ! Adding this to the dev server TODOs

@roniaxe
Copy link

roniaxe commented May 9, 2023

Can confirm to have the same issue. Starting the server once is OK, killing it and starting again getting me that error:

Remix dev server ready
Error: listen EADDRINUSE: address already in use :::3000
    at Server.setupListenHandle [as _listen2] (node:net:1485:16)
    at listenInCluster (node:net:1533:12)
    at Server.listen (node:net:1621:7)
    at Function.listen (D:\wellspring-montessori-app\wellspring-frontend-remix\node_modules\express\lib\application.js:635:24)
    at Object.<anonymous> (D:\wellspring-montessori-app\wellspring-frontend-remix\node_modules\@remix-run\serve\dist\cli.js:48:84)
    at Module._compile (node:internal/modules/cjs/loader:1159:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1213:10)
    at Module.load (node:internal/modules/cjs/loader:1037:32)
    at Function.Module._load (node:internal/modules/cjs/loader:878:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)

I then have to kill the port manually with taskkill /PID PID /F in order to start again on port 3000

pcattori added a commit that referenced this issue May 15, 2023
pcattori added a commit that referenced this issue May 15, 2023
@pcattori
Copy link
Contributor

@trevor-hackett merged #6322, so you'll be able to try this fix out on nightly tomorrow.

I'm working on getting Parallels setup for local Windows dev on my mac, but haven't done so yet. So in the meantime, any feedback from you or others using Windows is invaluable!

@MichaelDeBoey MichaelDeBoey linked a pull request May 15, 2023 that will close this issue
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-360e77e-20230516 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.16.1-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.16.1 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants