tests: fix test_grpc_custommsg_notification flake#9154
Open
cdecker wants to merge 2 commits into
Open
Conversation
Changelog-Added: askrene: askrene-remove-channel-update, a new RPC to remove channel_update entries from layers. Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
The test was missing the same synchronization barrier that test_grpc_connect_notification already has: after calling SubscribeCustomMsg() on the Python side the gRPC server-side subscribe_custom_msg() handler runs asynchronously, so the broadcast::Receiver is not yet registered when the event fires. Waiting for 'plugin-cln-grpc: received settings ACK' (emitted by the h2 HTTP/2 layer on receipt of the client's SETTINGS_ACK frame) ensures the full connection handshake has completed and the subscription is live before we trigger the custommsg.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
test_grpc_custommsg_notificationwas flaky (example failure: https://github.com/ElementsProject/lightning/actions/runs/26100805227/job/76753937004).Root Cause
After
SubscribeCustomMsg()returns on the Python side, the gRPC server-side handler (subscribe_custom_msgincln-grpc/src/server.rs) runs asynchronously in the Rust async runtime. It creates abroadcast::Receiverby callingself.events.subscribe(), which only receives events published after registration. Ifsendcustommsgfires before the handler has registered the receiver, the notification is broadcast with no listeners and is permanently lost.Fix
test_grpc_connect_notificationalready has this exact fix (and a FIXME comment explaining it): wait for'plugin-cln-grpc: received settings ACK'in the node log before triggering the event. This string is emitted by the h2 HTTP/2 library (used internally by tonic) when the server receives the client'sSETTINGS_ACKframe — guaranteeing the full connection handshake is complete and the subscription receiver is live.Apply the same synchronization barrier to
test_grpc_custommsg_notification.