webrtc-rs/webrtc

Use error enums instead of Anyhow

Closed this issue · 7 comments

algesten commented

Original discussion on discord, and further issue raised here: webrtc-rs/rtp#14

Anyhow is great, but maybe not a good fit for webrtc.rs. To summarize:

  • Anyhow is mostly meant for applications, not libraries.
  • Anyhow doesn't implement std::error::Error (it can't), which makes it hard to interoperate in apps that rely on that.
  • Not having explicit enumerations of possible errors means the webrtc.rs API effectively have "hidden" code paths. It's not possible for a user to know which errors could potentially be thrown from an API call returning anyhow::Result<X>

Way forward

  • Replace anyhow with idiomatic Rust error enums.
  • Keep thiserror to help implement said enums.
  • Maintain ergonomics for ? use internally in webrtc.rs (work with From traits and rewrap errors).
  • Bring out possible errors in API calls. Either via types or documentation.

This issue will be used to coordinate the effort which will span all the sub-crates.

  • sdp
  • util (done, but deliberately marked as draft)
  • mdns
  • stun
  • turn
  • ice
  • dtls
  • rtp
  • rtcp
  • srtp
  • sctp
  • media
  • interceptor
  • data
  • webrtc
algesten commented

First stab at sdp is done. Next up I'll do util. I want to do from the "bottom up" following the dependency tree, and we won't get a good idea of how this PR will work before we get to the second/third level of dependencies.

algesten commented

@rainliu Would you mind looking at the two first PRs? I'm wondering what you think about it so far. I've hard to maintain the convenience of ? and PartialEq.

rainliu commented

@algesten, thanks for the efforts.

PR for sdp crate looks good to me.

and I leave some comments on PR for util crate.

algesten commented

Alright. That's all for me for today. I'll continue tomorrow.

  • 👍1
algesten commented

@rainliu I believe this is done. The next step is probably reviewing, merging and releasing new versions of the subcrates.

algesten commented

@rainliu I've fixed clippy lints in all the PRs, and also speculatively bump deps everywhere. I assumed since this is a breaking change, we're doing minor revisions everywhere. This is the version plan I've bumped to:

sdp 0.3.0 
util 0.5.0 (released already)
mdns 0.4.0
stun 0.4.0
turn 0.5.0
ice 0.5.0
dtls 0.5.0
rtp 0.5.0
rtcp 0.5.0
srtp 0.6.0
sctp 0.4.0
media 0.3.0
interceptor 0.3.0
data 0.3.0
algesten commented

The idea is that if you merge/bump (to the version above)/release in the right order, it should "just work"