Skip to content

Commit 9758058

Browse files
committed
fix(sync): make sync listener bind fatal and address configurable
The sync WebSocket listener always bound to its hardcoded default address and, if the bind failed, logged a warning and continued booting. That left the server up with no reachable sync listener and no clear indication anything was wrong, silently stranding NodeDB-Lite clients. Add a `sync` port to `PortsConfig` (default 9090, overridable via config file or `NODEDB_PORT_SYNC`) and a `ServerConfig::sync_addr()` accessor, and wire `spawn_protocol_listeners` to use it instead of `SyncListenerConfig::default()`. A sync-listener bind failure now propagates as an `Err` instead of a non-fatal warning, so boot fails loudly. Closes #209
1 parent f7fcc77 commit 9758058

6 files changed

Lines changed: 88 additions & 4 deletions

File tree

nodedb/src/bootstrap/listeners.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
55
use std::sync::Arc;
66

7+
use anyhow::Context;
78
use tracing::info;
89

910
use crate::ServerConfig;
@@ -34,14 +35,19 @@ pub struct ListenerInfra {
3435
///
3536
/// The native listener is not spawned here — it is run on the main task
3637
/// by the caller after this returns.
38+
///
39+
/// Returns `Err` if the sync WebSocket listener fails to bind — that
40+
/// failure is boot-fatal rather than a silent warning, since a server
41+
/// that comes up without a reachable sync listener leaves NodeDB-Lite
42+
/// clients unable to connect with no indication anything is wrong.
3743
pub async fn spawn_protocol_listeners(
3844
listeners: ProtocolListeners,
3945
shared: Arc<SharedState>,
4046
config: &ServerConfig,
4147
infra: ListenerInfra,
4248
base_acceptor: Option<tokio_rustls::TlsAcceptor>,
4349
cluster_handle: &Option<Arc<ClusterHandle>>,
44-
) {
50+
) -> anyhow::Result<()> {
4551
let ProtocolListeners {
4652
pg_listener,
4753
ilp_listener,
@@ -157,7 +163,10 @@ pub async fn spawn_protocol_listeners(
157163
}
158164

159165
// Sync WebSocket listener for NodeDB-Lite clients.
160-
let sync_config = crate::control::server::sync::listener::SyncListenerConfig::default();
166+
let sync_config = crate::control::server::sync::listener::SyncListenerConfig {
167+
listen_addr: config.sync_addr(),
168+
..Default::default()
169+
};
161170
match crate::control::server::sync::listener::start_sync_listener(
162171
sync_config,
163172
Some(Arc::clone(&shared)),
@@ -172,7 +181,7 @@ pub async fn spawn_protocol_listeners(
172181
);
173182
}
174183
Err(e) => {
175-
tracing::warn!(error = %e, "sync listener failed to start (non-fatal)");
184+
return Err(e).context("sync listener failed to bind");
176185
}
177186
}
178187

@@ -182,6 +191,8 @@ pub async fn spawn_protocol_listeners(
182191
handle.lifecycle.to_ready(nodes);
183192
}
184193
nodedb_cluster::readiness::notify_ready();
194+
195+
Ok(())
185196
}
186197

187198
/// Bind all protocol listeners to their configured addresses.

nodedb/src/config/server/config.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use crate::config::EngineConfig;
3939
/// pgwire = 6432
4040
/// native = 6433
4141
/// http = 6480
42+
/// sync = 9090
4243
///
4344
/// [auth]
4445
/// # ...
@@ -161,6 +162,11 @@ impl ServerConfig {
161162
self.addr(self.server.ports.http)
162163
}
163164

165+
/// Sync WebSocket listen address (NodeDB-Lite clients).
166+
pub fn sync_addr(&self) -> SocketAddr {
167+
self.addr(self.server.ports.sync)
168+
}
169+
164170
/// RESP listen address (None if disabled).
165171
pub fn resp_addr(&self) -> Option<SocketAddr> {
166172
self.server.ports.resp.map(|p| self.addr(p))
@@ -200,6 +206,7 @@ mod tests {
200206
assert_eq!(cfg.server.ports.native, 6433);
201207
assert_eq!(cfg.server.ports.pgwire, 6432);
202208
assert_eq!(cfg.server.ports.http, 6480);
209+
assert_eq!(cfg.server.ports.sync, 9090);
203210
assert!(cfg.server.ports.resp.is_none());
204211
assert!(cfg.server.ports.ilp.is_none());
205212
assert!(cfg.server.data_plane_cores >= 1);

nodedb/src/config/server/env.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,7 @@ fn apply_bool_env(var: &str, target: &mut bool) {
180180
/// - `NODEDB_PORT_NATIVE` — overrides `config.ports.native` (default 6433)
181181
/// - `NODEDB_PORT_PGWIRE` — overrides `config.ports.pgwire` (default 6432)
182182
/// - `NODEDB_PORT_HTTP` — overrides `config.ports.http` (default 6480)
183+
/// - `NODEDB_PORT_SYNC` — overrides `config.ports.sync` (default 9090)
183184
/// - `NODEDB_PORT_RESP` — overrides `config.ports.resp` (set to enable RESP)
184185
/// - `NODEDB_PORT_ILP` — overrides `config.ports.ilp` (set to enable ILP)
185186
/// - `NODEDB_DATA_DIR` — overrides `config.data_dir`
@@ -235,6 +236,7 @@ pub fn apply_env_overrides(config: &mut ServerConfig) {
235236
apply_port_env("NODEDB_PORT_NATIVE", &mut config.server.ports.native);
236237
apply_port_env("NODEDB_PORT_PGWIRE", &mut config.server.ports.pgwire);
237238
apply_port_env("NODEDB_PORT_HTTP", &mut config.server.ports.http);
239+
apply_port_env("NODEDB_PORT_SYNC", &mut config.server.ports.sync);
238240
apply_optional_port_env("NODEDB_PORT_RESP", &mut config.server.ports.resp);
239241
apply_optional_port_env("NODEDB_PORT_ILP", &mut config.server.ports.ilp);
240242

@@ -679,6 +681,29 @@ mod tests {
679681
unsafe { std::env::remove_var("NODEDB_MEMORY_LIMIT") };
680682
}
681683

684+
/// Tests valid and malformed `NODEDB_PORT_SYNC` sequentially to avoid
685+
/// env-var races (env vars are process-global, Rust tests run in parallel).
686+
#[test]
687+
fn env_sync_port_overrides() {
688+
// ── Valid value → overrides ports.sync ──
689+
unsafe { std::env::set_var("NODEDB_PORT_SYNC", "19090") };
690+
let mut cfg = ServerConfig::default();
691+
apply_env_overrides(&mut cfg);
692+
assert_eq!(cfg.server.ports.sync, 19090);
693+
694+
// ── Malformed value → ports.sync unchanged ──
695+
unsafe { std::env::set_var("NODEDB_PORT_SYNC", "notaport") };
696+
let mut cfg = ServerConfig::default();
697+
let before = cfg.server.ports.sync;
698+
apply_env_overrides(&mut cfg);
699+
assert_eq!(
700+
cfg.server.ports.sync, before,
701+
"malformed value must not change config"
702+
);
703+
704+
unsafe { std::env::remove_var("NODEDB_PORT_SYNC") };
705+
}
706+
682707
#[test]
683708
fn env_cluster_overrides() {
684709
// Always start clean.

nodedb/src/config/server/ports.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ pub struct PortsConfig {
2020
/// HTTP API port (REST, SSE, WebSocket). Default: 6480.
2121
#[serde(default = "default_http_port")]
2222
pub http: u16,
23+
/// Sync WebSocket port (NodeDB-Lite clients). Default: 9090.
24+
#[serde(default = "default_sync_port")]
25+
pub sync: u16,
2326
/// RESP (Redis-compatible) port. Disabled by default. Set to enable.
2427
#[serde(default)]
2528
pub resp: Option<u16>,
@@ -34,6 +37,7 @@ impl Default for PortsConfig {
3437
native: default_native_port(),
3538
pgwire: default_pgwire_port(),
3639
http: default_http_port(),
40+
sync: default_sync_port(),
3741
resp: None,
3842
ilp: None,
3943
}
@@ -49,6 +53,9 @@ fn default_pgwire_port() -> u16 {
4953
fn default_http_port() -> u16 {
5054
6480
5155
}
56+
fn default_sync_port() -> u16 {
57+
9090
58+
}
5259

5360
#[cfg(test)]
5461
mod tests {
@@ -60,6 +67,7 @@ mod tests {
6067
assert_eq!(p.native, 6433);
6168
assert_eq!(p.pgwire, 6432);
6269
assert_eq!(p.http, 6480);
70+
assert_eq!(p.sync, 9090);
6371
assert!(p.resp.is_none());
6472
assert!(p.ilp.is_none());
6573
}

nodedb/src/control/server/sync/listener.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,3 +163,36 @@ async fn accept_loop(
163163
}
164164
}
165165
}
166+
167+
#[cfg(test)]
168+
mod tests {
169+
use super::*;
170+
171+
/// Binding to an address that's already occupied must surface as `Err`,
172+
/// not panic or silently succeed — this is the behavior
173+
/// `spawn_protocol_listeners` relies on to treat a sync-listener bind
174+
/// failure as boot-fatal instead of a silent non-fatal warning.
175+
#[tokio::test]
176+
async fn start_sync_listener_returns_err_on_occupied_port() {
177+
// Reserve an ephemeral port via a real listener so we know it's taken.
178+
let occupied = TcpListener::bind(SocketAddr::from(([127, 0, 0, 1], 0)))
179+
.await
180+
.expect("bind ephemeral listener to reserve a port");
181+
let addr = occupied
182+
.local_addr()
183+
.expect("local addr of reserved listener");
184+
185+
let cfg = SyncListenerConfig {
186+
listen_addr: addr,
187+
..Default::default()
188+
};
189+
let result = start_sync_listener(cfg, None).await;
190+
191+
assert!(
192+
result.is_err(),
193+
"expected start_sync_listener to return Err when the address is already bound"
194+
);
195+
196+
drop(occupied);
197+
}
198+
}

nodedb/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ async fn main() -> anyhow::Result<()> {
224224
base_acceptor.clone(),
225225
&cluster_handle,
226226
)
227-
.await;
227+
.await?;
228228

229229
// Native protocol TLS.
230230
let native_tls = tls_for(native_tls_enabled);

0 commit comments

Comments
 (0)