joshwcomeau / guppy

🐠A friendly application manager and task runner for React.js
ISC License
3.27k stars 154 forks source link

Confirmation prompt upon exit if the servers are still in running state. #364

Closed JayaKrishnaNamburu closed 5 years ago

JayaKrishnaNamburu commented 5 years ago

Confirmation popup when clicked on close if any project is already in running state.

First, I am pretty excited to see an application like this for the front end but here is a bug or maybe a un-implemented feature which I would like to see in this.

Current Behaviour

When we start a server and then click on the close window, the application closes but the server will keep on running in the background as the server is not being stopped upon exit, and next when we open the application again and start the project as we all know the project starts with the new port as the previous port is already occupied.

When I read the docs, I understood the motivation for the project is making beginner friendly which removes the use of the terminal but in this case, we need to go back and kill the process which are running in the ports.

guppy

Expected Solution

Showing a prompt upon exit by checking the status of the projects that are running, so the use manually decides on stopping the servers.

Thank you guys for the awesome work, looking ahead to start my contribution.

JayaKrishnaNamburu commented 5 years ago

Have something similar #211 but it describes about another scenario.

Haroenv commented 5 years ago

Maybe the child processes could be killed without warning, as an alternative solution. Not sure which is better

AWolf81 commented 5 years ago

@JayaKrishnaNamburu Thanks for reporting the issue.

We're having a killAllRunningProcesses in place here. But maybe it's not called or the active processes are not correclty stored in the processIds Array.

The killing should happen in the electron.js file.

We could also check if it's possible to add a test for the process killing as mentioned in issue #344.

AWolf81 commented 5 years ago

Seems like the processes are killed correctly if it's closed with top-left icon & close (on Windows) but it's not correctly working if closed with the x-button in top right corner.

grafik (Process correctly killed if closed like this)

Really a weird bug. killProcessId from kill-process-id.service is correctly called in both cases but for the x-button the Node child process is not terminated.

OK, Guppy closes too early with-out fully finishing taskkill as spawn is asynchronous and we're not waiting for it to finish. Changing to .spawnSync will fix the issue (tested on Windows). Doing this sync and blocking the event loop is OK here as we're about to terminate Guppy.

I'll create a PR later today.

I think killing with-out a message is OK but I'm also not sure what's better.

JayaKrishnaNamburu commented 5 years ago

Yeah @AWolf81 i think without a promt too should be fine if the servers which are running being killed :smile:

superhawk610 commented 5 years ago

Fixed in #367.