Skip to content

adapt event types and values to messaging-api spec #3839

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Ivansete-status wants to merge 2 commits into
base: master
Choose a base branch
from
Open

adapt event types and values to messaging-api spec #3839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc90281
adapt event types and values to messaging-api spec
Ivansete-status Apr 29, 2026
5cc6900
force nimbledeps deps update if corrupted in CI
Ivansete-status Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ jobs:
key: ${{ runner.os }}-nimbledeps-nimble${{ env.NIMBLE_VERSION }}-${{ hashFiles('nimble.lock', 'BearSSL.mk', 'Nat.mk') }}

- name: Install nimble deps
if: steps.cache-nimbledeps.outputs.cache-hit != 'true'
# Also run if sentinel file is missing: guards against a corrupt cache
# saved after a partial install (e.g. interrupted run).
if: steps.cache-nimbledeps.outputs.cache-hit != 'true' || !hashFiles('nimbledeps/.nimble-setup')
run: |
nimble setup --localdeps -y
make rebuild-nat-libs-nimbledeps
Expand Down Expand Up @@ -140,7 +142,9 @@ jobs:
key: ${{ runner.os }}-nimbledeps-nimble${{ env.NIMBLE_VERSION }}-${{ hashFiles('nimble.lock', 'BearSSL.mk', 'Nat.mk') }}

- name: Install nimble deps
if: steps.cache-nimbledeps.outputs.cache-hit != 'true'
# Also run if sentinel file is missing: guards against a corrupt cache
# saved after a partial install (e.g. interrupted run).
if: steps.cache-nimbledeps.outputs.cache-hit != 'true' || !hashFiles('nimbledeps/.nimble-setup')
run: |
nimble setup --localdeps -y
make rebuild-nat-libs-nimbledeps
Expand Down Expand Up @@ -219,7 +223,9 @@ jobs:
key: ${{ runner.os }}-nimbledeps-nimble${{ env.NIMBLE_VERSION }}-${{ hashFiles('nimble.lock', 'BearSSL.mk', 'Nat.mk') }}

- name: Install nimble deps
if: steps.cache-nimbledeps.outputs.cache-hit != 'true'
# Also run if sentinel file is missing: guards against a corrupt cache
# saved after a partial install (e.g. interrupted run).
if: steps.cache-nimbledeps.outputs.cache-hit != 'true' || !hashFiles('nimbledeps/.nimble-setup')
run: |
nimble setup --localdeps -y
make rebuild-nat-libs-nimbledeps
Expand Down
16 changes: 8 additions & 8 deletions examples/api_example/api_example.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,26 @@ proc periodicSender(w: Waku): Future[void] {.async.} =
echo "Failed to listen to message sent event: ", error
return

let errorListener = MessageErrorEvent.listen(
proc(event: MessageErrorEvent) {.async: (raises: []).} =
let errorListener = MessageSendErrorEvent.listen(
proc(event: MessageSendErrorEvent) {.async: (raises: []).} =
echo "Message failed to send with request ID: ",
event.requestId, " error: ", event.error
).valueOr:
echo "Failed to listen to message error event: ", error
echo "Failed to listen to message send error event: ", error
return

let propagatedListener = MessagePropagatedEvent.listen(
proc(event: MessagePropagatedEvent) {.async: (raises: []).} =
let propagatedListener = MessageSendPropagatedEvent.listen(
proc(event: MessageSendPropagatedEvent) {.async: (raises: []).} =
echo "Message propagated with request ID: ",
event.requestId, " hash: ", event.messageHash
).valueOr:
echo "Failed to listen to message propagated event: ", error
echo "Failed to listen to message send propagated event: ", error
return

defer:
MessageSentEvent.dropListener(sentListener)
MessageErrorEvent.dropListener(errorListener)
MessagePropagatedEvent.dropListener(propagatedListener)
MessageSendErrorEvent.dropListener(errorListener)
MessageSendPropagatedEvent.dropListener(propagatedListener)

## Periodically sends a Waku message every 30 seconds
var counter = 0
Expand Down
32 changes: 16 additions & 16 deletions liblogosdelivery/logos_delivery_api/node_api.nim
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@NagyZoltanPeter NagyZoltanPeter Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are breaking changes for logos-delivery-module! Needs follow up and new release afterward.

Original file line number Diff line number Diff line change
Expand Up @@ -131,34 +131,34 @@ proc logosdelivery_start_node(
ctx.myLib[].brokerCtx,
proc(event: MessageSentEvent) {.async: (raises: []).} =
callEventCallback(ctx, "onMessageSent"):
$newJsonEvent("message_sent", event),
$newJsonEvent("message:sent", event),
Copy link
Copy Markdown
Contributor

@NagyZoltanPeter NagyZoltanPeter Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the : is better than _? just curious!

Copy link
Copy Markdown
Contributor

@igor-sirotin igor-sirotin May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also looks like a breaking change 🤔

).valueOr:
chronicles.error "MessageSentEvent.listen failed", err = $error
return err("MessageSentEvent.listen failed: " & $error)

let errorListener = MessageErrorEvent.listen(
let errorListener = MessageSendErrorEvent.listen(
ctx.myLib[].brokerCtx,
proc(event: MessageErrorEvent) {.async: (raises: []).} =
callEventCallback(ctx, "onMessageError"):
$newJsonEvent("message_error", event),
proc(event: MessageSendErrorEvent) {.async: (raises: []).} =
callEventCallback(ctx, "onMessageSendError"):
$newJsonEvent("message:send-error", event),
).valueOr:
chronicles.error "MessageErrorEvent.listen failed", err = $error
return err("MessageErrorEvent.listen failed: " & $error)
chronicles.error "MessageSendErrorEvent.listen failed", err = $error
return err("MessageSendErrorEvent.listen failed: " & $error)

let propagatedListener = MessagePropagatedEvent.listen(
let propagatedListener = MessageSendPropagatedEvent.listen(
ctx.myLib[].brokerCtx,
proc(event: MessagePropagatedEvent) {.async: (raises: []).} =
callEventCallback(ctx, "onMessagePropagated"):
$newJsonEvent("message_propagated", event),
proc(event: MessageSendPropagatedEvent) {.async: (raises: []).} =
callEventCallback(ctx, "onMessageSendPropagated"):
$newJsonEvent("message:send-propagated", event),
).valueOr:
chronicles.error "MessagePropagatedEvent.listen failed", err = $error
return err("MessagePropagatedEvent.listen failed: " & $error)
chronicles.error "MessageSendPropagatedEvent.listen failed", err = $error
return err("MessageSendPropagatedEvent.listen failed: " & $error)

let receivedListener = MessageReceivedEvent.listen(
ctx.myLib[].brokerCtx,
proc(event: MessageReceivedEvent) {.async: (raises: []).} =
callEventCallback(ctx, "onMessageReceived"):
$newJsonEvent("message_received", event),
$newJsonEvent("message:received", event),
).valueOr:
chronicles.error "MessageReceivedEvent.listen failed", err = $error
return err("MessageReceivedEvent.listen failed: " & $error)
Expand All @@ -184,9 +184,9 @@ proc logosdelivery_stop_node(
requireInitializedNode(ctx, "STOP_NODE"):
return err(errMsg)

MessageErrorEvent.dropAllListeners(ctx.myLib[].brokerCtx)
MessageSendErrorEvent.dropAllListeners(ctx.myLib[].brokerCtx)
MessageSentEvent.dropAllListeners(ctx.myLib[].brokerCtx)
MessagePropagatedEvent.dropAllListeners(ctx.myLib[].brokerCtx)
MessageSendPropagatedEvent.dropAllListeners(ctx.myLib[].brokerCtx)
MessageReceivedEvent.dropAllListeners(ctx.myLib[].brokerCtx)
EventConnectionStatusChange.dropAllListeners(ctx.myLib[].brokerCtx)

Expand Down
16 changes: 8 additions & 8 deletions tests/api/test_api_send.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ type SendEventOutcome {.pure.} = enum
type SendEventListenerManager = ref object
brokerCtx: BrokerContext
sentListener: MessageSentEventListener
errorListener: MessageErrorEventListener
propagatedListener: MessagePropagatedEventListener
errorListener: MessageSendErrorEventListener
propagatedListener: MessageSendPropagatedEventListener
sentFuture: Future[void]
errorFuture: Future[void]
propagatedFuture: Future[void]
Expand Down Expand Up @@ -48,9 +48,9 @@ proc newSendEventListenerManager(brokerCtx: BrokerContext): SendEventListenerMan
).valueOr:
raiseAssert error

manager.errorListener = MessageErrorEvent.listen(
manager.errorListener = MessageSendErrorEvent.listen(
brokerCtx,
proc(event: MessageErrorEvent) {.async: (raises: []).} =
proc(event: MessageSendErrorEvent) {.async: (raises: []).} =
inc manager.errorCount
manager.errorRequestIds.add(event.requestId)
echo "ERROR EVENT TRIGGERED (#", manager.errorCount, "): ", event.error
Expand All @@ -62,9 +62,9 @@ proc newSendEventListenerManager(brokerCtx: BrokerContext): SendEventListenerMan
).valueOr:
raiseAssert error

manager.propagatedListener = MessagePropagatedEvent.listen(
manager.propagatedListener = MessageSendPropagatedEvent.listen(
brokerCtx,
proc(event: MessagePropagatedEvent) {.async: (raises: []).} =
proc(event: MessageSendPropagatedEvent) {.async: (raises: []).} =
inc manager.propagatedCount
manager.propagatedRequestIds.add(event.requestId)
echo "PROPAGATED EVENT TRIGGERED (#",
Expand All @@ -79,8 +79,8 @@ proc newSendEventListenerManager(brokerCtx: BrokerContext): SendEventListenerMan

proc teardown(manager: SendEventListenerManager) =
MessageSentEvent.dropListener(manager.brokerCtx, manager.sentListener)
MessageErrorEvent.dropListener(manager.brokerCtx, manager.errorListener)
MessagePropagatedEvent.dropListener(manager.brokerCtx, manager.propagatedListener)
MessageSendErrorEvent.dropListener(manager.brokerCtx, manager.errorListener)
MessageSendPropagatedEvent.dropListener(manager.brokerCtx, manager.propagatedListener)

proc waitForEvents(
manager: SendEventListenerManager, timeout: Duration
Expand Down
4 changes: 2 additions & 2 deletions waku/events/message_events.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,14 @@ EventBroker:

EventBroker:
# Event emitted when a message send operation fails
type MessageErrorEvent* = object
type MessageSendErrorEvent* = object
requestId*: RequestId
messageHash*: string
error*: string

EventBroker:
# Confirmation that a message has been correctly delivered to some neighbouring nodes.
type MessagePropagatedEvent* = object
type MessageSendPropagatedEvent* = object
requestId*: RequestId
messageHash*: string

Expand Down
6 changes: 3 additions & 3 deletions waku/node/delivery_service/send_service/send_service.nim
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ proc reportTaskResult(self: SendService, task: DeliveryTask) =
if not task.propagateEventEmitted:
info "Message successfully propagated",
requestId = task.requestId, msgHash = task.msgHash.to0xHex()
MessagePropagatedEvent.emit(
MessageSendPropagatedEvent.emit(
Copy link
Copy Markdown
Contributor

@NagyZoltanPeter NagyZoltanPeter Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here the spec is wrong and the simpler form MessagePropagatedEvent is the better.
The wording of Send here misleading... compared to 'MessageSentEvent'.
WDYT?

Copy link
Copy Markdown
Contributor

@fcecin fcecin Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my fault; I should have renamed the events in the code when I cleaned up the spec before the LMAPI release.

I agree with Zoltan here. We should fix the spec, not the code.

MessageSendErrorEvent is technically correct, but MessageErrorEvent stuck and at the "simple send" LMAPI level, the only MessageErrorEvent is the send event, so the discrimination that I added there is doing zero actual work.

Also MessageSendPropagatedEvent -> MessagePropagatedEvent with even less loss of meaning or risk of symbol collision or rot.

What do you guys think? Should we submit a fix to the spec instead?

Copy link
Copy Markdown
Collaborator Author

@Ivansete-status Ivansete-status Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay fair enough. Thanks for the comments both!
Let's keep this opened and we can adapt the spec once we fully move to lips. logos-co/logos-lips#315

self.brokerCtx, task.requestId, task.msgHash.to0xHex()
)
task.propagateEventEmitted = true
Expand All @@ -196,7 +196,7 @@ proc reportTaskResult(self: SendService, task: DeliveryTask) =
requestId = task.requestId,
msgHash = task.msgHash.to0xHex(),
error = task.errorDesc
MessageErrorEvent.emit(
MessageSendErrorEvent.emit(
self.brokerCtx, task.requestId, task.msgHash.to0xHex(), task.errorDesc
)
return
Expand All @@ -211,7 +211,7 @@ proc reportTaskResult(self: SendService, task: DeliveryTask) =
error = "Message too old",
age = task.messageAge()
task.state = DeliveryState.FailedToDeliver
MessageErrorEvent.emit(
MessageSendErrorEvent.emit(
self.brokerCtx,
task.requestId,
task.msgHash.to0xHex(),
Expand Down
Loading