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

fix obvious errors when NetworkManager is missing #547

Closed
wants to merge 3 commits into from

Conversation

fogti
Copy link

@fogti fogti commented Sep 3, 2023

this fixes all the "startup time" errors encountered when trying to use this without NetworkManager

it doesn't really work yet tho, e.g.:

$ venv/bin/eduvpn-cli interactive
Network Manager not available
keyring not available due to import not available
Traceback (most recent call last):
  File "~/devel/python-eduvpn-client/venv/lib/python3.11/site-packages/eduvpn/nm.py", line 786, in <lambda>
    GLib.idle_add(lambda: action(callback=quit_loop))
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/devel/python-eduvpn-client/venv/lib/python3.11/site-packages/eduvpn/cli.py", line 485, in update_state_callback
    state = self.nm_manager.connection_state
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~/devel/python-eduvpn-client/venv/lib/python3.11/site-packages/eduvpn/nm.py", line 181, in connection_state
    for connection in self.client.get_active_connections()
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'get_active_connections'

@jwijenbergh
Copy link
Collaborator

jwijenbergh commented Sep 4, 2023

Thanks for the PR. Are you trying to use the client without NetworkManager or just looking to improve the error handling when it is not available?

Currently client cannot work without it but it does make sense to add extra error checking when NetworkManager is not available.

Note that for the error you're getting right now you might need to check if NetworkManager is available by adding an if statement here:

https://github.com/eduvpn/python-eduvpn-client/blob/master/eduvpn/cli.py#L655

Would you like to add that check?

@fogti
Copy link
Author

fogti commented Sep 4, 2023

basically, it would be nice to be able to generate plaintext configs without NM. (so that's the end goal, but this fixes just the stuff that directly crashes)

@@ -19,18 +19,16 @@
from eduvpn.utils import run_in_glib_thread
from eduvpn.variants import ApplicationVariant

gi.require_version("NM", "1.0") # noqa: E402
from gi.repository import GLib # type: ignore
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably something like gi.require_version("GLib", "2.0") or such should also be inserted here...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whatever works is fine by me :)

@jwijenbergh
Copy link
Collaborator

basically, it would be nice to be able to generate plaintext configs without NM. (so that's the end goal, but this fixes just the stuff that directly crashes)

I agree that this is useful especially for the CLI, maybe not so much for the GUI?

@jwijenbergh
Copy link
Collaborator

Also if you're right now looking to have a very basic eduVPN cli, you may also use https://github.com/eduvpn/eduvpn-common/blob/main/cmd/cli/main.go

@jwijenbergh
Copy link
Collaborator

jwijenbergh commented Oct 20, 2023

I made some tiny changes to fix #434, can you rebase if you're still working on this?

@nezos
Copy link

nezos commented Dec 12, 2023

@jwijenbergh if I understand correctly the changes will only skip the tests for NM? But how will it be possible with eduvpn-cli to connect without NM? Could you provide a step by step procedure?

Thanks

PS: I installed 4.2 but with existing configurations it complains about network manager - not sure if the present fix was merged in another way

 File "/usr/lib/python3.12/site-packages/eduvpn/nm.py", line 804, in add_connection_callback
    new_con = client.add_connection_finish(result)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
gi.repository.GLib.GError: nm-client-error-quark: NetworkManager is not running (1)

@jwijenbergh
Copy link
Collaborator

@jwijenbergh if I understand correctly the changes will only skip the tests for NM? But how will it be possible with eduvpn-cli to connect without NM? Could you provide a step by step procedure?

Thanks

PS: I installed 4.2 but with existing configurations it complains about network manager - not sure if the present fix was merged in another way

 File "/usr/lib/python3.12/site-packages/eduvpn/nm.py", line 804, in add_connection_callback
    new_con = client.add_connection_finish(result)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
gi.repository.GLib.GError: nm-client-error-quark: NetworkManager is not running (1)

Hi, I think the goal is that it will save the WireGuard/OpenVPN configuration somewhere so that you can manually configure the VPN using wg-quick or the OpenVPN cli. This pr would then make the tool at least run without have the NM tools installed (so that the next step is saving the VPN config somewhere)

@fogti
Copy link
Author

fogti commented Mar 8, 2024

@jwijenbergh

Also if you're right now looking to have a very basic eduVPN cli, you may also use

How does one use that?

@jwijenbergh
Copy link
Collaborator

@jwijenbergh

Also if you're right now looking to have a very basic eduVPN cli, you may also use

How does one use that?

Sorry didn't see this. See the help:

Usage of ./eduvpn-common-cli:
  -get-custom string
    	The url of a custom server to connect to
  -get-institute string
    	The url of an institute to connect to
  -get-secure string
    	Gets secure internet servers

For the URLs, see https://disco.eduvpn.org/v2/

Note that it outputs the OpenVPN/WireGuard config to the terminal output.

@fogti
Copy link
Author

fogti commented Mar 20, 2024

hmmm it's weird...

Opening browser...
2024/03/20 18:27:15 - Go - DEBUG - Getting access token
2024/03/20 18:27:15 - Go - DEBUG - Access token is not expired, returning
2024/03/20 18:27:15 - Go - DEBUG - Getting access token
2024/03/20 18:27:15 - Go - DEBUG - Access token is not expired, returning
panic: runtime/cgo: misuse of an invalid Handle

goroutine 1 [running]:
runtime/cgo.Handle.Value(...)
	/usr/lib/go/src/runtime/cgo/handle.go:124
github.com/eduvpn/eduvpn-common/types/cookie.NewWithContext({0x949398, 0xc000364090})
	/home/fogti/devel/_read-only/eduvpn-common/types/cookie/cookie.go:35 +0x13a
github.com/eduvpn/eduvpn-common/client.(*Client).InvalidProfile(0xc00009eea0, {0x949398?, 0xc000364090?}, 0xc05960?)
	/home/fogti/devel/_read-only/eduvpn-common/client/client.go:73 +0x31
github.com/eduvpn/eduvpn-common/internal/server.(*Servers).ConnectWithCallbacks(0xc00009eeb0, {0x949398, 0xc000364090}, 0xc000364060, 0x0?)
	/home/fogti/devel/_read-only/eduvpn-common/internal/server/servers.go:118 +0x16a
github.com/eduvpn/eduvpn-common/client.(*Client).GetConfig(0xc00009eea0, 0xc000115650, {0x7fff9cd9a0c0?, 0x1b?}, 0x1, 0x0?, 0x0)
	/home/fogti/devel/_read-only/eduvpn-common/client/client.go:434 +0xb05
main.getConfig(0xc00009eea0?, {0x7fff9cd9a0c0?, 0xc0000142b0?}, 0xa?)
	/home/fogti/devel/_read-only/eduvpn-common/cmd/cli/main.go:153 +0x138
main.printConfig({0x7fff9cd9a0c0, 0x1b}, 0xa?)
	/home/fogti/devel/_read-only/eduvpn-common/cmd/cli/main.go:187 +0x24a
main.main()
	/home/fogti/devel/_read-only/eduvpn-common/cmd/cli/main.go:209 +0x145

@jwijenbergh
Copy link
Collaborator

hmmm it's weird...

Opening browser...
2024/03/20 18:27:15 - Go - DEBUG - Getting access token
2024/03/20 18:27:15 - Go - DEBUG - Access token is not expired, returning
2024/03/20 18:27:15 - Go - DEBUG - Getting access token
2024/03/20 18:27:15 - Go - DEBUG - Access token is not expired, returning
panic: runtime/cgo: misuse of an invalid Handle

goroutine 1 [running]:
runtime/cgo.Handle.Value(...)
	/usr/lib/go/src/runtime/cgo/handle.go:124
github.com/eduvpn/eduvpn-common/types/cookie.NewWithContext({0x949398, 0xc000364090})
	/home/fogti/devel/_read-only/eduvpn-common/types/cookie/cookie.go:35 +0x13a
github.com/eduvpn/eduvpn-common/client.(*Client).InvalidProfile(0xc00009eea0, {0x949398?, 0xc000364090?}, 0xc05960?)
	/home/fogti/devel/_read-only/eduvpn-common/client/client.go:73 +0x31
github.com/eduvpn/eduvpn-common/internal/server.(*Servers).ConnectWithCallbacks(0xc00009eeb0, {0x949398, 0xc000364090}, 0xc000364060, 0x0?)
	/home/fogti/devel/_read-only/eduvpn-common/internal/server/servers.go:118 +0x16a
github.com/eduvpn/eduvpn-common/client.(*Client).GetConfig(0xc00009eea0, 0xc000115650, {0x7fff9cd9a0c0?, 0x1b?}, 0x1, 0x0?, 0x0)
	/home/fogti/devel/_read-only/eduvpn-common/client/client.go:434 +0xb05
main.getConfig(0xc00009eea0?, {0x7fff9cd9a0c0?, 0xc0000142b0?}, 0xa?)
	/home/fogti/devel/_read-only/eduvpn-common/cmd/cli/main.go:153 +0x138
main.printConfig({0x7fff9cd9a0c0, 0x1b}, 0xa?)
	/home/fogti/devel/_read-only/eduvpn-common/cmd/cli/main.go:187 +0x24a
main.main()
	/home/fogti/devel/_read-only/eduvpn-common/cmd/cli/main.go:209 +0x145

Could you try the latest stable release? git checkout 1.2.0

@fogti
Copy link
Author

fogti commented Mar 20, 2024

stable release works.

@jwijenbergh
Copy link
Collaborator

jwijenbergh commented Mar 20, 2024

stable release works.

Thanks I am aware of the dev issue. Will sort that before the new stable release

@jwijenbergh
Copy link
Collaborator

Hi, do you still need this @fogti. I think I want to implement non-networkmanager support by adding a flag to dump the config file instead.

@fogti
Copy link
Author

fogti commented Jul 17, 2024

as long as that part works without having network manager installed, I'm fine with it.

@jwijenbergh
Copy link
Collaborator

as long as that part works without having network manager installed, I'm fine with it.

Thanks, is it okay if I close this and work on this later?

@fogti fogti closed this Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants