Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

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

Closed
JayaKrishnaNamburu opened this issue Feb 14, 2019 · 6 comments
Labels
bug Something isn't working

Comments

@JayaKrishnaNamburu
Copy link

JayaKrishnaNamburu commented Feb 14, 2019

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
Copy link
Author

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

@Haroenv
Copy link
Collaborator

Haroenv commented Feb 14, 2019

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

@AWolf81
Copy link
Collaborator

AWolf81 commented Feb 14, 2019

@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.

@melanieseltzer melanieseltzer added the bug Something isn't working label Feb 15, 2019
@AWolf81
Copy link
Collaborator

AWolf81 commented Feb 16, 2019

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
Copy link
Author

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

@superhawk610
Copy link
Collaborator

Fixed in #367.

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

No branches or pull requests

5 participants