BiagioFesta / wtransport

Async-friendly WebTransport implementation in Rust
Apache License 2.0
471 stars 31 forks source link

Don't panic upon connect error #95

Closed cBournhonesque closed 1 year ago

cBournhonesque commented 1 year ago

Hi,

Would it be possible to not panic here: https://github.com/BiagioFesta/wtransport/blob/master/wtransport/src/endpoint.rs#L306

when trying to connect to the server but instead return propagate the ConnectError?

I'm not sure if that's the cause of my issue. I'm creating a Client who tries to connect to a random address (with no server). I would expect to get the error "ConnectionError" but instead I get a called `Option::unwrap()` on a `None` value

BiagioFesta commented 1 year ago

Hi,

Definitely inner library panics are meant only for unreachable code paths, and they should not depend on client inputs.

As mention in the code, the expectation there is that all connection parameters have been already validate (a priori), thus I would not expect a invalid args there.

However, I am slightly confused because

called `Option::unwrap()` on a `None` value`

mentions an unwrap over Option, while connect use expect over a Result.

Could you please try to reproduce and provide to me some step how to do it on my side, so I can better understand the problem? Moreover, you could post the entire stack error you got on panic.

Thank you for your feedback, I really appreciate it

cBournhonesque commented 1 year ago

I'm not too familiar with async, but it looks like the error I was getting was because I was using a different executor (async-executor) instead of the tokio executor!

So the error is unrelated to your crate :) Thanks for the quick response.