fix(rust/gui-client): when the Client starts with a token but no Internet, wait for Internet and then connect #6414
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
ReactorScram
changed the base branch from
main
to
fix/gui-connect-without-internet
| controller.ipc_client.set_dns(resolvers).await?; | ||
| }, | ||
| Status::WaitingForNetwork { token } => { | ||
| tracing::debug!("New DNS resolvers, retrying Portal connection..."); |
There was a problem hiding this comment.
Does this effectively mean we trigger on DNS changes? I suppose on Windows that's pretty reliable, but what's the behavior on Linux?
There was a problem hiding this comment.
It should be DNS changes and network changes on both OSes
- Linux DNS via D-Bus
- Linux network via D-Bus
- Windows DNS via registry listener
- Windows network via complicated COM callback https://github.com/firezone/firezone/blob/4ec78f70c220f564e143ed4667543cd58a2baa6b/rust/bin-shared/src/network_changes/windows.rs#L122thing
There was a problem hiding this comment.
The only alternative I could think of was polling which didn't sound pleasant
Co-authored-by: Jamil <jamilbk@users.noreply.github.com> Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
thomaseizinger
left a comment
thomaseizinger
left a comment
There was a problem hiding this comment.
I left some ideas on how we can potentially simplify the state machine. Concept ACK though
| /// Firezone is ready to use. | ||
| TunnelReady { resources: Vec<ResourceDescription> }, | ||
| /// Firezone is signing in to the Portal. | ||
| WaitingForPortal { |
There was a problem hiding this comment.
What do we gain from splitting Connecting into the two Waiting states? connlib will internally reconnect to the portal if it fails whilst we have the tunnel.
Currently, the fact that we resolve the IPs on startup is somewhat of an implementation detail and that may change again. For example, if we end up doing something like #6427, we may resolve them dynamically on startup and thus, creating phoenix-channel will become infallible again.
I would suggest to not let this implementation detail escape the lower layers and just treat the entire time as Connecting.
There was a problem hiding this comment.
It doesn't affect the behavior, but it's nice to see "Connecting to Portal" in the menu if we're trying to debug something
There was a problem hiding this comment.
If it is just for debugging, could we also achieve the same with logs? My recommendation would be to not treat the initial boot-up of connlib as two phases. Where it failed in its boot-process shouldn't really matter to the clients.
The above state machine kind of suggests that we would be also showing this when we are internally re-connecting to the portal but we don't. That is completely transparent to the client (until we do #5557).
Closes #6389
I added a retry button since the network change detection is flaky inside Parallels. On bare metal Windows it works fine.