Skip to content

implement futures::stream::Stream for WebexEventStream  #7

@TonySamuels

Description

@TonySamuels

With the current implementation, the documented way to use the stream in the examples is

while let Ok(event) = event_stream.next().await {
  ...
}

This means if an error is hit, such as a parsing failure (which I have encountered), then:

  1. You don't know what error has been hit, It could be a parsing failure, in which case it's fine, but it could be the websocket is closed.
  2. You need to loop over the while loop in order to keep using the connection if an error is hit.
  3. If you do access the error, you should check is_open to verify whether the event_stream can be re-used, or needs to be recreated.

So instead the code ends up doing something like:

while event_stream.is_open() { match event_stream.next().await { ... } }

It would enable the utilities from futures, and fit better with the rust async book if Stream was implemented instead. Presumably with Item = Result<types::Event, Error>, which means the event can be handled exactly the same way, with the following changes:

  • The loop can be replace with while let Some(event) = event_stream.next().await { ... }, whilst maintaining access to the error
  • Errors that mean the stream cannot be continued to be used (probably just websocket closure at this point) can return None (as Streams return Option<Self::Item>) - consistent with other Streams, instead of changing a boolean field.

I'll probably end up implementing this in my code as a wrapper around the event_stream, but if you agree this would be a sensible change then I'm happy to discuss the details and raise a PR for the changes.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions