fether
fether copied to clipboard
Exiting still requires CTRL+C
- When run with
yarn electronand then once it has loaded exit by right clicking the icon in the Dock and choosing Quit, in the terminal it fully exits and shows the prompt (i.e.~/code/src/paritytech/fether - [master] $). But when run with "yarn start" instead of exiting the same way it doesn't return to the prompt, it just hangs and displays the following and it's necessary to press CTRL+C to kill it
[start-electron] ┏ Electron -------------------
[start-electron]
[start-electron] [2019-01-08T02:38:46.817Z] INFO (@parity/electron:main/62633 on lss-MacBook-Pro.local): Another instance of parity is already running on http://127.0.0.1:8546, skip running local instance.
[start-electron]
[start-electron] ┗ ----------------------------
[start-react ] Compiled successfully!
[start-react ]
[start-react ] You can now view fether-react in the browser.
[start-react ]
[start-react ] Local: http://localhost:3000/
[start-react ] On Your Network: http://192.168.43.40:3000/
[start-react ]
[start-react ] Note that the development build is not optimized.
[start-react ] To create a production build, use npm run build.
[start-react ]
[start-react ] Compiling...
[start-react ] Compiled successfully!
-
When I run with
yarn electronand then once it has loaded exit by going to Electron menu File > Quit, in the terminal it fully exits and shows the prompt (i.e.~/code/src/paritytech/fether - [master] $). But when run with "yarn start" instead of exiting the same way it doesn't return to the prompt, it just hangs and displays similar to previous message and it's necessary to press CTRL+C to kill it -
When run with
yarn electronoryarn startand then once it has loaded exit by pressing CTRL+C in terminal it fully exits and shows the prompt (i.e.~/code/src/paritytech/fether - [master] $).
Potential solutions:
- Option 1 - Remove all event listeners before quiting. Check if there are still event listeners hanging and remove them https://github.com/electron/electron/issues/9862#issuecomment-311074928
i.e. something like
app.on('something', () => {
const listenerNames = ['did-this', 'did-that'];
listenerNames.forEach((name, index) => {
app.removeAllListeners(name);
});
pino.info('Remaining listeners: ',
events.EventEmitter.listenerCount(app, 'event'));
});
- Option 2 - pkill library will makes it fully quit, but doesn't exit gracefully:
Try by adding dependency
"pkill": "^2.0.0",And updating:
app.on('quit', () => {
killParity();
pkill.full('paritytech/fether', function (err, validPid) {
console.log('err', err);
console.log('validPid', validPid); // if matched any processes
});
pino.info('Exited Fether with PKill so no need to press CTRL+C to return to terminal prompt');
});
- Option 3: node-powershell library or custom script...
- Option 4: No action, assume that the user will just press CTRL+C if we show a pino log like 'Exited Fether. Press CTRL+C'
Do you know it this happens with the packaged version (.dmg, .exe, .deb)? if not then it's not a big deal.
Using .dmg Fether v0.1.2 on macOS Mojave 10.14. I tested it by running ps aux | grep "Fether" after launching Fether to view its running processes. Then if I exit by either exit by right clicking the icon in the Dock and choosing Quit or exit by going to Electron menu File > Quit I found that when I ran ps aux | grep "Fether" again afterwards the process was no longer listed.
Is there are script we could use to automatically check this happens for new versions that we're about to publish?
We really don't need to make this work with yarn start. yarn start spawns multiple processes:
- one for Electron
- one for create-react-app
- one for building fether-ui
What you're suggesting is that when we kill the process that runs Electron, the 2 others are also killed. I don't think it's needed, and quite hard to achieve. yarn start is only for dev, and killing with CTRL+C is more than enough.
Concerning the binary, we need to make sure that the process is killed when we quit the app. If you can create an e2e test for that, it'll be great. If not, we'll just manually test it, like we do right now.