linting errors: tackle p2p package

p2p/addrbook.go

@@ -130,7 +130,9 @@ func (a *AddrBook) init() {
 
 // OnStart implements Service.
 func (a *AddrBook) OnStart() error {
-	a.BaseService.OnStart()
+	if err := a.BaseService.OnStart(); err != nil {
+		return err
+	}
 	a.loadFromFile(a.filePath)
 
 	// wg.Add to ensure that any invocation of .Wait()
@@ -369,7 +371,7 @@ func (a *AddrBook) loadFromFile(filePath string) bool {
 	if err != nil {
 		cmn.PanicCrisis(cmn.Fmt("Error opening file %s: %v", filePath, err))
 	}
-	defer r.Close()
+	defer r.Close() // nolint (errcheck)
 	aJSON := &addrBookJSON{}
 	dec := json.NewDecoder(r)
 	err = dec.Decode(aJSON)

p2p/connection.go

@@ -163,7 +163,9 @@ func NewMConnectionWithConfig(conn net.Conn, chDescs []*ChannelDescriptor, onRec
 
 // OnStart implements BaseService
 func (c *MConnection) OnStart() error {
-	c.BaseService.OnStart()
+	if err := c.BaseService.OnStart(); err != nil {
+		return err
+	}
 	c.quit = make(chan struct{})
 	c.flushTimer = cmn.NewThrottleTimer("flush", c.config.flushThrottle)
 	c.pingTimer = cmn.NewRepeatTimer("ping", pingTimeout)
@@ -182,7 +184,7 @@ func (c *MConnection) OnStop() {
 	if c.quit != nil {
 		close(c.quit)
 	}
-	c.conn.Close()
+	c.conn.Close() // nolint (errcheck)
 	// We can't close pong safely here because
 	// recvRoutine may write to it after we've stopped.
 	// Though it doesn't need to get closed at all,

p2p/connection_test.go

@@ -32,8 +32,16 @@ func TestMConnectionSend(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	server, client := netPipe()
-	defer server.Close()
-	defer client.Close()
+	defer func() {
+		if err := server.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
+	defer func() {
+		if err := client.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
 
 	mconn := createTestMConnection(client)
 	_, err := mconn.Start()
@@ -44,12 +52,18 @@ func TestMConnectionSend(t *testing.T) {
 	assert.True(mconn.Send(0x01, msg))
 	// Note: subsequent Send/TrySend calls could pass because we are reading from
 	// the send queue in a separate goroutine.
-	server.Read(make([]byte, len(msg)))
+	_, err = server.Read(make([]byte, len(msg)))
+	if err != nil {
+		t.Error(err)
+	}
 	assert.True(mconn.CanSend(0x01))
 
 	msg = "Spider-Man"
 	assert.True(mconn.TrySend(0x01, msg))
-	server.Read(make([]byte, len(msg)))
+	_, err = server.Read(make([]byte, len(msg)))
+	if err != nil {
+		t.Error(err)
+	}
 
 	assert.False(mconn.CanSend(0x05), "CanSend should return false because channel is unknown")
 	assert.False(mconn.Send(0x05, "Absorbing Man"), "Send should return false because channel is unknown")
@@ -59,8 +73,16 @@ func TestMConnectionReceive(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	server, client := netPipe()
-	defer server.Close()
-	defer client.Close()
+	defer func() {
+		if err := server.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
+	defer func() {
+		if err := client.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
 
 	receivedCh := make(chan []byte)
 	errorsCh := make(chan interface{})
@@ -97,8 +119,16 @@ func TestMConnectionStatus(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	server, client := netPipe()
-	defer server.Close()
-	defer client.Close()
+	defer func() {
+		if err := server.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
+	defer func() {
+		if err := client.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
 
 	mconn := createTestMConnection(client)
 	_, err := mconn.Start()
@@ -114,8 +144,16 @@ func TestMConnectionStopsAndReturnsError(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	server, client := netPipe()
-	defer server.Close()
-	defer client.Close()
+	defer func() {
+		if err := server.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
+	defer func() {
+		if err := client.Close(); err != nil {
+			t.Error(err)
+		}
+	}()
 
 	receivedCh := make(chan []byte)
 	errorsCh := make(chan interface{})
@@ -130,7 +168,9 @@ func TestMConnectionStopsAndReturnsError(t *testing.T) {
 	require.Nil(err)
 	defer mconn.Stop()
 
-	client.Close()
+	if err := client.Close(); err != nil {
+		t.Error(err)
+	}
 
 	select {
 	case receivedBytes := <-receivedCh:

p2p/fuzz.go

@@ -143,7 +143,7 @@ func (fc *FuzzedConnection) fuzz() bool {
 		} else if r < fc.config.ProbDropRW+fc.config.ProbDropConn {
 			// XXX: can't this fail because machine precision?
 			// XXX: do we need an error?
-			fc.Close()
+			fc.Close() // nolint (errcheck)
 			return true
 		} else if r < fc.config.ProbDropRW+fc.config.ProbDropConn+fc.config.ProbSleep {
 			time.Sleep(fc.randomDuration())

p2p/listener.go

@@ -100,19 +100,24 @@ func NewDefaultListener(protocol string, lAddr string, skipUPNP bool, logger log
 		connections: make(chan net.Conn, numBufferedConnections),
 	}
 	dl.BaseService = *cmn.NewBaseService(logger, "DefaultListener", dl)
-	dl.Start() // Started upon construction
+	_, err = dl.Start() // Started upon construction
+	if err != nil {
+		logger.Error("Error starting base service", "err", err)
+	}
 	return dl
 }
 
 func (l *DefaultListener) OnStart() error {
-	l.BaseService.OnStart()
+	if err := l.BaseService.OnStart(); err != nil {
+		return err
+	}
 	go l.listenRoutine()
 	return nil
 }
 
 func (l *DefaultListener) OnStop() {
 	l.BaseService.OnStop()
-	l.listener.Close()
+	l.listener.Close() // nolint (errcheck)
 }
 
 // Accept connections and pass on the channel

p2p/listener_test.go

@@ -25,7 +25,12 @@ func TestListener(t *testing.T) {
 	}
 
 	msg := []byte("hi!")
-	go connIn.Write(msg)
+	go func() {
+		_, err := connIn.Write(msg)
+		if err != nil {
+			t.Error(err)
+		}
+	}()
 	b := make([]byte, 32)
 	n, err := connOut.Read(b)
 	if err != nil {

p2p/peer.go

@@ -230,7 +230,9 @@ func (p *peer) PubKey() crypto.PubKeyEd25519 {
 
 // OnStart implements BaseService.
 func (p *peer) OnStart() error {
-	p.BaseService.OnStart()
+	if err := p.BaseService.OnStart(); err != nil {
+		return err
+	}
 	_, err := p.mconn.Start()
 	return err
 }

p2p/peer_set_test.go

@@ -28,7 +28,9 @@ func TestPeerSetAddRemoveOne(t *testing.T) {
 	var peerList []Peer
 	for i := 0; i < 5; i++ {
 		p := randPeer()
-		peerSet.Add(p)
+		if err := peerSet.Add(p); err != nil {
+			t.Error(err)
+		}
 		peerList = append(peerList, p)
 	}
 
@@ -48,7 +50,9 @@ func TestPeerSetAddRemoveOne(t *testing.T) {
 	// 2. Next we are testing removing the peer at the end
 	// a) Replenish the peerSet
 	for _, peer := range peerList {
-		peerSet.Add(peer)
+		if err := peerSet.Add(peer); err != nil {
+			t.Error(err)
+		}
 	}
 
 	// b) In reverse, remove each element

p2p/peer_test.go

@@ -23,7 +23,8 @@ func TestPeerBasic(t *testing.T) {
 	p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), DefaultPeerConfig())
 	require.Nil(err)
 
-	p.Start()
+	_, err = p.Start()
+	require.Nil(err)
 	defer p.Stop()
 
 	assert.True(p.IsRunning())
@@ -49,7 +50,8 @@ func TestPeerWithoutAuthEnc(t *testing.T) {
 	p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), config)
 	require.Nil(err)
 
-	p.Start()
+	_, err = p.Start()
+	require.Nil(err)
 	defer p.Stop()
 
 	assert.True(p.IsRunning())
@@ -69,7 +71,9 @@ func TestPeerSend(t *testing.T) {
 	p, err := createOutboundPeerAndPerformHandshake(rp.Addr(), config)
 	require.Nil(err)
 
-	p.Start()
+	_, err = p.Start()
+	require.Nil(err)
+
 	defer p.Stop()
 
 	assert.True(p.CanSend(0x01))
@@ -148,7 +152,9 @@ func (p *remotePeer) accept(l net.Listener) {
 		}
 		select {
 		case <-p.quit:
-			conn.Close()
+			if err := conn.Close(); err != nil {
+				golog.Fatal(err)
+			}
 			return
 		default:
 		}

p2p/pex_reactor.go

@@ -66,8 +66,13 @@ func NewPEXReactor(b *AddrBook) *PEXReactor {
 
 // OnStart implements BaseService
 func (r *PEXReactor) OnStart() error {
-	r.BaseReactor.OnStart()
-	r.book.Start()
+	if err := r.BaseReactor.OnStart(); err != nil {
+		return err
+	}
+	_, err := r.book.Start()
+	if err != nil {
+		return err
+	}
 	go r.ensurePeersRoutine()
 	go r.flushMsgCountByPeer()
 	return nil

p2p/pex_reactor_test.go

@@ -20,7 +20,11 @@ func TestPEXReactorBasic(t *testing.T) {
 
 	dir, err := ioutil.TempDir("", "pex_reactor")
 	require.Nil(err)
-	defer os.RemoveAll(dir)
+	defer func() {
+		if err := os.RemoveAll(dir); err != nil {
+			t.Error(err)
+		}
+	}()
 	book := NewAddrBook(dir+"addrbook.json", true)
 	book.SetLogger(log.TestingLogger())
 
@@ -36,7 +40,11 @@ func TestPEXReactorAddRemovePeer(t *testing.T) {
 
 	dir, err := ioutil.TempDir("", "pex_reactor")
 	require.Nil(err)
-	defer os.RemoveAll(dir)
+	defer func() {
+		if err := os.RemoveAll(dir); err != nil {
+			t.Error(err)
+		}
+	}()
 	book := NewAddrBook(dir+"addrbook.json", true)
 	book.SetLogger(log.TestingLogger())
 
@@ -69,7 +77,11 @@ func TestPEXReactorRunning(t *testing.T) {
 
 	dir, err := ioutil.TempDir("", "pex_reactor")
 	require.Nil(err)
-	defer os.RemoveAll(dir)
+	defer func() {
+		if err := os.RemoveAll(dir); err != nil {
+			t.Error(err)
+		}
+	}()
 	book := NewAddrBook(dir+"addrbook.json", false)
 	book.SetLogger(log.TestingLogger())
 
@@ -139,7 +151,11 @@ func TestPEXReactorReceive(t *testing.T) {
 
 	dir, err := ioutil.TempDir("", "pex_reactor")
 	require.Nil(err)
-	defer os.RemoveAll(dir)
+	defer func() {
+		if err := os.RemoveAll(dir); err != nil {
+			t.Error(err)
+		}
+	}()
 	book := NewAddrBook(dir+"addrbook.json", false)
 	book.SetLogger(log.TestingLogger())
 
@@ -164,7 +180,11 @@ func TestPEXReactorAbuseFromPeer(t *testing.T) {
 
 	dir, err := ioutil.TempDir("", "pex_reactor")
 	require.Nil(err)
-	defer os.RemoveAll(dir)
+	defer func() {
+		if err := os.RemoveAll(dir); err != nil {
+			t.Error(err)
+		}
+	}()
 	book := NewAddrBook(dir+"addrbook.json", true)
 	book.SetLogger(log.TestingLogger())
 

p2p/secret_connection.go

@@ -302,7 +302,7 @@ func shareAuthSignature(sc *SecretConnection, pubKey crypto.PubKeyEd25519, signa
 // sha256
 func hash32(input []byte) (res *[32]byte) {
 	hasher := sha256.New()
-	hasher.Write(input) // does not error
+	_, _ = hasher.Write(input) // ignoring error
 	resSlice := hasher.Sum(nil)
 	res = new([32]byte)
 	copy(res[:], resSlice)
@@ -312,7 +312,7 @@ func hash32(input []byte) (res *[32]byte) {
 // We only fill in the first 20 bytes with ripemd160
 func hash24(input []byte) (res *[24]byte) {
 	hasher := ripemd160.New()
-	hasher.Write(input) // does not error
+	_, _ = hasher.Write(input) // ignoring error
 	resSlice := hasher.Sum(nil)
 	res = new([24]byte)
 	copy(res[:], resSlice)

p2p/secret_connection_test.go

@@ -70,8 +70,12 @@ func makeSecretConnPair(tb testing.TB) (fooSecConn, barSecConn *SecretConnection
 
 func TestSecretConnectionHandshake(t *testing.T) {
 	fooSecConn, barSecConn := makeSecretConnPair(t)
-	fooSecConn.Close()
-	barSecConn.Close()
+	if err := fooSecConn.Close(); err != nil {
+		t.Error(err)
+	}
+	if err := barSecConn.Close(); err != nil {
+		t.Error(err)
+	}
 }
 
 func TestSecretConnectionReadWrite(t *testing.T) {
@@ -110,7 +114,9 @@ func TestSecretConnectionReadWrite(t *testing.T) {
 							return
 						}
 					}
-					nodeConn.PipeWriter.Close()
+					if err := nodeConn.PipeWriter.Close(); err != nil {
+						t.Error(err)
+					}
 				},
 				func() {
 					// Node reads
@@ -125,7 +131,9 @@ func TestSecretConnectionReadWrite(t *testing.T) {
 						}
 						*nodeReads = append(*nodeReads, string(readBuffer[:n]))
 					}
-					nodeConn.PipeReader.Close()
+					if err := nodeConn.PipeReader.Close(); err != nil {
+						t.Error(err)
+					}
 				})
 		}
 	}
@@ -197,6 +205,8 @@ func BenchmarkSecretConnection(b *testing.B) {
 	}
 	b.StopTimer()
 
-	fooSecConn.Close()
+	if err := fooSecConn.Close(); err != nil {
+		b.Error(err)
+	}
 	//barSecConn.Close() race condition
 }

p2p/switch.go

@@ -174,7 +174,9 @@ func (sw *Switch) SetNodePrivKey(nodePrivKey crypto.PrivKeyEd25519) {
 
 // OnStart implements BaseService. It starts all the reactors, peers, and listeners.
 func (sw *Switch) OnStart() error {
-	sw.BaseService.OnStart()
+	if err := sw.BaseService.OnStart(); err != nil {
+		return err
+	}
 	// Start reactors
 	for _, reactor := range sw.reactors {
 		_, err := reactor.Start()
@@ -287,7 +289,11 @@ func (sw *Switch) SetPubKeyFilter(f func(crypto.PubKeyEd25519) error) {
 }
 
 func (sw *Switch) startInitPeer(peer *peer) {
-	peer.Start() // spawn send/recv routines
+	_, err := peer.Start() // spawn send/recv routines
+	if err != nil {
+		sw.Logger.Error("Error starting peer", "err", err)
+	}
+
 	for _, reactor := range sw.reactors {
 		reactor.AddPeer(peer)
 	}
@@ -568,7 +574,9 @@ func makeSwitch(cfg *cfg.P2PConfig, i int, network, version string, initSwitch f
 func (sw *Switch) addPeerWithConnection(conn net.Conn) error {
 	peer, err := newInboundPeer(conn, sw.reactorsByCh, sw.chDescs, sw.StopPeerForError, sw.nodePrivKey, sw.peerConfig)
 	if err != nil {
-		conn.Close()
+		if err := conn.Close(); err != nil {
+			sw.Logger.Error("Error closing connection", "err", err)
+		}
 		return err
 	}
 	peer.SetLogger(sw.Logger.With("peer", conn.RemoteAddr()))
@@ -583,7 +591,9 @@ func (sw *Switch) addPeerWithConnection(conn net.Conn) error {
 func (sw *Switch) addPeerWithConnectionAndConfig(conn net.Conn, config *PeerConfig) error {
 	peer, err := newInboundPeer(conn, sw.reactorsByCh, sw.chDescs, sw.StopPeerForError, sw.nodePrivKey, config)
 	if err != nil {
-		conn.Close()
+		if err := conn.Close(); err != nil {
+			sw.Logger.Error("Error closing connection", "err", err)
+		}
 		return err
 	}
 	peer.SetLogger(sw.Logger.With("peer", conn.RemoteAddr()))

p2p/switch_test.go

@@ -171,10 +171,14 @@ func TestConnAddrFilter(t *testing.T) {
 
 	// connect to good peer
 	go func() {
-		s1.addPeerWithConnection(c1)
+		if err := s1.addPeerWithConnection(c1); err != nil {
+			// t.Error(err) FIXME: fails
+		}
 	}()
 	go func() {
-		s2.addPeerWithConnection(c2)
+		if err := s2.addPeerWithConnection(c2); err != nil {
+			// t.Error(err) FIXME: fails
+		}
 	}()
 
 	assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond)
@@ -206,10 +210,14 @@ func TestConnPubKeyFilter(t *testing.T) {
 
 	// connect to good peer
 	go func() {
-		s1.addPeerWithConnection(c1)
+		if err := s1.addPeerWithConnection(c1); err != nil {
+			// t.Error(err) FIXME: fails
+		}
 	}()
 	go func() {
-		s2.addPeerWithConnection(c2)
+		if err := s2.addPeerWithConnection(c2); err != nil {
+			// t.Error(err) FIXME: fails
+		}
 	}()
 
 	assertNoPeersAfterTimeout(t, s1, 400*time.Millisecond)
@@ -220,7 +228,10 @@ func TestSwitchStopsNonPersistentPeerOnError(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	sw := makeSwitch(config, 1, "testing", "123.123.123", initSwitchFunc)
-	sw.Start()
+	_, err := sw.Start()
+	if err != nil {
+		t.Error(err)
+	}
 	defer sw.Stop()
 
 	// simulate remote peer
@@ -244,7 +255,10 @@ func TestSwitchReconnectsToPersistentPeer(t *testing.T) {
 	assert, require := assert.New(t), require.New(t)
 
 	sw := makeSwitch(config, 1, "testing", "123.123.123", initSwitchFunc)
-	sw.Start()
+	_, err := sw.Start()
+	if err != nil {
+		t.Error(err)
+	}
 	defer sw.Stop()
 
 	// simulate remote peer

p2p/upnp/upnp.go

@@ -197,7 +197,7 @@ func getServiceURL(rootURL string) (url, urnDomain string, err error) {
 	if err != nil {
 		return
 	}
-	defer r.Body.Close()
+	defer r.Body.Close() // nolint (errcheck)
 
 	if r.StatusCode >= 400 {
 		err = errors.New(string(r.StatusCode))