diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/features/session/ui/SessionViewModel.kt b/kotlin/android/app/src/main/java/dev/firezone/android/features/session/ui/SessionViewModel.kt index c2813e334..7e6c3f82d 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/features/session/ui/SessionViewModel.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/features/session/ui/SessionViewModel.kt @@ -58,11 +58,6 @@ internal class SessionViewModel resources = resources, ) } - - override fun onError(error: String): Boolean { - // TODO("Not yet implemented") - return true - } } fun connect(context: Context) { diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt index 976b39cb0..f2640efac 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/TunnelService.kt @@ -100,11 +100,6 @@ class TunnelService : VpnService() { return true } - override fun onError(error: String): Boolean { - Log.d(TAG, "onError: $error") - return true - } - override fun onAddRoute( addr: String, prefix: Int, diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/ConnlibCallback.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/ConnlibCallback.kt index 56199aa7e..5f21ab49f 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/ConnlibCallback.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/ConnlibCallback.kt @@ -24,7 +24,5 @@ interface ConnlibCallback { fun onDisconnect(error: String?): Boolean - fun onError(error: String): Boolean - fun getSystemDefaultResolvers(): Array } diff --git a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/TunnelListener.kt b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/TunnelListener.kt index 1367445aa..c56923f7e 100644 --- a/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/TunnelListener.kt +++ b/kotlin/android/app/src/main/java/dev/firezone/android/tunnel/callback/TunnelListener.kt @@ -8,6 +8,4 @@ interface TunnelListener { fun onTunnelStateUpdate(state: Tunnel.State) fun onResourcesUpdate(resources: List) - - fun onError(error: String): Boolean } diff --git a/rust/connlib/clients/android/src/lib.rs b/rust/connlib/clients/android/src/lib.rs index ac6734a13..db324dc6a 100644 --- a/rust/connlib/clients/android/src/lib.rs +++ b/rust/connlib/clients/android/src/lib.rs @@ -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 { 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 }) diff --git a/rust/connlib/clients/apple/src/lib.rs b/rust/connlib/clients/apple/src/lib.rs index 7bcf91fc3..3c4f134ee 100644 --- a/rust/connlib/clients/apple/src/lib.rs +++ b/rust/connlib/clients/apple/src/lib.rs @@ -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 { 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 }) } diff --git a/rust/connlib/clients/shared/src/control.rs b/rust/connlib/clients/shared/src/control.rs index d6d9a25f6..825b55be3 100644 --- a/rust/connlib/clients/shared/src/control.rs +++ b/rust/connlib/clients/shared/src/control.rs @@ -134,7 +134,7 @@ impl ControlPlane { 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 ControlPlane { .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 ControlPlane { 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 ControlPlane { tunnel.cleanup_connection(resource_id); tracing::error!("Error request connection details: {err}"); - let _ = tunnel.callbacks().on_error(&err); }); } @@ -231,7 +229,6 @@ impl ControlPlane { 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 ControlPlane { 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 ControlPlane { match (reply_error.error, reference) { (ErrorInfo::Offline, Some(reference)) => { let Ok(resource_id) = reference.parse::() 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 diff --git a/rust/connlib/clients/shared/src/lib.rs b/rust/connlib/clients/shared/src/lib.rs index 744536725..12a99838c 100644 --- a/rust/connlib/clients/shared/src/lib.rs +++ b/rust/connlib/clients/shared/src/lib.rs @@ -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; } diff --git a/rust/connlib/shared/src/callbacks.rs b/rust/connlib/shared/src/callbacks.rs index 2bb48406b..cc4d096ea 100644 --- a/rust/connlib/shared/src/callbacks.rs +++ b/rust/connlib/shared/src/callbacks.rs @@ -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>, Self::Error> { Ok(None) diff --git a/rust/connlib/shared/src/callbacks_error_facade.rs b/rust/connlib/shared/src/callbacks_error_facade.rs index 7a5e5c9e0..efb7d99e9 100644 --- a/rust/connlib/shared/src/callbacks_error_facade.rs +++ b/rust/connlib/shared/src/callbacks_error_facade.rs @@ -81,14 +81,6 @@ impl Callbacks for CallbackErrorFacade { 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 { self.0.roll_log_file() } diff --git a/rust/connlib/shared/src/control.rs b/rust/connlib/shared/src/control.rs index 8b3baa0c1..89b2b22a2 100644 --- a/rust/connlib/shared/src/control.rs +++ b/rust/connlib/shared/src/control.rs @@ -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 { diff --git a/rust/connlib/tunnel/src/control_protocol.rs b/rust/connlib/tunnel/src/control_protocol.rs index 0181ecdb5..e547ed50d 100644 --- a/rust/connlib/tunnel/src/control_protocol.rs +++ b/rust/connlib/tunnel/src/control_protocol.rs @@ -154,7 +154,6 @@ fn insert_peers( fn start_handlers( tunnel: Arc>, device: Arc>, - callbacks: impl Callbacks + 'static, peer: Arc>, ice: Arc, peer_receiver: tokio::sync::mpsc::Receiver, @@ -180,12 +179,7 @@ fn start_handlers( 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)); } }); diff --git a/rust/connlib/tunnel/src/control_protocol/client.rs b/rust/connlib/tunnel/src/control_protocol/client.rs index 8b5204723..fdda4f2e7 100644 --- a/rust/connlib/tunnel/src/control_protocol/client.rs +++ b/rust/connlib/tunnel/src/control_protocol/client.rs @@ -171,7 +171,6 @@ where start_handlers( Arc::clone(self), Arc::clone(&self.device), - self.callbacks.clone(), peer.clone(), ice, peer_receiver, diff --git a/rust/connlib/tunnel/src/control_protocol/gateway.rs b/rust/connlib/tunnel/src/control_protocol/gateway.rs index 3ba30886f..b5de9744b 100644 --- a/rust/connlib/tunnel/src/control_protocol/gateway.rs +++ b/rust/connlib/tunnel/src/control_protocol/gateway.rs @@ -245,7 +245,6 @@ where start_handlers( Arc::clone(self), Arc::clone(&self.device), - self.callbacks.clone(), peer.clone(), ice, peer_receiver, diff --git a/rust/connlib/tunnel/src/lib.rs b/rust/connlib/tunnel/src/lib.rs index 708481ee9..97925b3f2 100644 --- a/rust/connlib/tunnel/src/lib.rs +++ b/rust/connlib/tunnel/src/lib.rs @@ -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); } } diff --git a/rust/connlib/tunnel/src/peer.rs b/rust/connlib/tunnel/src/peer.rs index ce64493af..3c0ff7e59 100644 --- a/rust/connlib/tunnel/src/peer.rs +++ b/rust/connlib/tunnel/src/peer.rs @@ -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); } diff --git a/rust/connlib/tunnel/src/peer_handler.rs b/rust/connlib/tunnel/src/peer_handler.rs index 2fffda9cb..cad000065 100644 --- a/rust/connlib/tunnel/src/peer_handler.rs +++ b/rust/connlib/tunnel/src/peer_handler.rs @@ -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( device: Arc>, - callbacks: impl Callbacks + 'static, peer: Arc>, channel: Arc, ) where @@ -27,7 +25,7 @@ pub(crate) async fn start_peer_handler( 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( } async fn peer_handler( - callbacks: &impl Callbacks, peer: &Arc>, channel: Arc, 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); } } } diff --git a/rust/linux-client/src/main.rs b/rust/linux-client/src/main.rs index e5057b715..158d76527 100644 --- a/rust/linux-client/src/main.rs +++ b/rust/linux-client/src/main.rs @@ -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 }) } diff --git a/rust/windows-client/src-tauri/src/client/gui.rs b/rust/windows-client/src-tauri/src/client/gui.rs index 65d4dbe01..f2af7fa1b 100755 --- a/rust/windows-client/src-tauri/src/client/gui.rs +++ b/rust/windows-client/src-tauri/src/client/gui.rs @@ -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 { 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 }) diff --git a/swift/apple/FirezoneNetworkExtension/Adapter.swift b/swift/apple/FirezoneNetworkExtension/Adapter.swift index e775385a0..1697761b6 100644 --- a/swift/apple/FirezoneNetworkExtension/Adapter.swift +++ b/swift/apple/FirezoneNetworkExtension/Adapter.swift @@ -544,8 +544,4 @@ extension Adapter: CallbackHandlerDelegate { self.logger.info("getSystemDefaultResolvers: \(resolvers)") return resolvers } - - public func onError(error: String) { - self.logger.error("Internal connlib error: \(error, privacy: .public)") - } } diff --git a/swift/apple/FirezoneNetworkExtension/CallbackHandler.swift b/swift/apple/FirezoneNetworkExtension/CallbackHandler.swift index d8e1902f3..f2ee00915 100644 --- a/swift/apple/FirezoneNetworkExtension/CallbackHandler.swift +++ b/swift/apple/FirezoneNetworkExtension/CallbackHandler.swift @@ -27,7 +27,6 @@ public protocol CallbackHandlerDelegate: AnyObject { func onUpdateResources(resourceList: String) func onDisconnect(error: String?) func getSystemDefaultResolvers() -> [String] - func onError(error: String) } public class CallbackHandler { @@ -92,9 +91,4 @@ public class CallbackHandler { return "[]".intoRustString() } } - - func onError(error: RustString) { - logger.log("CallbackHandler.onError: \(error.toString(), privacy: .public)") - delegate?.onError(error: error.toString()) - } }