Show errors creating P2P listeners on startup (#2372)

* do it

* fix accuracy

* `useRef` as god intended

core/src/api/mod.rs

@@ -5,7 +5,6 @@ use crate::{
 		get_hardware_model_name, HardwareModel,
 	},
 	old_job::JobProgressEvent,
-	p2p::{into_listener2, Listener2},
 	Node,
 };
 
@@ -114,7 +113,6 @@ struct NodeState {
 	#[serde(flatten)]
 	config: SanitisedNodeConfig,
 	data_path: String,
-	listeners: Vec<Listener2>,
 	device_model: Option<String>,
 }
 
@@ -150,7 +148,6 @@ pub(crate) fn mount() -> Arc<Router> {
 						.to_str()
 						.expect("Found non-UTF-8 path")
 						.to_string(),
-					listeners: into_listener2(&node.p2p.p2p.listeners()),
 					device_model: Some(device_model),
 				})
 			})

core/src/api/p2p.rs

@@ -3,9 +3,9 @@ use crate::p2p::{operations, ConnectionMethod, DiscoveryMethod, Header, P2PEvent
 use sd_p2p::{PeerConnectionCandidate, RemoteIdentity};
 
 use rspc::{alpha::AlphaRouter, ErrorCode};
-use serde::Deserialize;
+use serde::{Deserialize, Serialize};
 use specta::Type;
-use std::path::PathBuf;
+use std::{path::PathBuf, sync::PoisonError};
 use tokio::io::AsyncWriteExt;
 use uuid::Uuid;
 
@@ -58,6 +58,55 @@ pub(crate) fn mount() -> AlphaRouter<Ctx> {
 		.procedure("state", {
 			R.query(|node, _: ()| async move { Ok(node.p2p.state().await) })
 		})
+		.procedure("listeners", {
+			#[derive(Serialize, Type)]
+			#[serde(tag = "type")]
+			pub enum ListenerState {
+				Listening,
+				Error { error: String },
+				Disabled,
+			}
+
+			#[derive(Serialize, Type)]
+			pub struct Listeners {
+				ipv4: ListenerState,
+				ipv6: ListenerState,
+			}
+
+			R.query(|node, _: ()| async move {
+				let addrs = node
+					.p2p
+					.p2p
+					.listeners()
+					.iter()
+					.map(|l| l.addrs.clone())
+					.flatten()
+					.collect::<Vec<_>>();
+
+				let errors = node
+					.p2p
+					.listener_errors
+					.lock()
+					.unwrap_or_else(PoisonError::into_inner);
+
+				Ok(Listeners {
+					ipv4: match errors.ipv4 {
+						Some(ref err) => ListenerState::Error { error: err.clone() },
+						None => match addrs.iter().any(|f| f.is_ipv4()) {
+							true => ListenerState::Listening,
+							false => ListenerState::Disabled,
+						},
+					},
+					ipv6: match errors.ipv6 {
+						Some(ref err) => ListenerState::Error { error: err.clone() },
+						None => match addrs.iter().any(|f| f.is_ipv6()) {
+							true => ListenerState::Listening,
+							false => ListenerState::Disabled,
+						},
+					},
+				})
+			})
+		})
 		.procedure("debugConnect", {
 			R.mutation(|node, identity: RemoteIdentity| async move {
 				let peer = { node.p2p.p2p.peers().get(&identity).cloned() };

core/src/location/mod.rs

@@ -23,7 +23,7 @@ use sd_sync::*;
 use sd_utils::{
 	db::{maybe_missing, MissingFieldError},
 	error::{FileIOError, NonUtf8PathError},
-	msgpack, uuid_to_bytes,
+	msgpack,
 };
 
 use std::{

core/src/p2p/manager.rs

@@ -14,17 +14,14 @@ use axum::routing::IntoMakeService;
 
 use sd_p2p::{
 	flume::{bounded, Receiver},
-	HookId, Libp2pPeerId, Listener, Mdns, Peer, QuicTransport, RelayServerEntry, RemoteIdentity,
+	HookId, Libp2pPeerId, Mdns, Peer, QuicTransport, RelayServerEntry, RemoteIdentity,
 	UnicastStream, P2P,
 };
 use sd_p2p_tunnel::Tunnel;
-use serde::Serialize;
 use serde_json::json;
-use specta::Type;
 use std::{
-	collections::{HashMap, HashSet},
+	collections::HashMap,
 	convert::Infallible,
-	net::SocketAddr,
 	sync::{atomic::AtomicBool, Arc, Mutex, PoisonError},
 	time::Duration,
 };
@@ -37,6 +34,12 @@ use uuid::Uuid;
 
 use super::{P2PEvents, PeerMetadata};
 
+#[derive(Default)]
+pub struct ListenerErrors {
+	pub ipv4: Option<String>,
+	pub ipv6: Option<String>,
+}
+
 pub struct P2PManager {
 	pub(crate) p2p: Arc<P2P>,
 	mdns: Mutex<Option<Mdns>>,
@@ -48,6 +51,7 @@ pub struct P2PManager {
 	pub(super) spacedrop_cancellations: Arc<Mutex<HashMap<Uuid, Arc<AtomicBool>>>>,
 	pub(crate) node_config: Arc<config::Manager>,
 	pub libraries_hook_id: HookId,
+	pub listener_errors: Mutex<ListenerErrors>,
 }
 
 impl P2PManager {
@@ -75,6 +79,7 @@ impl P2PManager {
 			spacedrop_cancellations: Default::default(),
 			node_config,
 			libraries_hook_id,
+			listener_errors: Default::default(),
 		});
 		this.on_node_config_change().await;
 
@@ -153,6 +158,11 @@ impl P2PManager {
 				.write(|c| c.p2p_ipv4_port = Port::Disabled)
 				.await
 				.ok();
+
+			self.listener_errors
+				.lock()
+				.unwrap_or_else(PoisonError::into_inner)
+				.ipv4 = Some(format!("{err}"));
 		}
 
 		let port = match config.p2p_ipv6_port {
@@ -160,13 +170,18 @@ impl P2PManager {
 			Port::Random => Some(0),
 			Port::Discrete(port) => Some(port),
 		};
-		info!("Setting quic ipv4 listener to: {port:?}");
+		info!("Setting quic ipv6 listener to: {port:?}");
 		if let Err(err) = self.quic.set_ipv6_enabled(port).await {
 			error!("Failed to enabled quic ipv6 listener: {err}");
 			self.node_config
 				.write(|c| c.p2p_ipv6_port = Port::Disabled)
 				.await
 				.ok();
+
+			self.listener_errors
+				.lock()
+				.unwrap_or_else(PoisonError::into_inner)
+				.ipv6 = Some(format!("{err}"));
 		}
 
 		let should_revert = match config.p2p_discovery {
@@ -350,23 +365,6 @@ async fn start(
 	Ok::<_, ()>(())
 }
 
-#[derive(Debug, Serialize, Type)]
-pub struct Listener2 {
-	pub id: String,
-	pub name: &'static str,
-	pub addrs: HashSet<SocketAddr>,
-}
-
-pub fn into_listener2(l: &[Listener]) -> Vec<Listener2> {
-	l.iter()
-		.map(|l| Listener2 {
-			id: format!("{:?}", l.id),
-			name: l.name,
-			addrs: l.addrs.clone(),
-		})
-		.collect()
-}
-
 fn unwrap_infallible<T>(result: Result<T, Infallible>) -> T {
 	match result {
 		Ok(value) => value,

interface/app/index.tsx

@@ -46,12 +46,20 @@ import './style.scss';
 
 import { useZodRouteParams } from '~/hooks';
 
+import { useP2PErrorToast } from './p2p';
+
 // NOTE: all route `Layout`s below should contain
 // the `usePlausiblePageViewMonitor` hook, as early as possible (ideally within the layout itself).
 // the hook should only be included if there's a valid `ClientContext` (so not onboarding)
 
 const LibraryIdParamsSchema = z.object({ libraryId: z.string() });
 
+// Broken out so this always runs after the `Toaster` is merged.
+function P2PErrorToast() {
+	useP2PErrorToast();
+	return null;
+}
+
 export const createRoutes = (platform: Platform, cache: NormalisedCache) =>
 	[
 		{
@@ -68,6 +76,7 @@ export const createRoutes = (platform: Platform, cache: NormalisedCache) =>
 						<Outlet />
 						<Dialogs />
 						<Toaster position="bottom-right" expand={true} offset={18} />
+						<P2PErrorToast />
 					</RootContext.Provider>
 				);
 			},

interface/app/p2p/index.tsx

@@ -1,49 +1,56 @@
-import { useEffect, useState } from 'react';
-import { useBridgeQuery, useFeatureFlag, useP2PEvents, withFeatureFlag } from '@sd/client';
+import { useEffect, useRef, useState } from 'react';
+import { useBridgeQuery } from '@sd/client';
 import { toast } from '@sd/ui';
 
 export function useP2PErrorToast() {
-	// const nodeState = useBridgeQuery(['nodeState']);
-	// const [didShowError, setDidShowError] = useState({
-	// 	ipv4: false,
-	// 	ipv6: false
-	// });
+	const listeners = useBridgeQuery(['p2p.listeners']);
+	const didShowError = useRef(false);
 
-	// // TODO: This can probally be improved in the future. Theorically if you enable -> disable -> then enable and it fails both enables the error won't be shown.
-	// useEffect(() => {
-	// 	const ipv4Error =
-	// 		(nodeState.data?.p2p_enabled && nodeState.data?.p2p.ipv4.status === 'Error') || false;
-	// 	const ipv6Error =
-	// 		(nodeState.data?.p2p_enabled && nodeState.data?.p2p.ipv6.status === 'Error') || false;
+	useEffect(() => {
+		if (!listeners.data) return;
+		if (didShowError.current) return;
 
-	// 	if (!didShowError.ipv4 && ipv4Error)
-	// 		toast.error(
-	// 			{
-	// 				title: 'Error starting up P2P!',
-	// 				body: 'Error creating the IPv4 listener. Please check your firewall settings!'
-	// 			},
-	// 			{
-	// 				id: 'ipv4-listener-error'
-	// 			}
-	// 		);
+		let body: JSX.Element | undefined;
+		if (listeners.data.ipv4.type === 'Error' && listeners.data.ipv6.type === 'Error') {
+			body = (
+				<div>
+					<p>
+						Error creating the IPv4 and IPv6 listeners. Please check your firewall
+						settings!
+					</p>
+					<p>{listeners.data.ipv4.error}</p>
+				</div>
+			);
+		} else if (listeners.data.ipv4.type === 'Error') {
+			body = (
+				<div>
+					<p>Error creating the IPv4 listeners. Please check your firewall settings!</p>
+					<p>{listeners.data.ipv4.error}</p>
+				</div>
+			);
+		} else if (listeners.data.ipv6.type === 'Error') {
+			body = (
+				<div>
+					<p>Error creating the IPv6 listeners. Please check your firewall settings!</p>
+					<p>{listeners.data.ipv6.error}</p>
+				</div>
+			);
+		}
 
-	// 	if (!didShowError.ipv6 && ipv6Error)
-	// 		toast.error(
-	// 			{
-	// 				title: 'Error starting up P2P!',
-	// 				body: 'Error creating the IPv6 listener. Please check your firewall settings!'
-	// 			},
-	// 			{
-	// 				id: 'ipv6-listener-error'
-	// 			}
-	// 		);
-
-	// 	setDidShowError({
-	// 		ipv4: ipv4Error,
-	// 		ipv6: ipv6Error
-	// 	});
-	// 	// eslint-disable-next-line react-hooks/exhaustive-deps
-	// }, [nodeState.data]);
+		if (body) {
+			toast.error(
+				{
+					title: 'Error starting up networking!',
+					body
+				},
+				{
+					id: 'p2p-listener-error'
+				}
+			);
+			didShowError.current = true;
+		}
+		// eslint-disable-next-line react-hooks/exhaustive-deps
+	}, [listeners.data]);
 
 	return null;
 }

interface/index.tsx

@@ -17,7 +17,6 @@ import { toast, TooltipProvider } from '@sd/ui';
 import { createRoutes } from './app';
 import { SpacedropProvider } from './app/$libraryId/Spacedrop';
 import i18n from './app/I18n';
-import { useP2PErrorToast } from './app/p2p';
 import { Devtools } from './components/Devtools';
 import { WithPrismTheme } from './components/TextViewer/prism';
 import ErrorFallback, { BetterErrorBoundary } from './ErrorFallback';
@@ -79,7 +78,6 @@ export function SpacedriveRouterProvider(props: {
 
 export function SpacedriveInterfaceRoot({ children }: PropsWithChildren) {
 	useLoadBackendFeatureFlags();
-	useP2PErrorToast();
 	useInvalidateQuery();
 	useTheme();
 

packages/client/src/core.ts

@@ -40,6 +40,7 @@ export type Procedures = {
         { key: "notifications.dismiss", input: NotificationId, result: null } | 
         { key: "notifications.dismissAll", input: never, result: null } | 
         { key: "notifications.get", input: never, result: Notification[] } | 
+        { key: "p2p.listeners", input: never, result: Listeners } | 
         { key: "p2p.state", input: never, result: JsonValue } | 
         { key: "preferences.get", input: LibraryArgs<null>, result: LibraryPreferences } | 
         { key: "search.objects", input: LibraryArgs<ObjectSearchArgs>, result: SearchData<ExplorerItem> } | 
@@ -408,7 +409,9 @@ export type LibraryPreferences = { location?: { [key in string]: LocationSetting
 
 export type LightScanArgs = { location_id: number; sub_path: string }
 
-export type Listener2 = { id: string; name: string; addrs: string[] }
+export type ListenerState = { type: "Listening" } | { type: "Error"; error: string } | { type: "Disabled" }
+
+export type Listeners = { ipv4: ListenerState; ipv6: ListenerState }
 
 export type Location = { id: number; pub_id: number[]; name: string | null; path: string | null; total_capacity: number | null; available_capacity: number | null; size_in_bytes: number[] | null; is_archived: boolean | null; generate_preview_media: boolean | null; sync_preview_media: boolean | null; hidden: boolean | null; date_created: string | null; scan_state: number; instance_id: number | null }
 
@@ -457,7 +460,7 @@ id: string;
 /**
  * name is the display name of the current node. This is set by the user and is shown in the UI. // TODO: Length validation so it can fit in DNS record
  */
-name: string; identity: RemoteIdentity; p2p_ipv4_port: Port; p2p_ipv6_port: Port; p2p_discovery: P2PDiscoveryState; features: BackendFeature[]; preferences: NodePreferences; image_labeler_version: string | null }) & { data_path: string; listeners: Listener2[]; device_model: string | null }
+name: string; identity: RemoteIdentity; p2p_ipv4_port: Port; p2p_ipv6_port: Port; p2p_discovery: P2PDiscoveryState; features: BackendFeature[]; preferences: NodePreferences; image_labeler_version: string | null }) & { data_path: string; device_model: string | null }
 
 export type NonIndexedPathItem = { path: string; name: string; extension: string; kind: number; is_dir: boolean; date_created: string; date_modified: string; size_in_bytes_bytes: number[]; hidden: boolean }