From 689ef70de9be3a8f0f8836d0633204fab2d90479 Mon Sep 17 00:00:00 2001 From: Fabiana Cecin Date: Mon, 4 May 2026 20:40:43 -0300 Subject: [PATCH] Fixes from Ivan's second pass review --- tests/factory/test_node_factory.nim | 4 +++- waku/discovery/waku_discv5.nim | 2 +- waku/factory/waku.nim | 12 +++++------- waku/net/auto_port.nim | 18 ++++++++++++++---- waku/net/bound_ports.nim | 27 ++++++++++----------------- waku/node/waku_metrics.nim | 5 +++-- 6 files changed, 36 insertions(+), 32 deletions(-) diff --git a/tests/factory/test_node_factory.nim b/tests/factory/test_node_factory.nim index 81e7745f7..1fe242532 100644 --- a/tests/factory/test_node_factory.nim +++ b/tests/factory/test_node_factory.nim @@ -13,7 +13,9 @@ import tests/testlib/[wakunode, wakucore], waku/[waku_node, waku_enr, net/auto_port, discovery/waku_discv5, node/waku_metrics], waku/factory/[ - node_factory, internal_config, conf_builder/conf_builder, + node_factory, + internal_config, + conf_builder/conf_builder, conf_builder/web_socket_conf_builder, ] diff --git a/waku/discovery/waku_discv5.nim b/waku/discovery/waku_discv5.nim index cb448fae1..5877a0c1e 100644 --- a/waku/discovery/waku_discv5.nim +++ b/waku/discovery/waku_discv5.nim @@ -478,7 +478,7 @@ proc setupAndStartDiscv5*( return err(error) let startRes = await wd.start() if startRes.isErr(): - return err(startRes.error) + return err("failed to start discovery, attempt: " & startRes.error) return ok(wd) let wd = (await tryWithAutoPort[WakuDiscoveryV5](conf.udpPort, attempt)).valueOr: diff --git a/waku/factory/waku.nim b/waku/factory/waku.nim index 0e78f1fd9..e2efb471c 100644 --- a/waku/factory/waku.nim +++ b/waku/factory/waku.nim @@ -204,7 +204,7 @@ proc new*( if not restServer.isNil(): let boundRestPort = restServer.httpServer.address.port - node.ports.rest = some(boundRestPort.uint16) + node.ports.rest = boundRestPort.uint16 wakuConf.restServerConf.get().port = boundRestPort # Set the extMultiAddrsOnly flag so the node knows not to replace explicit addresses @@ -398,10 +398,8 @@ proc startWaku*(waku: ptr Waku): Future[Result[void, string]] {.async: (raises: let bound = getPorts(waku.node.switch.peerInfo.listenAddrs).valueOr: return err("failed to read bound ports from switch: " & $error) - if bound.tcpPort.isSome(): - waku[].node.ports.tcp = some(bound.tcpPort.get().uint16) - if bound.websocketPort.isSome(): - waku[].node.ports.webSocket = some(bound.websocketPort.get().uint16) + waku[].node.ports.tcp = bound.tcpPort.get(Port(0)).uint16 + waku[].node.ports.webSocket = bound.websocketPort.get(Port(0)).uint16 ## Discv5 if conf.discv5Conf.isSome(): @@ -420,7 +418,7 @@ proc startWaku*(waku: ptr Waku): Future[Result[void, string]] {.async: (raises: ).valueOr: return err("failed to start waku discovery v5: " & error) - waku[].node.ports.discv5Udp = some(waku[].wakuDiscV5.udpPort.uint16) + waku[].node.ports.discv5Udp = waku[].wakuDiscV5.udpPort.uint16 waku[].conf.discv5Conf.get().udpPort = waku[].wakuDiscV5.udpPort ## Update waku data that is set dynamically on node start @@ -505,7 +503,7 @@ proc startWaku*(waku: ptr Waku): Future[Result[void, string]] {.async: (raises: ).valueOr: return err("Starting monitoring and external interfaces failed: " & error) waku[].metricsServer = server - waku[].node.ports.metrics = some(port.uint16) + waku[].node.ports.metrics = port.uint16 waku[].conf.metricsServerConf.get().httpPort = port except CatchableError: return err( diff --git a/waku/net/auto_port.nim b/waku/net/auto_port.nim index a49461aad..38176d27d 100644 --- a/waku/net/auto_port.nim +++ b/waku/net/auto_port.nim @@ -7,10 +7,10 @@ const AutoPortRetryCount* = 20 AutoPortMin = 50000'u16 AutoPortMax = 59000'u16 - -var rng = initRand() + AutoPortAttemptTimeout = chronos.seconds(30) proc getAutoPort*(): uint16 = + var rng = initRand() uint16(rng.rand(AutoPortMin.int .. AutoPortMax.int)) proc tryWithAutoPort*[T]( @@ -29,10 +29,20 @@ proc tryWithAutoPort*[T]( Port(getAutoPort()) else: startingPort - let res = await attempt(port) + let fut = attempt(port) + let res = + try: + if await fut.withTimeout(AutoPortAttemptTimeout): + await fut + else: + fut.cancelSoon() + Result[T, string].err("bind attempt timed out") + except CancelledError: + fut.cancelSoon() + Result[T, string].err("bind attempt cancelled") if res.isOk(): return ok(res.get()) lastErr = res.error if autoMode: return err("auto-port exhausted; last error: " & lastErr) - return err(lastErr) + return err("port bind failed: " & lastErr) diff --git a/waku/net/bound_ports.nim b/waku/net/bound_ports.nim index 0c7e259bf..797698a60 100644 --- a/waku/net/bound_ports.nim +++ b/waku/net/bound_ports.nim @@ -1,27 +1,20 @@ {.push raises: [].} -import std/[json, options] +import std/json type BoundPorts* {.requiresInit.} = object ## Set by the factory once each service has bound to a port. - tcp*: Option[uint16] - webSocket*: Option[uint16] - rest*: Option[uint16] - discv5Udp*: Option[uint16] - metrics*: Option[uint16] + ## A value of 0 means the service was not enabled or did not bind. + tcp*: uint16 + webSocket*: uint16 + rest*: uint16 + discv5Udp*: uint16 + metrics*: uint16 proc init*(T: type BoundPorts): BoundPorts = - BoundPorts( - tcp: none(uint16), - webSocket: none(uint16), - rest: none(uint16), - discv5Udp: none(uint16), - metrics: none(uint16), + return BoundPorts( + tcp: 0'u16, webSocket: 0'u16, rest: 0'u16, discv5Udp: 0'u16, metrics: 0'u16 ) proc toJsonString*(p: BoundPorts): string = - var obj = newJObject() - for name, value in fieldPairs(p): - if value.isSome(): - obj[name] = %value.get() - return $obj + return $(%*p) diff --git a/waku/node/waku_metrics.nim b/waku/node/waku_metrics.nim index 7ab308605..af74b1532 100644 --- a/waku/node/waku_metrics.nim +++ b/waku/node/waku_metrics.nim @@ -67,12 +67,13 @@ proc startMetricsServer( info "Starting metrics HTTP server", serverIp = $serverIp, serverPort = $port let server = MetricsHttpServerRef.new($serverIp, port).valueOr: - return err($error) + return err("fail to start service metrics server, attempt:" & $error) try: await server.start() except CatchableError: - return err(getCurrentExceptionMsg()) + return + err("exception while startMetricsServer, attempt: " & getCurrentExceptionMsg()) info "Metrics HTTP server started", serverIp = $serverIp, serverPort = $port return ok((server: server, port: port))