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

Provide an option to specify base context for RPC connection #595

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

homier
Copy link

@homier homier commented Nov 28, 2024

Rationale

Providing a function to build base context is really useful in situations, when we need to propagate some, let's say, identity data to server handlers. Let's take the following issue as an example:

  • "I need to pass the remote peer ID to my underlying handlers, so I could do some basic identification stuff"

The problem is that now the only way to do this is to set it in rpc.Conn and pass the connection to handlers, but this breaks abstraction between transport layer and application layer.
But we could do a simple change to have a context factory, so all the necessary connection information could be set in context, that will be passed to handlers later. And it's similar to how the native http.Server interface is built.

Real-life example (using libp2p as network stack):

package handler

import 	(
       "capnproto.org/go/capnp/v3/rpc"
       "github.com/libp2p/go-libp2p/core"
)

type Handler struct {
    ...
}

func (h *Handler) Serve(stream core.Stream) *rpc.Conn {
        remotePeerID := stream.Conn().RemotePeer()

	conn := rpc.NewConn(rpc.NewPackedStreamTransport(stream), &rpc.Options{
		BootstrapClient: h.resolverHandler.Client(),
		BaseContext: func() context.Context {
		       return context.WithValue(context.Background(), "peerID", remotePeerID)
		},
	})

	return conn
}

Signed-off-by: Dzmitry Mikhalapau <[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.

2 participants