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

fix: ignore event channel overflows #5605

Merged
merged 1 commit into from
May 20, 2024
Merged

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented May 20, 2024

async-broadcast returns Overflowed error once
if channel overflow happened.
Public APIs such as get_next_event JSON-RPC method are only expecting an error if the channel is closed, so we should not propagate overflow error outside. In particular, Delta Chat Desktop
stop receiving events completely if an error
is returned once.
If overflow happens, we should ignore it
and try again until we get an event or an error because the channel is closed (in case of recv())
or empty (in case of try_recv()).

async-broadcast returns Overflowed error once
if channel overflow happened.
Public APIs such as get_next_event JSON-RPC method
are only expecting an error if the channel is closed,
so we should not propagate overflow error outside.
In particular, Delta Chat Desktop
stop receiving events completely if an error
is returned once.
If overflow happens, we should ignore it
and try again until we get an event or an error because
the channel is closed (in case of recv())
or empty (in case of try_recv()).
@link2xt
Copy link
Collaborator Author

link2xt commented May 20, 2024

The problem exists since switch to async-broadcast in #5478. I discovered the problem while playing with realtime channels with webxdc and creating too many WebxdcRealtimeData events.

@link2xt link2xt added the bug Something is not working label May 20, 2024
@link2xt link2xt requested review from iequidoo and Simon-Laux May 20, 2024 10:03
@link2xt link2xt merged commit b32fb05 into main May 20, 2024
37 checks passed
@link2xt link2xt deleted the link2xt/fix-event-overflow branch May 20, 2024 10:44
Err(async_broadcast::RecvError::Overflowed(_)) => {
// Some events have been lost,
// but the channel is not closed.
continue;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's worth returning a special event telling that some events are lost? I.e. if we don't see such an event in the log, then no events are missing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PR adding this event: #5616

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

Successfully merging this pull request may close these issues.

3 participants