Skip to content

Conversation

@dimartiro
Copy link
Contributor

Description

Improve robustness by replacing .expect() calls with proper error handling in opening.rs.

Instead of panicking when on_noise_channel_open() or on_noise_channel_data() fail, the errors are now logged and the connection is gracefully closed by returning WebRtcEvent::ConnectionClosed

Closes #350
Closes ChainSafe/gossamer-parity#77

@haikoschol
Copy link
Contributor

lgtm. @dimartiro where these the only places in the webrtc implementation that use expect(), unwrap(), etc? I understood the issue more general than just being about opening.rs and IIRC there are more places that need proper error handling. Having said that, having many small PRs is probably a good approach either way.

@dimartiro
Copy link
Contributor Author

lgtm. @dimartiro where these the only places in the webrtc implementation that use expect(), unwrap(), etc? I understood the issue more general than just being about opening.rs and IIRC there are more places that need proper error handling. Having said that, having many small PRs is probably a good approach either way.

Thanks for the review!
So far, these are the ones annotated with the issue in the comments. I'm not too familiar with the code yet, but I'll take a look and check if there are more places where this change might apply

Copy link
Collaborator

@lexnv lexnv left a comment

Choose a reason for hiding this comment

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

Looks good to me! I would keep the issues open until we sort out all the places we might panic 🙏

@lexnv lexnv merged commit ba3d7bd into paritytech:master Jan 12, 2026
8 checks passed
dmitry-markin added a commit that referenced this pull request Jan 21, 2026
## [0.13.0] - 2026-01-21

This release brings multiple fixes to both the transport and
application-level protocols.

Specifically, it enhances WebSocket stability by resolving AsyncWrite
errors and ensuring that partial writes during the negotiation phase no
longer trigger connection failures.

At the same time, Bitswap client functionality is introduced, which
makes this release semver breaking.

### Added

- Add Bitswap client
([#501](#501))

### Fixed

- notif/fix: Avoid CPU busy loops on litep2p full shutdown
([#521](#521))
- protocol: Ensure transport manager knows about closed connections
([#515](#515))
- substream: Decrement the bytes counter to avoid excessive flushing
([#511](#511))
- crypto/noise: Improve stability of websockets by fixing AsyncWrite
implementation ([#518](#518))
- bitswap: Split block responses into batches under 2 MiB
([#516](#516))
- crypto/noise: Fix connection negotiation logic on partial writes
([#519](#519))
- substream/fix: Fix partial reads for ProtocolCodec::Identity
([#512](#512))
- webrtc: Avoid panics returning error instead
([#509](#509))
- bitswap: e2e test & max payload fix
([#508](#508))
- tcp: Exit connections when events fail to propagate to protocols
([#506](#506))
- webrtc: Avoid future being dropped when channel is full
([#483](#483))

---------

Co-authored-by: Alexandru Vasile <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

webrtc: Improve robustness by avoiding panics webrtc: Improve robustness by avoiding panics

3 participants