search

waku-org / nwaku

Waku node and protocol.
Other
201 stars 53 forks source link

fix: changing libwaku's error handling format #3093

Closed gabrielmer closed 1 month ago

gabrielmer commented 1 month ago

Description

For some reason, when immediately chaining sendRequestToWakuThread's response with the handleRes template, there's cases in which the errors are not handled properly. See this message for a more detailed description of the issue.

It's probably due to some misusage of Nim templates, but until we further understand what's wrong with out chaining approach, modifying to a working version.

Update

The template usage was wrong - changed it to a proc and now works great :)

Changes

Issue

3039

github-actions[bot] commented 1 month ago

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:3093

Built from ad0c77748ebbb05217d2dca97c9cbb40ad502068

gabrielmer commented 1 month ago

@arnetheduck whenever you're available could you please take a quick look? maybe you have a better understanding on what's going on 😶 we're not sure why does

waku_thread
  .sendRequestToWakuThread(
    ctx,
    RequestType.LIFECYCLE,
    NodeLifecycleRequest.createShared(NodeLifecycleMsgType.START_NODE),
  )
  .handleRes(callback, userData)

behave differently than

let res = waku_thread
  .sendRequestToWakuThread(
    ctx,
    RequestType.LIFECYCLE,
    NodeLifecycleRequest.createShared(NodeLifecycleMsgType.START_NODE),
  )

res.handleRes(callback, userData)

handleRes is defined here https://github.com/waku-org/nwaku/blob/0f8e87400024a4f0cb08b2f2db18336b40e1e8c0/library/libwaku.nim#L55-L72

arnetheduck commented 1 month ago 
[12:22]arnetheduck: the way the code is written, the handleRes template is calling sendRequestToWakuThread twice when there's an error, I'm pretty sure that's not what you intended
[12:24]arnetheduck: parameters in templates get their full ast expanded at every point of use - if you use res twice, you get two expansions of sendRequestToWakuThread...
[12:25]arnetheduck: that's why moving to let "fixes" it - instead of two calls, it accesses the variable twice