Replace unwrap/expect with proper error propagation#535
Conversation
haikoschol
left a comment
haikoschol
left a comment
There was a problem hiding this comment.
A lot of these are actually not in the webrtc transport, but AFAICT can't hurt to address them either.
yeah I wanted to solve as much as possible not only the ones related to webrtc |
|
@lexnv conflicts solved |
|
@dimartiro could you ask Claude to do a self-review on this PR to highlight the top 10 issues introduced here and try to tackle each of them 🙏 I believe this aims at tackling webrtc specific work not necesarily quic? |
@lexnv good call on the scope — I ran a self-review and you're right, this was reaching well beyond WebRTC. I've narrowed it down to WebRTC-specific work only. |
Three `.expect()` sites that upstream introduced after the original avoid_panics commit was authored — replace with proper error propagation so the dial/setup paths cannot panic: - `quic/mod.rs` async dial: `make_client_config` failure now returns `QuicError::InvalidCertificate`; `IdleTimeout::try_from` overflow returns `DialError::Timeout`. - `webrtc/mod.rs` transport setup: `Multihash::wrap` on the listener certificate fingerprint propagates `Error::InvalidData` instead of panicking on length mismatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…hanges Per review feedback, narrow this PR to webrtc-specific work. Revert the unwrap/expect removals in quic, s2n-quic and the shared noise codec; keep only the changes under src/transport/webrtc/.
3 participants
Description
Replace
.unwrap()and.expect()calls with proper error propagation in functions that already return Result, reducing the risk of unexpected panics in production.This continues the work started in #509 and is also related to #350.