Fix race condition in rpc client / test #1829
Conversation
✅ Deploy Preview for nitro-payment-demo canceled.
|
👷 Deploy Preview for nitrodocs processing.
|
| h := tint.NewHandler(w, &tint.Options{ | ||
| Level: level, | ||
| TimeFormat: time.Kitchen, | ||
| }) |
There was a problem hiding this comment.
gives output like this:
and be more selective in uploading logs
| * @param objectiveId - The channel id to wait for | ||
| * @param status - The channel id to wait for (e.g. Ready or Closing) | ||
| */ | ||
| WaitForLedgerChannelToHaveStatus( |
There was a problem hiding this comment.
Nit: could change the function name to WaitForLedgerChannelStatus to be a bit more concise
| } | ||
| ); | ||
| }); | ||
| const ledger = await this.GetLedgerChannel(channelId); |
There was a problem hiding this comment.
Is there need for both this GetLedgerChannel call and the same one above nested inside the Notifications handler?
There was a problem hiding this comment.
What I am trying to do here is make it impossible for there to be a race condition. So the approach is,
Step 1: register a handler in case the event has not yet happened (if it already happened, this will never fire)
Step 2: make a static query in case the event has already happened.
Which should result in the promise resolving whenever the event fires / was fired.
Does that make sense?
Closes #1749
There are a number of ways to solve this issue (essentially that the
ObjectiveCompletedevent can fire before an event listener is registered for it). One would be to cache completed objectives in the client (which is how the go rpc client does it).I went another way, since that solution will only work if the event fired after the client itself is started. Here we attach the listener and perform a static query for the information straight afterward (with this being abstracted inside the rpc client).
Further, since we do not have a query path for objectives, I have flipped over the events and queries to be channel-centric and not objective-centric. This is mostly justified by expediency, but I think also it simplifies the API to not have objectives as an additional concept for developers to understand.