MathNodes / meile-gui

Meile dVPN GUI for Linux, OS X, and Windows - Powered by the Sentinel Network
https://meile.app
GNU General Public License v3.0
35 stars 6 forks source link

[BUG] Connection switch turns off if user cancels on disconnect #51

Open MathNodes opened 10 months ago

MathNodes commented 10 months ago

Sounds like a relatively easy fix, but there are some intricacies involved in this.

The heart of the bug is in connect_to_node() in https://github.com/MathNodes/meile-gui/blob/ffb085ef452e167eb2a1f822f34ddb602b674dc3/src/ui/widgets.py#L660

and disconnect_from_node() in https://github.com/MathNodes/meile-gui/blob/ffb085ef452e167eb2a1f822f34ddb602b674dc3/src/ui/screens.py#L563

The idea would be to check the returncode of the calling disconnect() backend function, and if not 0 or the appropriate value for each platform, then the disconnect_from_node() function should return False and connect_to_node() should handle this False value and turn the switch back to active so the user can disconnect from it at a later time.

I'm offering a bounty for this so I don't spend time testing methods that may or may not work and need to spend time on other things.

HammadRafique29 commented 10 months ago

Hi @MathNodes I want to work on this issue.

HammadRafique29 commented 10 months ago

@MathNodes

What i have understand is:

Detailed Info:

  1. First Condition [DISCONNECTING]: return True if node terminated return False if node failed to terminate

  2. Second Condition [CONNECTED]: return None as alredy CONNECTED

  3. Third Condition [CONNECTING]: connect to new location

freQniK commented 10 months ago

Thanks for tackling this. I'll have a look at it Monday as I'm busy this weekend and afk.

If it suffices we will pay the bounty. I will personally test it on Monday.

Have a good weekend!

freQniK commented 10 months ago

This isn't going to work. There is a UI element that needs changing. Namely self.ids.node_switch in widgets.py which has an attribute active. It should be set back to True so that the UI element stays active. The issue I've been having in my testing is that when I set self.ids.node_switch.active = True is that it keeps trying to reconnect to the node because it triggers on_active within the meile.kv. Basically in an infinite loop. Not sure how I can set the switch to be back on without it trying to connect. Maybe check if mw.CONNECTED is still True in which case don't try to reconnect.

HammadRafique29 commented 10 months ago

@freQniK Sure, i will figure this out

HammadRafique29 commented 10 months ago

This isn't going to work. There is a UI element that needs changing. Namely self.ids.node_switch in widgets.py which has an attribute active. It should be set back to True so that the UI element stays active. The issue I've been having in my testing is that when I set self.ids.node_switch.active = True is that it keeps trying to reconnect to the node because it triggers on_active within the meile.kv. Basically in an infinite loop. Not sure how I can set the switch to be back on without it trying to connect. Maybe check if mw.CONNECTED is still True in which case don't try to reconnect.

Inifinite loop, means that the switch button is calling itself, repeatedly.

freQniK commented 10 months ago

Upping the bounty to 0.2 XMR

HammadRafique29 commented 10 months ago

Upping the bounty to 0.2 XMR

@freQniK Will you provide a little video of running codde in your system, it will help me to find the bug and understand the core logic

freQniK commented 10 months ago

This isn't going to work. There is a UI element that needs changing. Namely self.ids.node_switch in widgets.py which has an attribute active. It should be set back to True so that the UI element stays active. The issue I've been having in my testing is that when I set self.ids.node_switch.active = True is that it keeps trying to reconnect to the node because it triggers on_active within the meile.kv. Basically in an infinite loop. Not sure how I can set the switch to be back on without it trying to connect. Maybe check if mw.CONNECTED is still True in which case don't try to reconnect.

Inifinite loop, means that the switch button is calling itself, repeatedly.

Yeah. Basically when the user clicks disconnect the switch goes to off. But if the user click disconnect and cancels the sudo prompt then the switch stays off but they are still connected. Handling the cancel request and setting self.ids.node_switch.active = True calls connect_to_node() again. So there should be a temp variable that causes the routine to do nothing if the user already cancelled thereby leaving the switch to the on position.

freQniK commented 10 months ago

Upping the bounty to 0.2 XMR

@freQniK Will you provide a little video of running codde in your system, it will help me to find the bug and understand the core logic

Sure. I can do that tomorrow

HammadRafique29 commented 10 months ago

@freQniK I have fixed the major bug, according to my logic, if still there is problem let me know (Pull Request)

HammadRafique29 commented 10 months ago

@freQniK @MathNodes

freQniK commented 10 months ago

This is not going to work either. call_ip_get() is called once a connection is established so you dont want to fool around with changing the switch in that routine. Not to mention your logic is backwards as once it had successfully connected you turn the switch off so the user has no way to disconnect.

I can't spend any more time on this and have decided to scrap the switch in favor of a global Connect/Disconnect button that will roll out in v1.8.0.

I will however pay half the bounty to you (0.1 XMR) for your time spent reviewing the code and your attempts to resolve the issue even though they were not successful.

Please drop a Monero address in the comments and I will send the coins. Thank you for your time and please check back as we will be posting many more Monero bounties in the future.

HammadRafique29 commented 10 months ago

8BGZnRN3NF4UzLHz13Ae3BRzGC9JbDxznF8ERD8JpW7zQZqhvHCGRRxANyBApnsBpwKJqRwZpbWnW4jtX2cXCs835rSDnib

freQniK commented 10 months ago

Screenshot_20230914-110855.png