Skip to content
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.

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

misc: handle receiver errors in the event bus #367

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

undercover-cactus
Copy link
Contributor

In this PR, we handle errors coming from the event bus receiver. It also changes the way we behave in case of Lagged error.

There is 2 possible errors return from the receiver :

  • Lagged - In this scenario we print a warning message saying that messages have been skipped and we keep reading from the receiver for new messages. This error should only happened when handling events that are p2p. If a peer fill the channel older messages will be dropped and newer will be read. The limit is 2000 events so it will limit the number of tasks being spawned at once avoiding risk of DoS by a peer.
  • Closed - This error will happen if there is no more available sender. It shouldn't happen normally. In this case we print a warning message and leave function.

In the scenario the event bus is handling event from the runtime, we are assuming that the limit 2000 is more than enough to never fill the channel because we can handle events faster that the blockchain can accept new transactions.

Copy link
Contributor

@ffarall ffarall left a comment

Choose a reason for hiding this comment

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

Neat job

}
}

pub struct EventBusListener<T: EventBusMessage, E: EventHandler<T>> {
spawner: TaskSpawner,
receiver: broadcast::Receiver<T>,
event_handler: E,
// Indicate if the event is critical or not and if the receiver can drop it safely or have to panic.
Copy link
Contributor

Choose a reason for hiding this comment

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

Really appreciate the comment here. If you can add comments to other parts of the code here, much appreciated. But I do admit that this whole file is lacking docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants