From bfae1f9b77a15e3a7c744d5948ac8fe7a87f2301 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 10 Jan 2022 16:35:44 +0400 Subject: [PATCH 1/2] simplify error handling when running identify No functional change expected. --- p2p/protocol/identify/id.go | 43 +++++++++++++------------------------ 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/p2p/protocol/identify/id.go b/p2p/protocol/identify/id.go index 0ceb17ce56..4177f24b29 100644 --- a/p2p/protocol/identify/id.go +++ b/p2p/protocol/identify/id.go @@ -321,7 +321,6 @@ func (ids *idService) IdentifyWait(c network.Conn) <-chan struct{} { defer ids.connsMu.Unlock() wait, found = ids.conns[c] - if !found { wait = make(chan struct{}) ids.conns[c] = wait @@ -329,7 +328,14 @@ func (ids *idService) IdentifyWait(c network.Conn) <-chan struct{} { // Spawn an identify. The connection may actually be closed // already, but that doesn't really matter. We'll fail to open a // stream then forget the connection. - go ids.identifyConn(c, wait) + go func() { + defer close(wait) + if err := ids.identifyConn(c); err != nil { + ids.emitters.evtPeerIdentificationFailed.Emit(event.EvtPeerIdentificationFailed{Peer: c.RemotePeer(), Reason: err}) + return + } + ids.emitters.evtPeerIdentificationCompleted.Emit(event.EvtPeerIdentificationCompleted{Peer: c.RemotePeer()}) + }() } return wait @@ -341,24 +347,8 @@ func (ids *idService) removeConn(c network.Conn) { ids.connsMu.Unlock() } -func (ids *idService) identifyConn(c network.Conn, signal chan struct{}) { - var ( - s network.Stream - err error - ) - - defer func() { - close(signal) - - // emit the appropriate event. - if p := c.RemotePeer(); err == nil { - ids.emitters.evtPeerIdentificationCompleted.Emit(event.EvtPeerIdentificationCompleted{Peer: p}) - } else { - ids.emitters.evtPeerIdentificationFailed.Emit(event.EvtPeerIdentificationFailed{Peer: p, Reason: err}) - } - }() - - s, err = c.NewStream(network.WithUseTransient(context.TODO(), "identify")) +func (ids *idService) identifyConn(c network.Conn) error { + s, err := c.NewStream(network.WithUseTransient(context.TODO(), "identify")) if err != nil { log.Debugw("error opening identify stream", "error", err) // the connection is probably already closed if we hit this. @@ -368,21 +358,18 @@ func (ids *idService) identifyConn(c network.Conn, signal chan struct{}) { // We usually do this on disconnect, but we may have already // processed the disconnect event. ids.removeConn(c) - return + return err } s.SetProtocol(ID) // ok give the response to our handler. - if err = msmux.SelectProtoOrFail(ID, s); err != nil { - log.Infow("failed negotiate identify protocol with peer", - "peer", c.RemotePeer(), - "error", err, - ) + if err := msmux.SelectProtoOrFail(ID, s); err != nil { + log.Infow("failed negotiate identify protocol with peer", "peer", c.RemotePeer(), "error", err) s.Reset() - return + return err } - err = ids.handleIdentifyResponse(s) + return ids.handleIdentifyResponse(s) } func (ids *idService) sendIdentifyResp(s network.Stream) { From 1485100f0e18125fa8f5b67f5734694e2b5f51e9 Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Mon, 10 Jan 2022 16:37:48 +0400 Subject: [PATCH 2/2] don't close the connection when opening the identify stream fails --- p2p/protocol/identify/id.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/p2p/protocol/identify/id.go b/p2p/protocol/identify/id.go index 4177f24b29..388f16135d 100644 --- a/p2p/protocol/identify/id.go +++ b/p2p/protocol/identify/id.go @@ -351,9 +351,6 @@ func (ids *idService) identifyConn(c network.Conn) error { s, err := c.NewStream(network.WithUseTransient(context.TODO(), "identify")) if err != nil { log.Debugw("error opening identify stream", "error", err) - // the connection is probably already closed if we hit this. - // TODO: Remove this? - c.Close() // We usually do this on disconnect, but we may have already // processed the disconnect event.