Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Provide Unwrap an method on manet.Conn, transport.CapableConn, and network.Conn #3196

Open
sukunrt opened this issue Feb 16, 2025 · 0 comments

Comments

@sukunrt
Copy link
Member

sukunrt commented Feb 16, 2025

Currently wrapping any of these connection types is very error prone. We've run in to interface assertion errors a bunch of times around these connections.
The last few were with the introduction of the shared tcp listener for tcp and websocket.

The problem is that any wrapping breaks interface assertions downstream.
Most recently, when implementing #3186 I ran into it again when the tcp.tracingListener rejected the wrapped manet.Conn as it didn't expose the tcp metrics tracing methods. Here's the code where this happens: https://github.com/libp2p/go-libp2p/blob/master/p2p/transport/tcp/metrics.go#L292

If we add Unwrap() Conn methods to the most used types(manet.Conn, transport.Conn, network.Conn), similar to the Unwrap() []error method on error wrappers, we will have an idiomatic way of asserting if any connection in the whole wrapping chain implements the expected method.

This will still be brittle. It depends on correctly implementing Unwrap always. But it should be better than the situation right now.

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

No branches or pull requests

1 participant