github-personal/firezone
1
0
Fork 0
mirror of https://github.com/outbackdingo/firezone.git synced 2026-01-27 10:18:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(connlib): Remove unused on_error callback (#3162)

Browse Source 
Fixes #3161 
Fixes #2867
This commit is contained in:
Jamil
2024-01-11 04:42:41 -08:00
committed by GitHub
parent e5af828982
commit 4f37bfab93
21 changed files with 16 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
rust/connlib/clients/android/src/lib.rs
View File

@@ -269,28 +269,9 @@ impl Callbacks for CallbackHandler {
})
}
fn on_error(&self, error: &Error) -> Result<(), Self::Error> {
self.env(|mut env| {
let error = env.new_string(error.to_string()).map_err(|source| {
CallbackError::NewStringFailed {
name: "error",
source,
}
})?;
call_method(
&mut env,
&self.callback_handler,
"onError",
"(Ljava/lang/String;)Z",
&[JValue::from(&error)],
)
})
}
fn roll_log_file(&self) -> Option<PathBuf> {
self.handle.roll_to_new_file().unwrap_or_else(|e| {
tracing::debug!("Failed to roll over to new file: {e}");
let _ = self.on_error(&Error::LogFileRollError(e));
None
})

12
rust/connlib/clients/apple/src/lib.rs
View File

@@ -62,9 +62,6 @@ mod ffi {
#[swift_bridge(swift_name = "getSystemDefaultResolvers")]
fn get_system_default_resolvers(&self) -> String;
#[swift_bridge(swift_name = "onError")]
fn on_error(&self, error: String);
}
}
@@ -146,16 +143,9 @@ impl Callbacks for CallbackHandler {
Ok(Some(resolvers))
}
fn on_error(&self, error: &Error) -> Result<(), Self::Error> {
self.inner.on_error(error.to_string());
Ok(())
}
fn roll_log_file(&self) -> Option<PathBuf> {
self.handle.roll_to_new_file().unwrap_or_else(|e| {
tracing::debug!("Failed to roll over to new file: {e}");
let _ = self.on_error(&Error::LogFileRollError(e));
tracing::error!("Failed to roll over to new log file: {e}");
None
})
}

20
rust/connlib/clients/shared/src/control.rs
View File

@@ -134,7 +134,7 @@ impl<CB: Callbacks + 'static> ControlPlane<CB> {
gateway_payload.domain_response,
gateway_public_key.0.into(),
) {
let _ = self.tunnel.callbacks().on_error(&e);
tracing::debug!(error = ?e, "Error accepting connection: {e:#?}");
}
}
GatewayResponse::ResourceAccepted(gateway_payload) => {
@@ -142,7 +142,7 @@ impl<CB: Callbacks + 'static> ControlPlane<CB> {
.tunnel
.received_domain_parameters(resource_id, gateway_payload.domain_response)
{
let _ = self.tunnel.callbacks().on_error(&e);
tracing::debug!(error = ?e, "Error accepting resource: {e:#?}");
}
}
}
@@ -152,7 +152,6 @@ impl<CB: Callbacks + 'static> ControlPlane<CB> {
pub fn add_resource(&self, resource_description: ResourceDescription) {
if let Err(e) = self.tunnel.add_resource(resource_description) {
tracing::error!(message = "Can't add resource", error = ?e);
let _ = self.tunnel.callbacks().on_error(&e);
}
}
@@ -217,7 +216,6 @@ impl<CB: Callbacks + 'static> ControlPlane<CB> {
tunnel.cleanup_connection(resource_id);
tracing::error!("Error request connection details: {err}");
let _ = tunnel.callbacks().on_error(&err);
});
}
@@ -231,7 +229,6 @@ impl<CB: Callbacks + 'static> ControlPlane<CB> {
for candidate in candidates {
if let Err(e) = self.tunnel.add_ice_candidate(gateway_id, candidate).await {
tracing::error!(err = ?e,"add_ice_candidate");
let _ = self.tunnel.callbacks().on_error(&e);
}
}
}
@@ -259,7 +256,10 @@ impl<CB: Callbacks + 'static> ControlPlane<CB> {
tokio::spawn(async move {
if let Err(e) = upload(path.clone(), url).await {
tracing::warn!("Failed to upload log file: {e}");
tracing::warn!(
"Failed to upload log file at path {path_display}: {e}. Not retrying.",
path_display = path.display()
);
}
});
}
@@ -278,13 +278,7 @@ impl<CB: Callbacks + 'static> ControlPlane<CB> {
match (reply_error.error, reference) {
(ErrorInfo::Offline, Some(reference)) => {
let Ok(resource_id) = reference.parse::<ResourceId>() else {
tracing::error!(
"An offline error came back with a reference to a non-valid resource id"
);
let _ = self
.tunnel
.callbacks()
.on_error(&Error::ControlProtocolError);
tracing::warn!("The portal responded with an Offline error. Is the Resource associated with any online Gateways? Reference: {reference}");
return Ok(());
};
// TODO: Rate limit the number of attempts of getting the relays before just trying a local network connection

7
rust/connlib/clients/shared/src/lib.rs
View File

@@ -212,17 +212,16 @@ where
let result = connection.start(vec!["client".to_owned()], || exponential_backoff.reset()).await;
tracing::warn!("Disconnected from the portal");
if let Err(e) = &result {
tracing::warn!(error = ?e, "Portal connection error");
tracing::error!(error = ?e, "Connection to portal failed. Starting retries with exponential backoff timer.");
if e.is_http_client_error() {
fatal_error!(result, runtime_stopper, &callbacks);
}
}
if let Some(t) = exponential_backoff.next_backoff() {
tracing::warn!("Error connecting to portal, retrying in {} seconds", t.as_secs());
let _ = callbacks.on_error(&result.err().unwrap_or(Error::PortalConnectionError(tokio_tungstenite::tungstenite::Error::ConnectionClosed)));
tracing::error!("Connection to portal failed. Retrying connection to portal in {} seconds", t.as_secs());
tokio::time::sleep(t).await;
} else {
tracing::error!("Connection to portal failed, giving up");
tracing::error!("Connection to portal failed, giving up!");
Self::disconnect_inner(runtime_stopper, &callbacks, None);
break;
}

6
rust/connlib/shared/src/callbacks.rs
View File

@@ -64,12 +64,6 @@ pub trait Callbacks: Clone + Send + Sync {
std::process::exit(0);
}
/// Called when there's a recoverable error.
fn on_error(&self, error: &crate::Error) -> Result<(), Self::Error> {
tracing::warn!(error = ?error);
Ok(())
}
/// Returns the system's default resolver
fn get_system_default_resolvers(&self) -> Result<Option<Vec<IpAddr>>, Self::Error> {
Ok(None)

8
rust/connlib/shared/src/callbacks_error_facade.rs
View File

@@ -81,14 +81,6 @@ impl<CB: Callbacks> Callbacks for CallbackErrorFacade<CB> {
Ok(())
}
fn on_error(&self, error: &Error) -> Result<()> {
if let Err(err) = self.0.on_error(error) {
tracing::error!(?err, "`on_error` failed");
}
// There's nothing we really want to do if `on_error` fails.
Ok(())
}
fn roll_log_file(&self) -> Option<PathBuf> {
self.0.roll_log_file()
}

2
rust/connlib/shared/src/control.rs
View File

@@ -227,7 +227,7 @@ where
ReplyMessage::PhxReply(phx_reply) => match phx_reply {
// TODO: Here we should pass error info to a subscriber
PhxReply::Error(info) => {
tracing::warn!("Portal error: {info:?}");
tracing::debug!("Portal error: {info:?}");
handler(Err(ErrorReply { error: info }), m.reference, m.topic).await
}
PhxReply::Ok(reply) => match reply {

8
rust/connlib/tunnel/src/control_protocol.rs
View File

@@ -154,7 +154,6 @@ fn insert_peers<TId: Copy, TTransform>(
fn start_handlers<TId, TTransform, TRoleState>(
tunnel: Arc<Tunnel<impl Callbacks + 'static, TRoleState>>,
device: Arc<ArcSwapOption<Device>>,
callbacks: impl Callbacks + 'static,
peer: Arc<Peer<TId, TTransform>>,
ice: Arc<RTCIceTransport>,
peer_receiver: tokio::sync::mpsc::Receiver<Bytes>,
@@ -180,12 +179,7 @@ fn start_handlers<TId, TTransform, TRoleState>(
let Some(ep) = ice.new_endpoint(Box::new(|_| true)).await else {
return;
};
tokio::spawn(peer_handler::start_peer_handler(
device,
callbacks,
peer,
ep.clone(),
));
tokio::spawn(peer_handler::start_peer_handler(device, peer, ep.clone()));
tokio::spawn(peer_handler::handle_packet(ep, peer_receiver));
}
});

1
rust/connlib/tunnel/src/control_protocol/client.rs
View File

@@ -171,7 +171,6 @@ where
start_handlers(
Arc::clone(self),
Arc::clone(&self.device),
self.callbacks.clone(),
peer.clone(),
ice,
peer_receiver,

1
rust/connlib/tunnel/src/control_protocol/gateway.rs
View File

@@ -245,7 +245,6 @@ where
start_handlers(
Arc::clone(self),
Arc::clone(&self.device),
self.callbacks.clone(),
peer.clone(),
ice,
peer_receiver,

3
rust/connlib/tunnel/src/lib.rs
View File

@@ -328,11 +328,9 @@ where
if let Some(conn) = self.peer_connections.lock().remove(&conn_id) {
tokio::spawn({
let callbacks = self.callbacks.clone();
async move {
if let Err(e) = conn.stop().await {
tracing::warn!(%conn_id, error = ?e, "Can't close peer");
let _ = callbacks.on_error(&e.into());
}
}
});
@@ -364,7 +362,6 @@ where
if let Err(e) = device.refresh_mtu() {
tracing::error!(error = ?e, "refresh_mtu");
let _ = self.callbacks.on_error(&e);
}
}

2
rust/connlib/tunnel/src/peer.rs
View File

@@ -173,7 +173,7 @@ where
let (packet, addr) = self.transform.packet_untransform(&addr, packet)?;
if !self.is_allowed(addr) {
tracing::warn!("packet not allowed: {addr}");
tracing::debug!("A packet was seen from the tunnel with a destination address we didn't expect: {addr}");
return Ok(None);
}

6
rust/connlib/tunnel/src/peer_handler.rs
View File

@@ -4,7 +4,6 @@ use std::time::Duration;
use arc_swap::ArcSwapOption;
use bytes::Bytes;
use connlib_shared::Callbacks;
use webrtc::mux::endpoint::Endpoint;
use webrtc::util::Conn;
@@ -14,7 +13,6 @@ use crate::{peer::Peer, MAX_UDP_SIZE};
pub(crate) async fn start_peer_handler<TId, TTransform>(
device: Arc<ArcSwapOption<Device>>,
callbacks: impl Callbacks + 'static,
peer: Arc<Peer<TId, TTransform>>,
channel: Arc<Endpoint>,
) where
@@ -27,7 +25,7 @@ pub(crate) async fn start_peer_handler<TId, TTransform>(
tokio::time::sleep(Duration::from_millis(100)).await;
continue;
};
let result = peer_handler(&callbacks, &peer, channel.clone(), &device).await;
let result = peer_handler(&peer, channel.clone(), &device).await;
if matches!(result, Err(ref err) if err.raw_os_error() == Some(9)) {
tracing::warn!("bad_file_descriptor");
@@ -45,7 +43,6 @@ pub(crate) async fn start_peer_handler<TId, TTransform>(
}
async fn peer_handler<TId, TTransform>(
callbacks: &impl Callbacks,
peer: &Arc<Peer<TId, TTransform>>,
channel: Arc<Endpoint>,
device: &Device,
@@ -83,7 +80,6 @@ where
Ok(None) => {}
Err(other) => {
tracing::error!(error = ?other, "failed to handle peer packet");
let _ = callbacks.on_error(&other);
}
}
}

4
rust/linux-client/src/main.rs
View File

@@ -1,6 +1,6 @@
use anyhow::Result;
use clap::Parser;
use connlib_client_shared::{file_logger, Callbacks, Error, Session};
use connlib_client_shared::{file_logger, Callbacks, Session};
use firezone_cli_utils::{block_on_ctrl_c, setup_global_subscriber, CommonArgs};
use secrecy::SecretString;
use std::path::PathBuf;
@@ -43,8 +43,6 @@ impl Callbacks for CallbackHandler {
.roll_to_new_file()
.unwrap_or_else(|e| {
tracing::debug!("Failed to roll over to new file: {e}");
let _ = self.on_error(&Error::LogFileRollError(e));
None
})
}

6
rust/windows-client/src-tauri/src/client/gui.rs
View File

@@ -250,11 +250,6 @@ impl connlib_client_shared::Callbacks for CallbackHandler {
Ok(())
}
fn on_error(&self, error: &connlib_client_shared::Error) -> Result<(), Self::Error> {
tracing::error!("on_error not implemented. Error: {error:?}");
Ok(())
}
fn on_tunnel_ready(&self) -> Result<(), Self::Error> {
// TODO: implement
tracing::info!("on_tunnel_ready");
@@ -274,7 +269,6 @@ impl connlib_client_shared::Callbacks for CallbackHandler {
fn roll_log_file(&self) -> Option<PathBuf> {
self.logger.roll_to_new_file().unwrap_or_else(|e| {
tracing::debug!("Failed to roll over to new file: {e}");
let _ = self.on_error(&connlib_client_shared::Error::LogFileRollError(e));
None
})