versatica / mediasoup

Cutting Edge WebRTC Video Conferencing
https://mediasoup.org
ISC License
6.18k stars 1.12k forks source link

[Rust] `Box<dyn Error>` error types make mediasoup error handling annoying #1325

Closed PaulOlteanu closed 7 months ago

PaulOlteanu commented 8 months ago

mediasoup for Rust has a lot of error enums with a variant that wraps a Box<dyn Error> and not Box<dyn Error + Send + Sync>. These are annoying to work with because they make a lot of futures !Send which makes some things harder than they should be (and in the case of piping producers, very hard to do).

I've created a demo repo here, with 2 examples that show the issues I'm having. I put the compile errors in that readme rather than here because they're very verbose.

src/bin/one.rs is a very simple example, where I'd like to be able to use Rust's question mark operator to return an anyhow::Error in the case of an error, but line 18 causes an error if uncommented (and if 19 and 20 are commented). Alternatively I could have do_mediasoup_stuff return a Result<(), Box<dyn Error>> itself, but that means I can't use do_mediasoup_stuff from a Send future (so in this case it would cause an error when spawning it with tokio).

src/bin/two.rs shows a similar case, where I use the .map_err(|e| anyhow!(e.to_string())) trick I used in example 1 but still get a similar error, this time caused by pipe_producer_to_router (this is the issue that motivated this ticket). In order to get example 2 to compile, I would have to wrap the call to pipe_producer_to_router in a tokio::task::spawn_local, and await that task (I believe probably because pipe_producer_to_router awaits on some other !Send future).

Anyway, I'd like to propose either replacing the uses of Box<dyn Error> with Box<dyn Error + Send + Sync>> (unless there's some reason errors shouldn't be sent across threads, but I doubt that) or using anyhow for the catch-all error variants within mediasoup.

I think I could make a PR for whichever option is preferred

nazar-pc commented 7 months ago

Box<dyn Error + Send + Sync> is fine if it compiled, feel free to send a PR