From 777ccdade39c7b483410585d5e65e88db304f156 Mon Sep 17 00:00:00 2001 From: Jonathan Gillham Date: Wed, 10 Feb 2016 14:52:59 +0000 Subject: [PATCH] peer: Remove error return from Connect. --- peer/example_test.go | 10 ++---- peer/peer.go | 11 ++++--- peer/peer_test.go | 77 +++++++++++++++++++------------------------- server.go | 11 ++----- 4 files changed, 45 insertions(+), 64 deletions(-) diff --git a/peer/example_test.go b/peer/example_test.go index 7ca8f74c..ff524efc 100644 --- a/peer/example_test.go +++ b/peer/example_test.go @@ -39,10 +39,7 @@ func mockRemotePeer() error { // Create and start the inbound peer. p := peer.NewInboundPeer(peerCfg) - if err := p.Connect(conn); err != nil { - fmt.Printf("Connect: error %v\n", err) - return - } + p.Connect(conn) }() return nil @@ -93,10 +90,7 @@ func Example_newOutboundPeer() { fmt.Printf("net.Dial: error %v\n", err) return } - if err := p.Connect(conn); err != nil { - fmt.Printf("Connect: error %v\n", err) - return - } + p.Connect(conn) // Wait for the verack message or timeout in case of failure. select { diff --git a/peer/peer.go b/peer/peer.go index 7db2b586..b837a115 100644 --- a/peer/peer.go +++ b/peer/peer.go @@ -775,7 +775,6 @@ func (p *Peer) pushVersionMsg() error { // recently seen nonces. nonce, err := wire.RandomUint64() if err != nil { - fmt.Println(err) return err } sentNonces.Add(nonce) @@ -1951,10 +1950,10 @@ func (p *Peer) QueueInventory(invVect *wire.InvVect) { // Connect uses the given conn to connect to the peer. Calling this function when // the peer is already connected will have no effect. -func (p *Peer) Connect(conn net.Conn) error { +func (p *Peer) Connect(conn net.Conn) { // Already connected? if !atomic.CompareAndSwapInt32(&p.connected, 0, 1) { - return nil + return } if p.inbound { @@ -1963,7 +1962,11 @@ func (p *Peer) Connect(conn net.Conn) error { p.conn = conn p.timeConnected = time.Now() - return p.start() + go func() { + if err := p.start(); err != nil { + log.Errorf("Cannot start peer %v: %v", p, err) + } + }() } // Connected returns whether or not the peer is currently connected. diff --git a/peer/peer_test.go b/peer/peer_test.go index cddcdfaf..f2475b9c 100644 --- a/peer/peer_test.go +++ b/peer/peer_test.go @@ -245,16 +245,13 @@ func TestPeerConnection(t *testing.T) { &conn{raddr: "10.0.0.2:8333"}, ) inPeer := peer.NewInboundPeer(peerCfg) - if err := inPeer.Connect(inConn); err != nil { - return nil, nil, err - } + inPeer.Connect(inConn) + outPeer, err := peer.NewOutboundPeer(peerCfg, "10.0.0.2:8333") if err != nil { return nil, nil, err } - if err := outPeer.Connect(outConn); err != nil { - return nil, nil, err - } + outPeer.Connect(outConn) for i := 0; i < 2; i++ { select { @@ -274,16 +271,14 @@ func TestPeerConnection(t *testing.T) { &conn{raddr: "10.0.0.2:8333"}, ) inPeer := peer.NewInboundPeer(peerCfg) - if err := inPeer.Connect(inConn); err != nil { - return nil, nil, err - } + inPeer.Connect(inConn) + outPeer, err := peer.NewOutboundPeer(peerCfg, "10.0.0.2:8333") if err != nil { return nil, nil, err } - if err := outPeer.Connect(outConn); err != nil { - return nil, nil, err - } + outPeer.Connect(outConn) + for i := 0; i < 2; i++ { select { case <-verack: @@ -393,10 +388,8 @@ func TestPeerListeners(t *testing.T) { &conn{raddr: "10.0.0.2:8333"}, ) inPeer := peer.NewInboundPeer(peerCfg) - if err := inPeer.Connect(inConn); err != nil { - t.Errorf("TestPeerListeners: unexpected err %v\n", err) - return - } + inPeer.Connect(inConn) + peerCfg.Listeners = peer.MessageListeners{ OnVerAck: func(p *peer.Peer, msg *wire.MsgVerAck) { verack <- struct{}{} @@ -407,10 +400,8 @@ func TestPeerListeners(t *testing.T) { t.Errorf("NewOutboundPeer: unexpected err %v\n", err) return } - if err := outPeer.Connect(outConn); err != nil { - t.Errorf("TestPeerListeners: unexpected err %v\n", err) - return - } + outPeer.Connect(outConn) + for i := 0; i < 2; i++ { select { case <-verack: @@ -524,14 +515,11 @@ func TestPeerListeners(t *testing.T) { // TestOutboundPeer tests that the outbound peer works as expected. func TestOutboundPeer(t *testing.T) { - // Use a mock NewestBlock func to test errs - var errBlockNotFound = errors.New("newest block not found") - var mockNewestSha = func() (*wire.ShaHash, int32, error) { - return nil, 0, errBlockNotFound - } peerCfg := &peer.Config{ - NewestBlock: mockNewestSha, + NewestBlock: func() (*wire.ShaHash, int32, error) { + return nil, 0, errors.New("newest block not found") + }, UserAgentName: "peer", UserAgentVersion: "1.0", ChainParams: &chaincfg.MainNetParams, @@ -547,26 +535,35 @@ func TestOutboundPeer(t *testing.T) { return } - wantErr := errBlockNotFound - if err := p.Connect(c); err != wantErr { - t.Errorf("Connect: expected err %v, got %v\n", wantErr, err) - return + // Test trying to connect twice. + p.Connect(c) + p.Connect(c) + + disconnected := make(chan struct{}) + go func() { + p.WaitForDisconnect() + disconnected <- struct{}{} + }() + + select { + case <-disconnected: + case <-time.After(time.Second): + t.Fatal("Peer did not automatically disconnect.") } - // Test already connected. - if err := p.Connect(c); err != nil { - t.Errorf("Connect: unexpected err %v\n", err) - return + if p.Connected() { + t.Fatalf("Should not be connected as NewestBlock produces error.") } // Test Queue Inv fakeBlockHash := &wire.ShaHash{0: 0x00, 1: 0x01} fakeInv := wire.NewInvVect(wire.InvTypeBlock, fakeBlockHash) + + // Should be noops as the peer could not connect. p.QueueInventory(fakeInv) p.AddKnownInventory(fakeInv) p.QueueInventory(fakeInv) - // Test Queue Message fakeMsg := wire.NewMsgVerAck() p.QueueMessage(fakeMsg, nil) done := make(chan struct{}) @@ -591,10 +588,7 @@ func TestOutboundPeer(t *testing.T) { t.Errorf("NewOutboundPeer: unexpected err - %v\n", err) return } - if err := p1.Connect(c1); err != nil { - t.Errorf("Connect: unexpected err %v\n", err) - return - } + p1.Connect(c1) // Test update latest block latestBlockSha, err := wire.NewShaHashFromStr("1a63f9cdff1752e6375c8c76e543a71d239e1a2e5c6db1aa679") @@ -624,10 +618,7 @@ func TestOutboundPeer(t *testing.T) { t.Errorf("NewOutboundPeer: unexpected err - %v\n", err) return } - if err := p2.Connect(c2); err != nil { - t.Errorf("Connect: unexpected err %v\n", err) - return - } + p2.Connect(c2) // Test PushXXX var addrs []*wire.NetAddress diff --git a/server.go b/server.go index 008bb9a0..25bdce4c 100644 --- a/server.go +++ b/server.go @@ -1573,12 +1573,7 @@ func (s *server) listenHandler(listener net.Listener) { sp := newServerPeer(s, false) sp.Peer = peer.NewInboundPeer(newPeerConfig(sp)) go s.peerDoneHandler(sp) - if err := sp.Connect(conn); err != nil { - if atomic.LoadInt32(&s.shutdown) == 0 { - srvrLog.Errorf("Can't accept connection: %v", err) - } - continue - } + sp.Connect(conn) } s.wg.Done() srvrLog.Tracef("Listener handler done for %s", listener.Addr()) @@ -1674,9 +1669,7 @@ func (s *server) establishConn(sp *serverPeer) error { if err != nil { return err } - if err := sp.Connect(conn); err != nil { - return err - } + sp.Connect(conn) srvrLog.Debugf("Connected to %s", sp.Addr()) s.addrManager.Attempt(sp.NA()) return nil