fix(rust/gui-client): throw error when failing to connect to Firezone (#6409)

Refs #6389 

```[tasklist]
- [x] Update changelog
- [x] Update manual test cases
```

This changes the behavior from "fail silently" to "fail loudly" so at
least the user knows something is wrong and they can restart Firezone
after they gain Internet.

<img width="439" alt="image"
src="https://github.com/user-attachments/assets/d5bbac66-9a5f-40a6-8b4c-71d8ab8abd6d">

<img width="554" alt="image"
src="https://github.com/user-attachments/assets/bcee1f87-bd29-4a44-b41f-a01217e3248e">
This commit is contained in:
Reactor Scram
2024-08-23 10:37:57 -05:00
committed by GitHub
parent 2e54ae19c9
commit 482ded889e
8 changed files with 134 additions and 64 deletions

View File

@@ -19,7 +19,7 @@ serde = { version = "1.0", default-features = false, features = ["std", "derive"
socket-factory = { workspace = true }
thiserror = "1.0.63"
time = { version = "0.3.36", features = ["formatting"] }
tokio = { workspace = true, features = ["sync"] }
tokio = { workspace = true, features = ["rt", "sync"] }
tracing = { workspace = true, features = ["std", "attributes"] }
tun = { workspace = true }
url = { version = "2.4.1", features = ["serde"] }

View File

@@ -146,3 +146,7 @@ Given the client is signed in, when you ping...
# Network changes
Moved to [`network_roaming.md`](network_roaming.md)
# No Internet
Given Firezone is signed in and not running, when you disconnect from the Internet and start Firezone, then Firezone should show an error and quit.

View File

@@ -12,7 +12,7 @@ use anyhow::{anyhow, bail, Context, Result};
use firezone_bin_shared::{new_dns_notifier, new_network_notifier};
use firezone_headless_client::{
IpcClientMsg::{self, SetDisabledResources},
IpcServerMsg, LogFilterReloader,
IpcServerMsg, IpcServiceError, LogFilterReloader,
};
use secrecy::{ExposeSecret, SecretString};
use std::{
@@ -509,8 +509,6 @@ impl Controller {
// Change the status after we begin connecting
self.status = Status::Connecting { start_instant };
self.refresh_system_tray_menu()?;
ran_before::set().await?;
Ok(())
}
@@ -685,6 +683,16 @@ impl Controller {
})?;
Ok(())
}
IpcServerMsg::ConnectResult(result) => match result {
Ok(()) => Ok(ran_before::set().await?),
Err(IpcServiceError::PortalConnection(s)) => Err(Error::PortalConnection(s)),
Err(error) => {
tracing::error!(?error, "Failed to connect to Firezone");
Err(Error::Other(anyhow!(
"Failed to connect to Firezone for non-Portal-related reason"
)))
}
},
IpcServerMsg::OnDisconnect {
error_msg,
is_authentication_error,

View File

@@ -20,6 +20,8 @@ pub(crate) enum Error {
IpcRead,
#[error("IPC service terminating")]
IpcServiceTerminating,
#[error("Failed to connect to portal")]
PortalConnection(String),
#[error("UserNotInFirezoneGroup")]
UserNotInFirezoneGroup,
#[error("WebViewNotInstalled")]
@@ -51,6 +53,10 @@ pub(crate) fn show_error_dialog(error: &Error) -> Result<()> {
Error::IpcRead => "IPC read failure".to_string(),
Error::IpcServiceTerminating => "The Firezone IPC service is terminating. Please restart the GUI Client.".to_string(),
Error::Logging(_) => "Logging error".to_string(),
Error::PortalConnection(error) => {
tracing::error!(?error, "Couldn't connect to the Portal");
"Couldn't connect to the Firezone Portal. Are you connected to the Internet?".to_string()
}
Error::UserNotInFirezoneGroup => format!("You are not a member of the group `{FIREZONE_GROUP}`. Try `sudo usermod -aG {FIREZONE_GROUP} $USER` and then reboot"),
Error::Other(error) => error.to_string(),
};

View File

@@ -5,6 +5,7 @@ use crate::{
use anyhow::{bail, Context as _, Result};
use clap::Parser;
use connlib_client_shared::{keypair, ConnectArgs, LoginUrl, Session};
use connlib_shared::callbacks::ResourceDescription;
use firezone_bin_shared::{
platform::{tcp_socket_factory, udp_socket_factory, DnsControlMethod},
TunDeviceManager, TOKEN_ENV_KEY,
@@ -14,6 +15,8 @@ use futures::{
task::{Context, Poll},
Future as _, SinkExt as _, Stream as _,
};
use secrecy::SecretString;
use serde::{Deserialize, Serialize};
use std::{collections::BTreeSet, net::IpAddr, path::PathBuf, pin::pin, sync::Arc, time::Duration};
use tokio::{sync::mpsc, task::spawn_blocking, time::Instant};
use tracing::subscriber::set_global_default;
@@ -69,7 +72,7 @@ impl Default for Cmd {
}
}
#[derive(Debug, PartialEq, serde::Deserialize, serde::Serialize)]
#[derive(Debug, PartialEq, Deserialize, Serialize)]
pub enum ClientMsg {
ClearLogs,
Connect { api_url: String, token: String },
@@ -80,6 +83,35 @@ pub enum ClientMsg {
SetDisabledResources(BTreeSet<ResourceId>),
}
/// Messages that end up in the GUI, either forwarded from connlib or from the IPC service.
#[derive(Debug, Deserialize, Serialize)]
pub enum ServerMsg {
/// The IPC service finished clearing its log dir.
ClearedLogs(Result<(), String>),
ConnectResult(Result<(), Error>),
OnDisconnect {
error_msg: String,
is_authentication_error: bool,
},
OnUpdateResources(Vec<ResourceDescription>),
/// The IPC service is terminating, maybe due to a software update
///
/// This is a hint that the Client should exit with a message like,
/// "Firezone is updating, please restart the GUI" instead of an error like,
/// "IPC connection closed".
TerminatingGracefully,
}
// All variants are `String` because almost no error type implements `Serialize`
#[derive(Debug, Deserialize, Serialize)]
pub enum Error {
DeviceId(String),
LoginUrl(String),
PortalConnection(String),
TunnelDevice(String),
UrlParse(String),
}
/// Only called from the GUI Client's build of the IPC service
pub fn run_only_ipc_service() -> Result<()> {
// Docs indicate that `remove_var` should actually be marked unsafe
@@ -383,46 +415,11 @@ impl<'a> Handler<'a> {
}
ClientMsg::Connect { api_url, token } => {
let token = secrecy::SecretString::from(token);
// There isn't an airtight way to implement a "disconnect and reconnect"
// right now because `Session::disconnect` is fire-and-forget:
// <https://github.com/firezone/firezone/blob/663367b6055ced7432866a40a60f9525db13288b/rust/connlib/clients/shared/src/lib.rs#L98-L103>
assert!(self.connlib.is_none());
let device_id =
device_id::get_or_create().context("Failed to get / create device ID")?;
let (private_key, public_key) = keypair();
let url = LoginUrl::client(
Url::parse(&api_url)?,
&token,
device_id.id,
None,
public_key.to_bytes(),
)?;
self.last_connlib_start_instant = Some(Instant::now());
let args = ConnectArgs {
tcp_socket_factory: Arc::new(tcp_socket_factory),
udp_socket_factory: Arc::new(udp_socket_factory),
private_key,
callbacks: self.callback_handler.clone(),
};
let portal = PhoenixChannel::connect(
Secret::new(url),
get_user_agent(None, env!("CARGO_PKG_VERSION")),
"client",
(),
ExponentialBackoffBuilder::default()
.with_max_elapsed_time(Some(Duration::from_secs(60 * 60 * 24 * 30)))
.build(),
Arc::new(tcp_socket_factory),
)?;
let new_session =
Session::connect(args, portal, tokio::runtime::Handle::try_current()?);
let tun = self.tun_device.make_tun()?;
new_session.set_tun(Box::new(tun));
new_session.set_dns(self.dns_controller.system_resolvers());
self.connlib = Some(new_session);
let result = self.connect_to_firezone(&api_url, token);
self.ipc_tx
.send(&IpcServerMsg::ConnectResult(result))
.await
.context("Failed to send `ConnectResult`")?
}
ClientMsg::Disconnect => {
if let Some(connlib) = self.connlib.take() {
@@ -451,6 +448,61 @@ impl<'a> Handler<'a> {
}
Ok(())
}
/// Connects connlib
///
/// Panics if there's no Tokio runtime or if connlib is already connected
///
/// Throws matchable errors for bad URLs, unable to reach the portal, or unable to create the tunnel device
fn connect_to_firezone(&mut self, api_url: &str, token: SecretString) -> Result<(), Error> {
// There isn't an airtight way to implement a "disconnect and reconnect"
// right now because `Session::disconnect` is fire-and-forget:
// <https://github.com/firezone/firezone/blob/663367b6055ced7432866a40a60f9525db13288b/rust/connlib/clients/shared/src/lib.rs#L98-L103>
assert!(self.connlib.is_none());
let device_id = device_id::get_or_create().map_err(|e| Error::DeviceId(e.to_string()))?;
let (private_key, public_key) = keypair();
let url = LoginUrl::client(
Url::parse(api_url).map_err(|e| Error::UrlParse(e.to_string()))?,
&token,
device_id.id,
None,
public_key.to_bytes(),
)
.map_err(|e| Error::LoginUrl(e.to_string()))?;
self.last_connlib_start_instant = Some(Instant::now());
let args = ConnectArgs {
tcp_socket_factory: Arc::new(tcp_socket_factory),
udp_socket_factory: Arc::new(udp_socket_factory),
private_key,
callbacks: self.callback_handler.clone(),
};
// Synchronous DNS resolution here
let portal = PhoenixChannel::connect(
Secret::new(url),
get_user_agent(None, env!("CARGO_PKG_VERSION")),
"client",
(),
ExponentialBackoffBuilder::default()
.with_max_elapsed_time(Some(Duration::from_secs(60 * 60 * 24 * 30)))
.build(),
Arc::new(tcp_socket_factory),
)
.map_err(|e| Error::PortalConnection(e.to_string()))?;
let new_session = Session::connect(args, portal, tokio::runtime::Handle::current());
let tun = self
.tun_device
.make_tun()
.map_err(|e| Error::TunnelDevice(e.to_string()))?;
new_session.set_tun(Box::new(tun));
new_session.set_dns(self.dns_controller.system_resolvers());
self.connlib = Some(new_session);
Ok(())
}
}
/// Starts logging for the production IPC service

View File

@@ -33,7 +33,10 @@ pub mod uptime;
pub use clear_logs::clear_logs;
pub use dns_control::DnsController;
pub use ipc_service::{ipc, run_only_ipc_service, ClientMsg as IpcClientMsg};
pub use ipc_service::{
ipc, run_only_ipc_service, ClientMsg as IpcClientMsg, Error as IpcServiceError,
ServerMsg as IpcServerMsg,
};
use ip_network::{Ipv4Network, Ipv6Network};
@@ -86,24 +89,6 @@ pub enum ConnlibMsg {
},
}
/// Messages that end up in the GUI, either from connlib or from the IPC service.
#[derive(Debug, serde::Deserialize, serde::Serialize)]
pub enum IpcServerMsg {
/// The IPC service finished clearing its log dir.
ClearedLogs(Result<(), String>),
OnDisconnect {
error_msg: String,
is_authentication_error: bool,
},
OnUpdateResources(Vec<callbacks::ResourceDescription>),
/// The IPC service is terminating, maybe due to a software update
///
/// This is a hint that the Client should exit with a message like,
/// "Firezone is updating, please restart the GUI" instead of an error like,
/// "IPC connection closed".
TerminatingGracefully,
}
#[derive(Clone)]
pub struct CallbackHandler {
pub cb_tx: mpsc::Sender<ConnlibMsg>,

View File

@@ -186,6 +186,12 @@ fn main() -> Result<()> {
callbacks,
};
let _guard = rt.enter(); // Constructing `PhoenixChannel` requires a runtime context.
// The Headless Client will bail out here if there's no Internet, because `PhoenixChannel` will try to
// resolve the portal host and fail. This is intentional behavior. The Headless Client should always be running under a manager like `systemd` or Windows' Service Controller,
// so when it fails it will be restarted with backoff. `systemd` can additionally make us wait
// for an Internet connection if it launches us at startup.
// When running interactively, it is useful for the user to see that we can't reach the portal.
let portal = PhoenixChannel::connect(
Secret::new(url),
get_user_agent(None, env!("CARGO_PKG_VERSION")),

View File

@@ -13,6 +13,15 @@ export default function GUI({ title }: { title: string }) {
return (
<Entries href={href} arches={arches} title={title}>
{/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This cannot be done when the issue's PR merges. */}
{/*
<Entry version="1.2.1" date={new Date("")}>
<ul className="list-disc space-y-2 pl-4 mb-4">
<ChangeItem pull="6409">
Shows an error if there's no Internet at startup
</ChangeItem>
</ul>
</Entry>
*/}
<Entry version="1.2.0" date={new Date("2024-08-21")}>
<ul className="list-disc space-y-2 pl-4 mb-4">
<ChangeItem pull="5901">