fix: harden LogosDelivery facade lifecycle

* MessagingClient.new: return a Result via a new newMessagingClient
   factory instead of quit(QuitFailure) on a recoverable SendService
   failure; no longer aborts the FFI host (restores master's behaviour).
   Callers (facade, FFI mount, tests) updated to the factory.
 * LogosDelivery.stop(): best-effort run-to-completion; stop and nil every
   component even if a prior one fails, aggregating errors (was: early
   return that leaked the messaging client and the node).
 * LogosDelivery.getNodeInfo(): nil-guard self.waku like the sibling
   getters (was: nil-deref before start / after stop).
This commit is contained in:
Fabiana Cecin 2026-06-19 12:09:36 -03:00
parent 43452e200b
commit 488266888f
No known key found for this signature in database
GPG Key ID: BCAB8A55CB51B6C7
4 changed files with 40 additions and 23 deletions

View File

@ -12,6 +12,7 @@
import results, chronos
import std/options # some()/Option for WakuNodeConf.mode
import std/json # newJArray/newJObject/%*/pretty in getAvailableConfigs
import std/strutils
import brokers/broker_context, brokers/broker_interface, brokers/broker_implement
import logos_delivery/api/logos_delivery_interface as logosdelivery_iface
@ -48,10 +49,8 @@ proc createNode(conf: WakuNodeConf): Future[Result[Waku, string]] {.async.} =
return ok(wakuRes)
proc initMessagingClient(self: LogosDelivery): Result[MessagingClient, string] =
return ok(
MessagingClient.createUnderContext(
globalBrokerContext(), self.waku.conf.p2pReliability, self.waku.node
)
newMessagingClient(
globalBrokerContext(), self.waku.conf.p2pReliability, self.waku.node
)
proc initReliableChannelManager(
@ -147,32 +146,36 @@ BrokerImplement LogosDelivery of LogosDeliveryInterface:
return err("initialize failed: " & e.msg)
method stop(self: LogosDelivery): Future[Result[void, string]] {.async.} =
var errs: seq[string]
if not self.reliableChannelManager.isNil():
try:
await self.reliableChannelManager.stop()
self.reliableChannelManager.close()
self.reliableChannelManager = nil
except CatchableError as e:
return err("ReliableChannelManager stop failed: " & e.msg)
errs.add("ReliableChannelManager stop failed: " & e.msg)
self.reliableChannelManager = nil
if not self.messagingClient.isNil():
try:
await self.messagingClient.stop()
self.messagingClient.close()
self.messagingClient = nil
except CatchableError as e:
return err("MessagingClient stop failed: " & e.msg)
errs.add("MessagingClient stop failed: " & e.msg)
self.messagingClient = nil
if not self.waku.isNil():
try:
(await self.waku.stop()).isOkOr:
return err("Node stop failed: " & $error)
# `Waku` is a plain ref object (not a BrokerImplement) — `stop()` is its
# only teardown; there is no `close()`.
self.waku = nil
(await self.waku.stop()).isOkOr:
errs.add("Node stop failed: " & $error)
except CatchableError as e:
return err("Node stop failed: " & e.msg)
errs.add("Node stop failed: " & e.msg)
self.waku = nil
if errs.len > 0:
return err(errs.join("; "))
return ok()
method shutdown(self: LogosDelivery): Future[Result[void, string]] {.async.} =
@ -181,6 +184,8 @@ BrokerImplement LogosDelivery of LogosDeliveryInterface:
method getNodeInfo(
self: LogosDelivery, id: NodeInfoId
): Future[Result[string, string]] {.async.} =
if self.waku.isNil():
return err("not initialized; call startAsNode or startAsClient first")
return ok(self.waku.stateInfo.getNodeInfoItem(id))
method getAvailableConfigs(

View File

@ -1,6 +1,6 @@
import results, chronos
import chronicles
import brokers/broker_implement
import brokers/broker_implement, brokers/broker_context
import logos_delivery/api/messaging_client_interface
import
logos_delivery/waku/api/types,
@ -30,12 +30,12 @@ proc stop*(self: MessagingClient) {.async.} =
self.started = false
BrokerImplement MessagingClient of MessagingClientInterface:
proc new*(T: typedesc[MessagingClient], useP2PReliability: bool, node: WakuNode): T =
let sendService = SendService.new(useP2PReliability, node).valueOr:
error "Failed to initialize SendService", error = error
quit(QuitFailure)
let recvService = RecvService.new(node)
proc new*(
T: typedesc[MessagingClient],
node: WakuNode,
sendService: SendService,
recvService: RecvService,
): T =
T(node: node, sendService: sendService, recvService: recvService, started: false)
method subscribe(
@ -71,3 +71,12 @@ BrokerImplement MessagingClient of MessagingClientInterface:
asyncSpawn self.sendService.send(deliveryTask)
return ok(requestId)
proc newMessagingClient*(
ctx: BrokerContext, useP2PReliability: bool, node: WakuNode
): Result[MessagingClient, string] =
let sendService = SendService.new(useP2PReliability, node).valueOr:
error "Failed to initialize SendService", error = error
return err("Failed to initialize SendService: " & error)
let recvService = RecvService.new(node)
ok(MessagingClient.createUnderContext(ctx, node, sendService, recvService))

View File

@ -44,8 +44,7 @@ proc mountMessagingClient*(w: Waku): Result[void, string] =
if not w.messagingClient.isNil():
return ok()
w.messagingClient =
MessagingClient.createUnderContext(w.brokerCtx, w.conf.p2pReliability, w.node)
w.messagingClient = ?newMessagingClient(w.brokerCtx, w.conf.p2pReliability, w.node)
return ok()
# TODO workaround for legacy use. It will be removed as soon all usage goes through LogosDelivery.

View File

@ -228,7 +228,9 @@ suite "Health Monitor - events":
nodeA.mountMetadata(1, @[0'u16]).expect("Node A failed to mount metadata")
await nodeA.start()
let ds = MessagingClient.new(false, nodeA)
let ds = newMessagingClient(nodeA.brokerCtx, false, nodeA).expect(
"Failed to create MessagingClient"
)
ds.start().expect("Failed to start MessagingClient")
let monitorA = NodeHealthMonitor.new(nodeA)
@ -331,7 +333,9 @@ suite "Health Monitor - events":
nodeA.mountMetadata(1, @[0'u16]).expect("Node A failed to mount metadata")
await nodeA.start()
let ds = MessagingClient.new(false, nodeA)
let ds = newMessagingClient(nodeA.brokerCtx, false, nodeA).expect(
"Failed to create MessagingClient"
)
ds.start().expect("Failed to start MessagingClient")
let subMgr = nodeA.subscriptionManager