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

stream: implement Error and Display for BroadcastStreamRecvError #3745

Merged
merged 2 commits into from May 5, 2021

Conversation

hyche
Copy link
Contributor

@hyche hyche commented May 3, 2021

Reuse the tokio::sync::broadcast::error::RecvError instead.

Motivation

I want to use the broadcast stream wrapper in the situtaion where some codes requiring the custom Error implementing std::error:Error trair, such as wrap_stream() function in hyper crate.

Solution

We can add impl std::error::Error, and core::fmt::Display for BroadcastStreamRecvError, but since the custom error doesn't provide any additional info, so it's better removing it and reusing the existing error from broadcast channel.

Reuse the tokio::sync::broadcast::error::RecvError instead.
@Darksonn Darksonn added A-tokio-stream Area: The tokio-stream crate M-sync Module: tokio/sync labels May 3, 2021
@Darksonn
Copy link
Contributor

Darksonn commented May 3, 2021

but since the custom error doesn't provide any additional info

It does provide additional info, namely that the wrapper never returns the Closed error as an error. Introducing a new error here was intentional.

Adding an impl for the Error trait is fine with me.

@hyche
Copy link
Contributor Author

hyche commented May 3, 2021

It does provide additional info, namely that the wrapper never returns the Closed error as an error. Introducing a new error here was intentional.

Thanks, understand it now!

Adding an impl for the Error trait is fine with me.

I have reverted and added the impl

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Thanks.

@Darksonn Darksonn changed the title stream: Remove BroadcastStreamRecvError from broadcast stream wrapper stream: implement Error and Display for BroadcastStreamRecvError May 5, 2021
@Darksonn Darksonn merged commit a945ce0 into tokio-rs:master May 5, 2021
@hyche hyche deleted the stream_remove_broadcaststream_error branch June 3, 2021 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-stream Area: The tokio-stream crate M-sync Module: tokio/sync
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants