Skip to content

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 Simon-Laux and iequidoo May 20, 2024 10:03
@link2xt link2xt merged commit b32fb05 into main May 20, 2024
@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.

4 participants